Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization of optional relationships is now failing where id did not in v8.2.0 #1017

Closed
TehWardy opened this issue Aug 11, 2023 · 4 comments · Fixed by #1021
Closed

Serialization of optional relationships is now failing where id did not in v8.2.0 #1017

TehWardy opened this issue Aug 11, 2023 · 4 comments · Fixed by #1021
Assignees
Labels
bug Something isn't working regression

Comments

@TehWardy
Copy link

Assemblies affected
Which assemblies and versions are known to be affected e.g. ASP.NET Core OData 8.2.1

Describe the bug
image

I think a new bug has been introduced here since if instance is null you attempt to call GetType() on it.

I'm experiencing a problem with expands (not sure if this is related to the code i've highlighted but I suspect a few places might have this same issue, I figured i'd dig through the release notes though to find out if I could see my issue and this is the only thing that stuck out to me).

The problem does not exist in version 8.2.0 but does in 8.2.1

Reproduce steps
Expanding with $expand=NullableRelationship causes a "Nullable object cannot be null" exception to be thrown even if the metadata is showing that the relationship is fine to be nullable.

EDM (CSDL) Model
My EDM metadata looks like this ...

<EntityType Name="OfferLine">
  <Key>
  <PropertyRef Name="Id"/>
  </Key>
  <Property Name="Id" Type="Edm.Guid" Nullable="false"/>
  <Property Name="LineNumber" Type="Edm.Int32" Nullable="false"/>
  <Property Name="Description" Type="Edm.String"/>
  <Property Name="LastUpdated" Type="Edm.DateTimeOffset" Nullable="false"/>
  <Property Name="IsActive" Type="Edm.Boolean" Nullable="false"/>
  <Property Name="ProductCode" Type="Edm.String"/>
  <Property Name="LinePrice" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="UnitPrice" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="Quantity" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="TaxRate" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="TaxFee" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="TaxableAmount" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="TaxAmount" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="TransactionValue" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="FundableValue" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="Rate" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="Cost" Type="Edm.Decimal" Nullable="false" Scale="Variable"/>
  <Property Name="CurrencyId" Nullable="false" Type="Edm.String"/>
  <Property Name="OfferId" Nullable="false" Type="Edm.Guid"/>
  <Property Name="InvoiceReferenceId" Type="Edm.String"/>
  <Property Name="CreditReferenceId" Type="Edm.String"/>
  <NavigationProperty Name="Currency" Type="B2B.Objects.Entities.Masterdata.Currency">
    <ReferentialConstraint Property="CurrencyId" ReferencedProperty="Id"/>
  </NavigationProperty>
  <NavigationProperty Name="Offer" Type="B2B.Objects.Entities.Financing.Offer.Offer">
    <ReferentialConstraint Property="OfferId" ReferencedProperty="Id"/>
  </NavigationProperty>
  <NavigationProperty Name="InvoiceReference" Type="B2B.Objects.Entities.InvoiceReference">
    <ReferentialConstraint Property="InvoiceReferenceId" ReferencedProperty="Id"/>
  </NavigationProperty>
  <NavigationProperty Name="CreditReference" Type="B2B.Objects.Entities.CreditReference">
    <ReferentialConstraint Property="CreditReferenceId" ReferencedProperty="Id"/>
  </NavigationProperty>
</EntityType>

When I ask for ~/OfferLine?$expand=InvoiceReference,CreditReference (I expect only one to be populated) I get the above exception when it hits the null relationship object.

As you can see OfferId and CurrencyId are not nullable so expanding on Offer or currency I would expect an exception if either was null, but for InvoiceReference and CreditReference the relationship is not required.

Expected behavior
Serialization should succeed as per 8.2.0 but the null value should be a json null response for the relationship object.

@TehWardy TehWardy added the bug Something isn't working label Aug 11, 2023
@xuzhg
Copy link
Member

xuzhg commented Aug 14, 2023

@ElizabethOkerio It's coming from this PR ea4e79c#diff-732b595c31c0f7e9b97687d1ab7bce7d1f35d3fc2ce9adfcec3ef13e1d373b99.

It looks like a regression and would you please take a look? any help, please do a hot release for the regression.

@greg-lomax
Copy link

I have a minimal sample project that demonstrates the null value exception.

webapi + entity framework + sqlite + swagger

Expand "AzureClassUser" with asterisk (*) or "userClass".
This fails with exception on 8.2.1 but works fine in 8.2.0.

https://github.com/greg-lomax/dotnet-core-odata-webapi-sample

@greg-lomax
Copy link

Further observation is that expand and select wildcard are not working. It appears that instead of matching * to expand and/or select, * is being matched against model properties, and is failing to match.

all of these fail in 8.2.1
entityset?$expand=*
entityset?$select=*
entityset?$expand=child($select=)
entityset?$expand=child($expand=named($select=
))

etc.

xuzhg added a commit that referenced this issue Aug 18, 2023
…check (#1021)

* Fixes #1017: resolve the null reference exception by adding the null check

* Update builder.versions.settings.targets

* Update builder.versions.settings.targets to version 8.2.2
@TehWardy
Copy link
Author

Seems to be working in v8.2.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants