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

Custom serializer support for LINQ (#4138) should support STJ ExtensionData #4297

Closed
onionhammer opened this issue Feb 1, 2024 · 3 comments · Fixed by #4306
Closed

Custom serializer support for LINQ (#4138) should support STJ ExtensionData #4297

onionhammer opened this issue Feb 1, 2024 · 3 comments · Fixed by #4306
Assignees

Comments

@onionhammer
Copy link
Contributor

onionhammer commented Feb 1, 2024

Just by switching your custom serializer to inherit from 'CosmosSerializer' to 'CosmosLinqSerializer' this breaks any usage of ExtensionData in queries.

With custom serializer support, we should add detection of STJ-based 'ExtensionData' attribute in addition to the newtonsoft one. See #3777

Expected behavior
ExtensionData part of the path should be skipped in the generated SQL query:

i.e. MyObject.ExtensionData.MyCustomProperty > 42 becomes MyObject.MyCustomProperty > 42

Actual behavior
MyObject.ExtensionData.MyCustomProperty is retained, breaking the query, since STJ serializes MyCustomProperty to the root of the object.

Also, you cannot utilize CosmosLinqSerializer if you are trying to query against JSON ExtensionData.

Environment summary
SDK Version: 3.39.0-preview.0
OS Version (e.g. Windows, Linux, MacOSX): Windows 11

Additional context
#3777
#3834
#4138

@Maya-Painter

@microsoft-github-policy-service microsoft-github-policy-service bot added the customer-reported Issue created by a customer label Feb 1, 2024
@onionhammer onionhammer changed the title Custom serializer support for LINQ (#4138) breaks ExtensionData LINQ support #3834 Custom serializer support for LINQ (#4138) should support STJ ExtensionData Feb 1, 2024
@Maya-Painter Maya-Painter self-assigned this Feb 1, 2024
@Maya-Painter
Copy link
Contributor

Hi @onionhammer, can you try handling extension data in your implementation of SerializeMemberName in your custom STJ serializer?

e.g.

  public override string SerializeMemberName(MemberInfo memberInfo)
  {
      JsonPropertyNameAttribute jsonPropertyNameAttribute = memberInfo.GetCustomAttribute<JsonPropertyNameAttribute>(true);

      string memberName = jsonPropertyNameAttribute != null && !string.IsNullOrEmpty(jsonPropertyNameAttribute.Name)
          ? jsonPropertyNameAttribute.Name
          : memberInfo.Name;

      // Handle ExtensionData members
      System.Text.Json.Serialization.JsonExtensionDataAttribute jsonExtensionDataAttribute = 
          memberInfo.GetCustomAttribute<System.Text.Json.Serialization.JsonExtensionDataAttribute>(true);
      if(jsonExtensionDataAttribute != null)
      {
          return null;
      }

      return memberName;
  }

@onionhammer
Copy link
Contributor Author

onionhammer commented Feb 1, 2024

@Maya-Painter Good idea, that does workaround this issue.

Can the base method be decorated as nullable?

e.g.

    public abstract string? SerializeMemberName(MemberInfo memberInfo);

@Maya-Painter
Copy link
Contributor

Since the string type is inherently nullable, I don't know if this adds any value.

I will add the ExtensionData handling code to the sample custom serializers for others to use as reference, however. Thanks for pointing this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants