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

Resolve ContentID in odata.bind annotation #643

Merged

Conversation

gathogojr
Copy link
Contributor

Issues

This pull request fixes OData/odata.net#2442.

Description

Resolve ContentID in odata.bind annotation

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

@ElizabethOkerio
Copy link
Contributor

ElizabethOkerio commented Jul 25, 2022

@gathogojr is this valid "Parent@odata.bind": "$1" in a request body?. Should the writer write something like this?

Uri resolvedId = id;

string idOriginalString = id.OriginalString;
if (ContentIdReferencePattern.IsMatch(idOriginalString))
Copy link
Contributor

@ElizabethOkerio ElizabethOkerio Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this Regex Matching? Is it possible to use substring or indexOf to do the matching instead of Regex. I remember some discussions where this Regex was being discouraged because of performance reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ElizabethOkerio We use similar logic in ODataMessageWrapper class. We're looking for a very particular pattern here - a $ followed by an integer. I think using Substring and Index would result into more complex logic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On an unrelated note, let's avoid substring in favor Span<char>.Slice() where possible, to avoid string allocations.

xuzhg
xuzhg previously approved these changes Jul 25, 2022
@gathogojr
Copy link
Contributor Author

gathogojr commented Jul 27, 2022

@gathogojr is this valid "Parent@odata.bind": "$1" in a request body?. Should the writer write something like this?

@ElizabethOkerio Yes. Consider the sample payload below that the customer provided on the issue. You should be able to establish a parent-child relationship in the manner illustrated

{
  "requests": [
    {
      "id": "1",
      "method": "POST",
      "url": "http://localhost:5254/odata/Parents",
      "headers": {
        "OData-Version": "4.0",
        "Content-Type": "application/json;odata.metadata=minimal",
        "Accept": "application/json;odata.metadata=minimal"
      },
      "body": {}
    },
    {
      "id": "2",
      "method": "POST",
      "url": "http://localhost:5254/odata/Children",
      "headers": {
        "OData-Version": "4.0",
        "Content-Type": "application/json;odata.metadata=minimal",
        "Accept": "application/json;odata.metadata=minimal"
      },
      "body": {
        "Parent@odata.bind": "$1"
      }
    }
  ]
}

@gathogojr gathogojr merged commit 0feec0a into OData:main Jul 27, 2022
@gathogojr gathogojr deleted the fix/2442-resolve-ContentID-odatabind-annotation branch July 27, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing key when using reference URIs in Batch with $parameter
4 participants