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

Change ODataResource.Properties property type to IEnumerable<ODataPropertyInfo> #2916

Merged

Conversation

gathogojr
Copy link
Contributor

@gathogojr gathogojr commented Apr 3, 2024

Description

ODataPropertyInfo was added as a base type for ODataProperty in order to have a common representation of the metadata about a property, including things like annotations.

Unfortunately, since ODataResource.Properties is a collection of ODataPropertys, it's not possible to read/write metadata (such as annotations) for properties that don't have values.

#2786 added the support for reading/writing annotations for missing properties (for example, an annotation saying why the property is missing). However, it requires reading and writing the property separately from the other properties of the resource.

This pull request makes ODataResource.Properties a collection of ODataPropertyInfo, enabling us to read/write this metadata even in cases where the property doesn't have a value.

Checklist (Uncheck if it is not completed)

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

@gathogojr gathogojr changed the title Change ODataResource Properties property type to IEnumerable<ODataPropertyInfo> Change ODataResource.Properties property type to IEnumerable<ODataPropertyInfo> Apr 3, 2024
@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from 6fb6905 to 743e665 Compare April 3, 2024 09:22

This PR has 2387 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Large
Size       : +1231 -1156
Percentile : 100%

Total files changed: 96

Change summary by file extension:
.cs : +1227 -1152
.txt : +4 -4

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

1 similar comment

This PR has 2387 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Large
Size       : +1231 -1156
Percentile : 100%

Total files changed: 96

Change summary by file extension:
.cs : +1227 -1152
.txt : +4 -4

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from 743e665 to bf15dd2 Compare April 3, 2024 11:13

This PR has 2375 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Large
Size       : +1225 -1150
Percentile : 100%

Total files changed: 96

Change summary by file extension:
.cs : +1221 -1146
.txt : +4 -4

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from bf15dd2 to c9a5c0b Compare April 3, 2024 13:22

This PR has 2383 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Large
Size       : +1226 -1157
Percentile : 100%

Total files changed: 99

Change summary by file extension:
.cs : +1220 -1151
.txt : +4 -4
.vb : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from c9a5c0b to b656371 Compare April 4, 2024 04:52

This PR has 2382 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Large
Size       : +1225 -1157
Percentile : 100%

Total files changed: 99

Change summary by file extension:
.cs : +1219 -1151
.txt : +4 -4
.vb : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from b656371 to 11e6d77 Compare April 5, 2024 07:03

This PR has 2382 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Large
Size       : +1225 -1157
Percentile : 100%

Total files changed: 99

Change summary by file extension:
.cs : +1219 -1151
.txt : +4 -4
.vb : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from 11e6d77 to 60dd302 Compare April 9, 2024 05:14

This PR has 2382 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Large
Size       : +1225 -1157
Percentile : 100%

Total files changed: 99

Change summary by file extension:
.cs : +1219 -1151
.txt : +4 -4
.vb : +2 -2

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from 60dd302 to ece042f Compare April 17, 2024 06:04
@@ -248,9 +250,10 @@ private TElement ParseAggregateSingletonResult<TElement>(QueryResult queryResult
{
case ODataReaderState.ResourceEnd:
entry = reader.Item as ODataResource;
if (entry != null && entry.Properties.Any())
IEnumerable<ODataProperty> properties = entry.Properties.OfType<ODataProperty>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can properties ever be null ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wachugamaina Not in this particular case

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from ece042f to 6e0045b Compare May 14, 2024 13:17
@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from 6e0045b to 1e7070f Compare May 29, 2024 08:29
@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from 1e7070f to 892db72 Compare June 5, 2024 09:01
@@ -248,9 +250,10 @@ private TElement ParseAggregateSingletonResult<TElement>(QueryResult queryResult
{
case ODataReaderState.ResourceEnd:
entry = reader.Item as ODataResource;
if (entry != null && entry.Properties.Any())
IEnumerable<ODataProperty> properties = entry.Properties.OfType<ODataProperty>();
Copy link
Contributor

@habbes habbes Jun 7, 2024

Choose a reason for hiding this comment

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

I wonder whether all the casting (and potential filtering) that's now introduced in this PR comes with a performance cost. Is that something we can evaluate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@habbes Obviously there's a cost associated with casting and we can't avoid it altogether. I'm working on getting some stats for the more common scenarios and will share on this pull request.

Copy link
Contributor Author

@gathogojr gathogojr Jun 10, 2024

Choose a reason for hiding this comment

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

@habbes Here are some stats from two runs each from dev-8.x branch and against this branch..

dev-8.x branch

Run Method Mean Error StdDev
1 WriteResourceAsync 778.4 us 25.89 us 73.45 us
1 WriteResource 243.0 us 6.47 us 18.46 us
1 ReadResourceAsync 1,295.7 us 63.32 us 184.71 us
1 ReadResource 480.3 us 9.24 us 23.51 us
2 WriteResourceAsync 852.3 us 26.40 us 76.16 us
2 WriteResource 252.0 us 7.01 us 20.56 us
2 ReadResourceAsync 1,303.4 us 55.14 us 160.84 us
2 ReadResource 500.7 us 14.91 us 43.96 us

this branch

Run Method Mean Error StdDev
1 WriteResourceAsync 1,061.6 us 35.36 us 102.57 us
1 WriteResource 222.0 us 9.20 us 26.26 us
1 ReadResourceAsync 1,193.5 us 29.11 us 82.57 us
1 ReadResource 435.3 us 8.54 us 14.27 us
2 WriteResourceAsync 867.9 us 42.17 us 120.32 us
2 WriteResource 192.5 us 3.82 us 6.89 us
2 ReadResourceAsync 856.3 us 20.60 us 57.77 us
2 ReadResource 359.5 us 7.04 us 10.76 us

It's not possible to conclude on the basis of these results whether there's a significant drop in performance due to change the ODataResource.Properties property from IEnumerable<ODataProperty> to IEnumerable<ODataPropertyInfo>.

I'll add the benchmarks I used to the performance tests project

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting jumps between runs. Hard to reach a conclusion. You could share tests I also try to run. We can also profile some simple scenarios and see what proportion of CPU (if any) goes to the casting and filtering in the common cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@habbes Here's code for the benchmarks that I ran against dev-8.x and odl-8/change-odata-resource-properties-type branches"

    public class Benchmarks
    {
        private const string payload = "{\"@odata.context\":\"https://tempuri.org/$metadata#SuperEntitySet/$entity\",\"Id\":1,\"BooleanProperty\":true,\"Int32Property\":13,\"SingleProperty\":3.142,\"Int16Property\":7,\"Int64Property\":6078747774547,\"DoubleProperty\":3.14159265359,\"DecimalProperty\":7654321,\"GuidProperty\":\"00000017-003b-003b-0001-020304050607\",\"DateTimeOffsetProperty\":\"1970-12-31T23:59:59Z\",\"TimeSpanProperty\":\"PT23H59M59S\",\"ByteProperty\":1,\"SignedByteProperty\":9,\"StringProperty\":\"foo\",\"ByteArrayProperty\":\"AQIDBAUGBwgJAA==\",\"DateProperty\":\"1970-01-01\",\"TimeOfDayProperty\":\"23:59:59.0000000\",\"ColorProperty\":\"Black\",\"GeographyPointProperty\":{\"type\":\"Point\",\"coordinates\":[22.2,22.2],\"crs\":{\"type\":\"name\",\"properties\":{\"name\":\"EPSG:4326\"}}},\"GeometryPointProperty\":{\"type\":\"Point\",\"coordinates\":[7.0,13.0],\"crs\":{\"type\":\"name\",\"properties\":{\"name\":\"EPSG:0\"}}},\"BooleanCollectionProperty\":[true,false],\"Int32CollectionProperty\":[13,31],\"SingleCollectionProperty\":[3.142,241.3],\"Int16CollectionProperty\":[7,11],\"Int64CollectionProperty\":[6078747774547,7454777478706],\"DoubleCollectionProperty\":[3.14159265359,95356295141.3],\"DecimalCollectionProperty\":[7654321,1234567],\"GuidCollectionProperty\":[\"00000017-003b-003b-0001-020304050607\",\"0000000b-001d-001d-0706-050403020100\"],\"DateTimeOffsetCollectionProperty\":[\"1970-12-31T23:59:59Z\",\"1858-11-17T11:29:29Z\"],\"TimeSpanCollectionProperty\":[\"PT23H59M59S\",\"PT11H29M29S\"],\"ByteCollectionProperty\":[1,9],\"SignedByteCollectionProperty\":[9,1],\"StringCollectionProperty\":[\"foo\",\"bar\"],\"ByteArrayCollectionProperty\":[\"AQIDBAUGBwgJAA==\",\"AAkIBwYFBAMCAQ==\"],\"DateCollectionProperty\":[\"1970-12-31\",\"1858-11-17\"],\"TimeOfDayCollectionProperty\":[\"23:59:59.0000000\",\"11:29:29.0000000\"],\"ColorCollectionProperty\":[\"Black\",\"White\"],\"GeographyPointCollectionProperty\":[{\"type\":\"Point\",\"coordinates\":[22.2,22.2],\"crs\":{\"type\":\"name\",\"properties\":{\"name\":\"EPSG:4326\"}}},{\"type\":\"Point\",\"coordinates\":[11.6,11.9],\"crs\":{\"type\":\"name\",\"properties\":{\"name\":\"EPSG:4326\"}}}],\"GeometryPointCollectionProperty\":[{\"type\":\"Point\",\"coordinates\":[7.0,13.0],\"crs\":{\"type\":\"name\",\"properties\":{\"name\":\"EPSG:0\"}}},{\"type\":\"Point\",\"coordinates\":[13.0,7.0],\"crs\":{\"type\":\"name\",\"properties\":{\"name\":\"EPSG:0\"}}}],\"DynamicPrimitiveProperty\":3.14159265359,\"DynamicSpatialProperty@odata.type\":\"#GeographyPoint\",\"DynamicSpatialProperty\":{\"type\":\"Point\",\"coordinates\":[11.1,11.1],\"crs\":{\"type\":\"name\",\"properties\":{\"name\":\"EPSG:4326\"}}},\"DynamicNullProperty\":null,\"DynamicStringValueProperty\":\"The quick brown fox jumps over the lazy dog\",\"DynamicBinaryValueProperty@odata.mediaEditLink\":\"https://tempuri.org/SuperEntitySet(1)/DynamicBinaryValueProperty\",\"DynamicBinaryValueProperty@odata.mediaReadLink\":\"https://tempuri.org/SuperEntitySet(1)/DynamicBinaryValueProperty\",\"DynamicBinaryValueProperty@odata.mediaContentType\":\"text/plain\",\"DynamicBinaryValueProperty\":\"AQIDBAUGBwgJAA==\",\"CoordinateProperty\":{\"Longitude\":47.64229583688,\"Latitude\":-122.13694393057},\"CustomerProperty@odata.navigationLink\":\"https://tempuri.org/SuperEntitySet(1)/CustomerProperty\",\"CustomerProperty\":{\"@odata.id\":\"https://tempuri.org/Customers(1)\",\"@odata.mediaEditLink\":\"https://tempuri.org/Customers(1)/Photo\",\"@odata.mediaContentType\":\"image/png\",\"Id\":1,\"Name\":\"Sue\"},\"CoordinateCollectionProperty@odata.type\":\"#Collection(NS.Customer)\",\"CoordinateCollectionProperty\":[{\"Longitude\":-1.25873495895,\"Latitude\":36.80558172342},{\"Longitude\":-0.0478076474524531,\"Latitude\":37.28654620793966}],\"CustomerCollectionProperty@odata.navigationLink\":\"https://tempuri.org/SuperEntitySet(1)/CustomerCollectionProperty\",\"CustomerCollectionProperty\":[{\"@odata.id\":\"https://tempuri.org/Customers(2)\",\"@odata.mediaEditLink\":\"https://tempuri.org/Customers(2)/Photo\",\"@odata.mediaContentType\":\"image/png\",\"Id\":2,\"Name\":\"Bob\"},{\"@odata.id\":\"https://tempuri.org/Customers(3)\",\"@odata.mediaEditLink\":\"https://tempuri.org/Customers(3)/Photo\",\"@odata.mediaContentType\":\"image/png\",\"Id\":3,\"Name\":\"Luc\"}],\"DynamicCoordinateProperty@odata.navigationLink\":\"https://tempuri.org/SuperEntitySet(1)/DynamicCoordinateProperty\",\"DynamicCoordinateProperty\":{\"@odata.type\":\"#NS.Coordinate\",\"Longitude\":-1.2884892023695735,\"Latitude\":36.823102995328014},\"DynamicCoordinateCollectionProperty@odata.navigationLink\":\"https://tempuri.org/SuperEntitySet(1)/DynamicCoordinateCollectionProperty\",\"DynamicCoordinateCollectionProperty@odata.type\":\"#Collection(NS.Customer)\",\"DynamicCoordinateCollectionProperty\":[{\"@odata.type\":\"#NS.Coordinate\",\"Longitude\":40.689412075427086,\"Latitude\":-74.04452186122666},{\"@odata.type\":\"#NS.Coordinate\",\"Longitude\":48.85854653960931,\"Latitude\":2.2944812969090544}]}";
        private readonly ODataMessageWriterSettings messageWriterSettings;
        private readonly ODataMessageReaderSettings messageReaderSettings;

        private readonly EdmModel model;
        private readonly EdmEntityType superEntityType;
        private readonly EdmEntityType customerEntityType;
        private readonly EdmComplexType coordinateComplexType;
        private readonly EdmEnumType colorEnumType;
        private readonly EdmEntitySet superEntitySet;
        private readonly EdmEntitySet customerEntitySet;

        private ODataResource? superEntityResource;
        private ODataResource? customer1Resource;
        private ODataResource? customer2Resource;
        private ODataResource? customer3Resource;
        private ODataResource? coordinate1Resource;
        private ODataResource? coordinate2Resource;
        private ODataResource? coordinate3Resource;
        private ODataResource? coordinate4Resource;
        private ODataResource? coordinate5Resource;
        private ODataResource? coordinate6Resource;
        private ODataNestedResourceInfo? customerPropertyNestedResourceInfo;
        private ODataNestedResourceInfo? customerCollectionPropertyNestedResourceInfo;
        private ODataNestedResourceInfo? coordinatePropertyNestedResourceInfo;
        private ODataNestedResourceInfo? coordinateCollectionPropertyNestedResourceInfo;
        private ODataNestedResourceInfo? dynamicCoordinatePropertyNestedResourceInfo;
        private ODataNestedResourceInfo? dynamicCoordinateCollectionPropertyNestedResourceInfo;
        private ODataResourceSet? customerCollectionPropertyResourceSet;
        private ODataResourceSet? coordinateCollectionPropertyResourceSet;
        private ODataProperty? dynamicPrimitiveProperty;
        private ODataPropertyInfo? dynamicStringPropertyInfo;
        private ODataProperty? dynamicSpatialProperty;
        private ODataProperty? dynamicNullProperty;
        private ODataStreamPropertyInfo? dynamicStreamPropertyInfo;

        public Benchmarks()
        {
            this.messageWriterSettings = new ODataMessageWriterSettings
            {
                Version = ODataVersion.V4,
                EnableMessageStreamDisposal = false,
                BaseUri = new Uri("https://tempuri.org"),
                ODataUri = new ODataUri { ServiceRoot = new Uri("https://tempuri.org") },
                BufferSize = 16 * 1024
            };
            this.messageReaderSettings = new ODataMessageReaderSettings
            {
                EnableMessageStreamDisposal = true
            };

            #region Model Initialization
            this.model = new EdmModel();

            this.colorEnumType = new EdmEnumType("NS", "Color");
            this.coordinateComplexType = new EdmComplexType("NS", "Coordinate");
            this.customerEntityType = new EdmEntityType("NS", "Customer", baseType: null, isAbstract: false, isOpen: false, hasStream: true);
            this.superEntityType = new EdmEntityType("NS", "SuperEntity", baseType: null, isAbstract: false, isOpen: true);

            this.colorEnumType.AddMember("Black", new EdmEnumMemberValue(1));
            this.colorEnumType.AddMember("White", new EdmEnumMemberValue(2));
            this.model.AddElement(this.colorEnumType);

            this.coordinateComplexType.AddStructuralProperty("Longitude", EdmPrimitiveTypeKind.Double);
            this.coordinateComplexType.AddStructuralProperty("Latitude", EdmPrimitiveTypeKind.Double);
            this.model.AddElement(this.coordinateComplexType);

            var customerIdProperty = this.customerEntityType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32, isNullable: false);
            this.customerEntityType.AddKeys(customerIdProperty);
            this.customerEntityType.AddStructuralProperty("Name", EdmPrimitiveTypeKind.String, isNullable: false);
            this.customerEntityType.AddStructuralProperty("Photo", EdmPrimitiveTypeKind.Stream);

            var superEntityIdProperty = this.superEntityType.AddStructuralProperty("Id", EdmPrimitiveTypeKind.Int32, false);
            this.superEntityType.AddKeys(superEntityIdProperty);
            this.superEntityType.AddStructuralProperty("BooleanProperty", EdmPrimitiveTypeKind.Boolean, false);
            this.superEntityType.AddStructuralProperty("Int32Property", EdmPrimitiveTypeKind.Int32, false);
            this.superEntityType.AddStructuralProperty("SingleProperty", EdmPrimitiveTypeKind.Single, false);
            this.superEntityType.AddStructuralProperty("Int16Property", EdmPrimitiveTypeKind.Int16, false);
            this.superEntityType.AddStructuralProperty("Int64Property", EdmPrimitiveTypeKind.Int64, false);
            this.superEntityType.AddStructuralProperty("DecimalProperty", EdmPrimitiveTypeKind.Decimal, false);
            this.superEntityType.AddStructuralProperty("DoubleProperty", EdmPrimitiveTypeKind.Double, false);
            this.superEntityType.AddStructuralProperty("GuidProperty", EdmPrimitiveTypeKind.Guid, false);
            this.superEntityType.AddStructuralProperty("DateTimeOffsetProperty", EdmPrimitiveTypeKind.DateTimeOffset, false);
            this.superEntityType.AddStructuralProperty("TimeSpanProperty", EdmPrimitiveTypeKind.Duration, false);
            this.superEntityType.AddStructuralProperty("ByteProperty", EdmPrimitiveTypeKind.Byte, false);
            this.superEntityType.AddStructuralProperty("SignedByteProperty", EdmPrimitiveTypeKind.SByte, false);
            this.superEntityType.AddStructuralProperty("StringProperty", EdmPrimitiveTypeKind.String, false);
            this.superEntityType.AddStructuralProperty("ByteArrayProperty", EdmPrimitiveTypeKind.Binary, false);
            this.superEntityType.AddStructuralProperty("DateProperty", EdmPrimitiveTypeKind.Date, false);
            this.superEntityType.AddStructuralProperty("TimeOfDayProperty", EdmPrimitiveTypeKind.TimeOfDay, false);
            this.superEntityType.AddStructuralProperty("ColorProperty", new EdmEnumTypeReference(this.colorEnumType, false));
            this.superEntityType.AddStructuralProperty("CoordinateProperty", new EdmComplexTypeReference(this.coordinateComplexType, true));
            this.superEntityType.AddStructuralProperty("GeographyPointProperty", EdmPrimitiveTypeKind.GeographyPoint, false);
            this.superEntityType.AddStructuralProperty("GeometryPointProperty", EdmPrimitiveTypeKind.GeometryPoint, false);
            var entityNavProperty = this.superEntityType.AddUnidirectionalNavigation(
                new EdmNavigationPropertyInfo
                {
                    Name = "CustomerProperty",
                    Target = this.customerEntityType,
                    TargetMultiplicity = EdmMultiplicity.ZeroOrOne
                });
            this.superEntityType.AddStructuralProperty("BooleanCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetBoolean(false))));
            this.superEntityType.AddStructuralProperty("Int32CollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetInt32(false))));
            this.superEntityType.AddStructuralProperty("SingleCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetSingle(false))));
            this.superEntityType.AddStructuralProperty("Int16CollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetInt16(false))));
            this.superEntityType.AddStructuralProperty("Int64CollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetInt64(false))));
            this.superEntityType.AddStructuralProperty("DecimalCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetDecimal(false))));
            this.superEntityType.AddStructuralProperty("DoubleCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetDouble(false))));
            this.superEntityType.AddStructuralProperty("GuidCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetGuid(false))));
            this.superEntityType.AddStructuralProperty("DateTimeOffsetCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetDateTimeOffset(false))));
            this.superEntityType.AddStructuralProperty("TimeSpanCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetDuration(false))));
            this.superEntityType.AddStructuralProperty("ByteCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetByte(false))));
            this.superEntityType.AddStructuralProperty("SignedByteCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetSByte(false))));
            this.superEntityType.AddStructuralProperty("StringCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetString(false))));
            this.superEntityType.AddStructuralProperty("ByteArrayCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetBinary(false))));
            this.superEntityType.AddStructuralProperty("DateCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetDate(false))));
            this.superEntityType.AddStructuralProperty("TimeOfDayCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(EdmCoreModel.Instance.GetTimeOfDay(false))));
            this.superEntityType.AddStructuralProperty("ColorCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(new EdmEnumTypeReference(this.colorEnumType, false))));
            this.superEntityType.AddStructuralProperty("CoordinateCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(new EdmComplexTypeReference(this.coordinateComplexType, true))));
            this.superEntityType.AddStructuralProperty("GeographyPointCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(new EdmPrimitiveTypeReference(
                    EdmCoreModel.Instance.GetPrimitiveType(EdmPrimitiveTypeKind.GeographyPoint), false))));
            this.superEntityType.AddStructuralProperty("GeometryPointCollectionProperty",
                new EdmCollectionTypeReference(new EdmCollectionType(new EdmPrimitiveTypeReference(
                    EdmCoreModel.Instance.GetPrimitiveType(EdmPrimitiveTypeKind.GeometryPoint), false))));
            var entityCollectionNavProperty = this.superEntityType.AddUnidirectionalNavigation(
                new EdmNavigationPropertyInfo
                {
                    Name = "CustomerCollectionProperty",
                    Target = this.customerEntityType,
                    TargetMultiplicity = EdmMultiplicity.Many
                });
            this.model.AddElement(this.superEntityType);

            var entityContainer = new EdmEntityContainer("Default", "Container");
            this.model.AddElement(entityContainer);

            this.customerEntitySet = entityContainer.AddEntitySet("Customers", this.customerEntityType);
            this.superEntitySet = entityContainer.AddEntitySet("SuperEntitySet", this.superEntityType);

            this.superEntitySet.AddNavigationTarget(entityNavProperty, this.customerEntitySet);
            this.superEntitySet.AddNavigationTarget(entityCollectionNavProperty, this.customerEntitySet);
            #endregion Model Initialization
        }

        public void InitResources()
        {
            this.superEntityResource = new ODataResource
            {
                TypeName = "NS.SuperEntity",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty
                    {
                        Name = "Id",
                        Value = 1,
                    },
                    new ODataProperty { Name = "BooleanProperty", Value = true },
                    new ODataProperty { Name = "Int32Property", Value = 13 },
                    new ODataProperty { Name = "SingleProperty", Value = 3.142f },
                    new ODataProperty { Name = "Int16Property", Value = (short)7 },
                    new ODataProperty { Name = "Int64Property", Value = 6078747774547L },
                    new ODataProperty { Name = "DoubleProperty", Value = 3.14159265359d },
                    new ODataProperty { Name = "DecimalProperty", Value = 7654321m },
                    new ODataProperty { Name = "GuidProperty", Value = new Guid(23, 59, 59, new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 }) },
                    new ODataProperty { Name = "DateTimeOffsetProperty", Value = new DateTimeOffset(new DateTime(1970, 12, 31, 23, 59, 59, 0, DateTimeKind.Utc)) },
                    new ODataProperty { Name = "TimeSpanProperty", Value = new TimeSpan(23, 59, 59) },
                    new ODataProperty { Name = "ByteProperty", Value = (byte)1 },
                    new ODataProperty { Name = "SignedByteProperty", Value = (sbyte)9 },
                    new ODataProperty { Name = "StringProperty", Value = "foo" },
                    new ODataProperty { Name = "ByteArrayProperty", Value = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 } },
                    new ODataProperty { Name = "DateProperty", Value = new Date(1970, 1, 1) },
                    new ODataProperty { Name = "TimeOfDayProperty", Value = new TimeOfDay(23, 59, 59, 0) },
                    new ODataProperty { Name = "ColorProperty", Value = new ODataEnumValue("Black") },
                    new ODataProperty { Name = "GeographyPointProperty", Value = GeographyPoint.Create(22.2, 22.2) },
                    new ODataProperty { Name = "GeometryPointProperty", Value = GeometryPoint.Create(7, 13) },
                    new ODataProperty { Name = "BooleanCollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { true, false } } },
                    new ODataProperty { Name = "Int32CollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { 13, 31 } } },
                    new ODataProperty { Name = "SingleCollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { 3.142f, 241.3f } } },
                    new ODataProperty { Name = "Int16CollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { (short)7, (short)11 } } },
                    new ODataProperty { Name = "Int64CollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { 6078747774547L, 7454777478706L } } },
                    new ODataProperty { Name = "DoubleCollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { 3.14159265359d, 95356295141.3d } } },
                    new ODataProperty { Name = "DecimalCollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { 7654321m, 1234567m } } },
                    new ODataProperty { Name = "GuidCollectionProperty", Value = new ODataCollectionValue { Items = new List<object>
                    {
                        new Guid(23, 59, 59, new byte[] { 0, 1, 2, 3, 4, 5, 6, 7 }),
                        new Guid(11, 29, 29, new byte[] { 7, 6, 5, 4, 3, 2, 1, 0 }),
                    } } },
                    new ODataProperty { Name = "DateTimeOffsetCollectionProperty", Value = new ODataCollectionValue { Items = new List<object>
                    {
                        new DateTimeOffset(new DateTime(1970, 12, 31, 23, 59, 59, 0, DateTimeKind.Utc)),
                        new DateTimeOffset(new DateTime(1858, 11, 17, 11, 29, 29, 0, DateTimeKind.Utc))
                    } } },
                    new ODataProperty { Name = "TimeSpanCollectionProperty", Value = new ODataCollectionValue { Items = new List<object>
                    {
                        new TimeSpan(23, 59, 59),
                        new TimeSpan(11, 29, 29)
                    } } },
                    new ODataProperty { Name = "ByteCollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { (byte)1, (byte)9 } } },
                    new ODataProperty { Name = "SignedByteCollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { (sbyte)9, (sbyte)1 } } },
                    new ODataProperty { Name = "StringCollectionProperty", Value = new ODataCollectionValue { Items = new List<object> { "foo", "bar" } } },
                    new ODataProperty { Name = "ByteArrayCollectionProperty", Value = new ODataCollectionValue { Items = new List<object>
                    {
                        new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 },
                        new byte[] { 0, 9, 8, 7, 6, 5, 4, 3, 2, 1 }
                    } } },
                    new ODataProperty { Name = "DateCollectionProperty", Value = new ODataCollectionValue { Items = new List<object>
                    {
                        new Date(1970, 12, 31),
                        new Date(1858, 11, 17)
                    } } },
                    new ODataProperty { Name = "TimeOfDayCollectionProperty", Value = new ODataCollectionValue { Items = new List<object>
                    {
                        new TimeOfDay(23, 59, 59, 0),
                        new TimeOfDay(11, 29, 29, 0)
                    } } },
                    new ODataProperty { Name = "ColorCollectionProperty", Value = new ODataCollectionValue { TypeName = "NS.Color", Items = new List<object>
                    {
                        new ODataEnumValue("Black"),
                        new ODataEnumValue("White")
                    } } },
                    new ODataProperty { Name = "GeographyPointCollectionProperty", Value = new ODataCollectionValue { Items = new List<object>
                    {
                        GeographyPoint.Create(22.2, 22.2),
                        GeographyPoint.Create(11.9, 11.6)
                    } } },
                    new ODataProperty { Name = "GeometryPointCollectionProperty", Value = new ODataCollectionValue { Items = new List<object>
                    {
                        GeometryPoint.Create(7, 13),
                        GeometryPoint.Create(13, 7)
                    } } },
                }
            };

            this.customer1Resource = new ODataResource
            {
                Id = new Uri("https://tempuri.org/Customers(1)"),
                TypeName = "NS.Customer",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty
                    {
                        Name = "Id",
                        Value = 1,
                    },
                    new ODataProperty { Name = "Name", Value = "Sue" }
                },
                MediaResource = new ODataStreamReferenceValue
                {
                    EditLink = new Uri("https://tempuri.org/Customers(1)/Photo"),
                    ReadLink = new Uri("https://tempuri.org/Customers(1)/Photo"),
                    ContentType = "image/png"
                }
            };

            this.customer2Resource = new ODataResource
            {
                Id = new Uri("https://tempuri.org/Customers(2)"),
                TypeName = "NS.Customer",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty
                    {
                        Name = "Id",
                        Value = 2,
                    },
                    new ODataProperty { Name = "Name", Value = "Bob" }
                },
                MediaResource = new ODataStreamReferenceValue
                {
                    EditLink = new Uri("https://tempuri.org/Customers(2)/Photo"),
                    ReadLink = new Uri("https://tempuri.org/Customers(2)/Photo"),
                    ContentType = "image/png"
                }
            };

            this.customer3Resource = new ODataResource
            {
                Id = new Uri("https://tempuri.org/Customers(3)"),
                TypeName = "NS.Customer",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty
                    {
                        Name = "Id",
                        Value = 3,
                    },
                    new ODataProperty { Name = "Name", Value = "Luc" }
                },
                MediaResource = new ODataStreamReferenceValue
                {
                    EditLink = new Uri("https://tempuri.org/Customers(3)/Photo"),
                    ReadLink = new Uri("https://tempuri.org/Customers(3)/Photo"),
                    ContentType = "image/png"
                }
            };

            this.coordinate1Resource = new ODataResource
            {
                TypeName = "NS.Coordinate",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty { Name = "Longitude", Value = 47.64229583688d },
                    new ODataProperty { Name = "Latitude", Value = -122.13694393057 }
                }
            };

            this.coordinate2Resource = new ODataResource
            {
                TypeName = "NS.Coordinate",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty { Name = "Longitude", Value = -1.25873495895d },
                    new ODataProperty { Name = "Latitude", Value = 36.80558172342d }
                }
            };

            this.coordinate3Resource = new ODataResource
            {
                TypeName = "NS.Coordinate",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty { Name = "Longitude", Value = -0.0478076474524531 },
                    new ODataProperty { Name = "Latitude", Value = 37.28654620793966d }
                }
            };

            this.coordinate4Resource = new ODataResource
            {
                TypeName = "NS.Coordinate",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty { Name = "Longitude", Value = -1.2884892023695735d },
                    new ODataProperty { Name = "Latitude", Value = 36.823102995328014d }
                }
            };

            this.coordinate5Resource = new ODataResource
            {
                TypeName = "NS.Coordinate",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty { Name = "Longitude", Value = 40.689412075427086d },
                    new ODataProperty { Name = "Latitude", Value = -74.04452186122666d }
                }
            };

            this.coordinate6Resource = new ODataResource
            {
                TypeName = "NS.Coordinate",
                Properties = new List<ODataPropertyInfo>
                {
                    new ODataProperty { Name = "Longitude", Value = 48.85854653960931d },
                    new ODataProperty { Name = "Latitude", Value = 2.2944812969090544d }
                }
            };

            this.customerPropertyNestedResourceInfo = new ODataNestedResourceInfo
            {
                Name = "CustomerProperty",
                Url = new Uri("https://tempuri.org/SuperEntitySet(1)/CustomerProperty"),
                IsCollection = false
            };

            this.customerCollectionPropertyNestedResourceInfo = new ODataNestedResourceInfo
            {
                Name = "CustomerCollectionProperty",
                Url = new Uri("https://tempuri.org/SuperEntitySet(1)/CustomerCollectionProperty"),
                IsCollection = true
            };

            this.coordinatePropertyNestedResourceInfo = new ODataNestedResourceInfo
            {
                Name = "CoordinateProperty",
                Url = new Uri("https://tempuri.org/SuperEntitySet(1)/CoordinateProperty"),
                IsCollection = false
            };

            this.coordinateCollectionPropertyNestedResourceInfo = new ODataNestedResourceInfo
            {
                Name = "CoordinateCollectionProperty",
                Url = new Uri("https://tempuri.org/SuperEntitySet(1)/CoordinateCollectionProperty"),
                IsCollection = true
            };

            this.dynamicCoordinatePropertyNestedResourceInfo = new ODataNestedResourceInfo
            {
                Name = "DynamicCoordinateProperty",
                Url = new Uri("https://tempuri.org/SuperEntitySet(1)/DynamicCoordinateProperty"),
                IsCollection = false
            };

            this.dynamicCoordinateCollectionPropertyNestedResourceInfo = new ODataNestedResourceInfo
            {
                Name = "DynamicCoordinateCollectionProperty",
                Url = new Uri("https://tempuri.org/SuperEntitySet(1)/DynamicCoordinateCollectionProperty"),
                IsCollection = true
            };

            this.customerCollectionPropertyResourceSet = new ODataResourceSet
            {
                TypeName = "Collection(NS.Customer)"
            };

            this.coordinateCollectionPropertyResourceSet = new ODataResourceSet
            {
                TypeName = "Collection(NS.Customer)"
            };

            this.dynamicPrimitiveProperty = new ODataProperty { Name = "DynamicPrimitiveProperty", Value = 3.14159265359d };
            this.dynamicStringPropertyInfo = new ODataPropertyInfo { Name = "DynamicStringValueProperty" };
            this.dynamicSpatialProperty = new ODataProperty { Name = "DynamicSpatialProperty", Value = GeographyPoint.Create(11.1, 11.1) };
            this.dynamicNullProperty = new ODataProperty { Name = "DynamicNullProperty", Value = null };
            this.dynamicStreamPropertyInfo = new ODataStreamPropertyInfo
            {
                Name = "DynamicBinaryValueProperty",
                EditLink = new Uri("https://tempuri.org/SuperEntitySet(1)/DynamicBinaryValueProperty"),
                ReadLink = new Uri("https://tempuri.org/SuperEntitySet(1)/DynamicBinaryValueProperty"),
                ContentType = "text/plain"
            };
        }

        [Benchmark]
        public async Task WriteResourceAsync()
        {
            for (int i = 0; i < 3; i++)
            {
                InitResources();

                using (var bufferedWriteStream = new BufferedStream(new MemoryStream(), this.messageWriterSettings.BufferSize))
                {
                    IODataResponseMessage asyncResponseMessage = new InMemoryMessage { Stream = bufferedWriteStream };

                    await using (var messageWriter = new ODataMessageWriter(asyncResponseMessage, this.messageWriterSettings))
                    {
                        var writer = await messageWriter.CreateODataResourceWriterAsync(this.superEntitySet, this.superEntityType).ConfigureAwait(false);

                        await writer.WriteStartAsync(superEntityResource).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.dynamicPrimitiveProperty).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.dynamicSpatialProperty).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.dynamicNullProperty).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.dynamicStringPropertyInfo).ConfigureAwait(false);
                        using (var textWriter = await writer.CreateTextWriterAsync().ConfigureAwait(false))
                        {
                            await textWriter.WriteAsync("The quick brown fox jumps over the lazy dog").ConfigureAwait(false);
                            await textWriter.FlushAsync().ConfigureAwait(false);
                        }

                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.dynamicStreamPropertyInfo).ConfigureAwait(false);
                        using (var stream = await writer.CreateBinaryWriteStreamAsync().ConfigureAwait(false))
                        {
                            var bytes = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 };

                            await stream.WriteAsync(bytes, 0, 4).ConfigureAwait(false);
                            await stream.WriteAsync(bytes, 4, 4).ConfigureAwait(false);
                            await stream.WriteAsync(bytes, 8, 2).ConfigureAwait(false);
                            await stream.FlushAsync().ConfigureAwait(false);
                        }

                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinatePropertyNestedResourceInfo).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinate1Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.customerPropertyNestedResourceInfo).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.customer1Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinateCollectionPropertyNestedResourceInfo).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinateCollectionPropertyResourceSet).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinate2Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinate3Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.customerCollectionPropertyNestedResourceInfo).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.customerCollectionPropertyResourceSet).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.customer2Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.customer3Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.dynamicCoordinatePropertyNestedResourceInfo).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinate4Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.dynamicCoordinateCollectionPropertyNestedResourceInfo).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinateCollectionPropertyResourceSet).ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinate5Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteStartAsync(this.coordinate6Resource).ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                        await writer.WriteEndAsync().ConfigureAwait(false);
                    }
                }
            }
        }

        [Benchmark]
        public void WriteResource()
        {
            InitResources();

            using (var writeStream = new MemoryStream())
            {
                IODataResponseMessage responseMessage = new InMemoryMessage { Stream = writeStream };

                using (var messageWriter = new ODataMessageWriter(responseMessage, this.messageWriterSettings))
                {
                    var writer = messageWriter.CreateODataResourceWriter(this.superEntitySet, this.superEntityType);

                    writer.WriteStart(superEntityResource);
                    writer.WriteStart(this.dynamicPrimitiveProperty);
                    writer.WriteEnd();
                    writer.WriteStart(this.dynamicSpatialProperty);
                    writer.WriteEnd();
                    writer.WriteStart(this.dynamicNullProperty);
                    writer.WriteEnd();
                    writer.WriteStart(this.dynamicStringPropertyInfo);
                    using (var textWriter = writer.CreateTextWriter())
                    {
                        textWriter.Write("The quick brown fox jumps over the lazy dog");
                        textWriter.Flush();
                    }

                    writer.WriteEnd();
                    writer.WriteStart(this.dynamicStreamPropertyInfo);
                    using (var stream = writer.CreateBinaryWriteStream())
                    {
                        var bytes = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 };

                        stream.Write(bytes, 0, 4);
                        stream.Write(bytes, 4, 4);
                        stream.Write(bytes, 8, 2);
                        stream.Flush();
                    }

                    writer.WriteEnd();
                    writer.WriteStart(this.coordinatePropertyNestedResourceInfo);
                    writer.WriteStart(this.coordinate1Resource);
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteStart(this.customerPropertyNestedResourceInfo);
                    writer.WriteStart(this.customer1Resource);
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteStart(this.coordinateCollectionPropertyNestedResourceInfo);
                    writer.WriteStart(this.coordinateCollectionPropertyResourceSet);
                    writer.WriteStart(this.coordinate2Resource);
                    writer.WriteEnd();
                    writer.WriteStart(this.coordinate3Resource);
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteStart(this.customerCollectionPropertyNestedResourceInfo);
                    writer.WriteStart(this.customerCollectionPropertyResourceSet);
                    writer.WriteStart(this.customer2Resource);
                    writer.WriteEnd();
                    writer.WriteStart(this.customer3Resource);
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteStart(this.dynamicCoordinatePropertyNestedResourceInfo);
                    writer.WriteStart(this.coordinate4Resource);
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteStart(this.dynamicCoordinateCollectionPropertyNestedResourceInfo);
                    writer.WriteStart(this.coordinateCollectionPropertyResourceSet);
                    writer.WriteStart(this.coordinate5Resource);
                    writer.WriteEnd();
                    writer.WriteStart(this.coordinate6Resource);
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteEnd();
                    writer.WriteEnd();
                }
            }
        }

        [Benchmark]
        public async Task ReadResourceAsync()
        {
            using (var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(payload)))
            {
                IODataResponseMessage asyncResponseMessage = new InMemoryMessage { Stream = memoryStream };
                asyncResponseMessage.SetHeader(ODataConstants.ContentTypeHeader, "application/json");

                using (var messageReader = new ODataMessageReader(asyncResponseMessage, this.messageReaderSettings, this.model))
                {
                    var reader = await messageReader.CreateODataResourceReaderAsync().ConfigureAwait(false);

                    while (await reader.ReadAsync().ConfigureAwait(false))
                    {
                        switch (reader.State)
                        {
                            case ODataReaderState.Stream:
                                var streamItem = reader.Item as ODataStreamItem;
                                if (streamItem == null)
                                {
                                    continue;
                                }

                                if (streamItem.PrimitiveTypeKind == EdmPrimitiveTypeKind.String)
                                {
                                    using (var textReader = await reader.CreateTextReaderAsync().ConfigureAwait(false))
                                    {
                                        await textReader.ReadToEndAsync().ConfigureAwait(false);
                                    }
                                }
                                else
                                {
                                    using (var readStream = await reader.CreateReadStreamAsync().ConfigureAwait(false))
                                    {
                                        var buffer = new byte[10];

                                        await readStream.ReadAsync(buffer, 0, 10);
                                    }
                                }

                                break;
                        }
                    }
                }
            }
        }

        [Benchmark]
        public void ReadResource()
        {
            using (var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(payload)))
            {
                IODataResponseMessage syncResponseMessage = new InMemoryMessage { Stream = memoryStream };
                syncResponseMessage.SetHeader(ODataConstants.ContentTypeHeader, "application/json");

                using (var messageReader = new ODataMessageReader(syncResponseMessage, this.messageReaderSettings, this.model))
                {
                    var reader = messageReader.CreateODataResourceReader();

                    while (reader.Read())
                    {
                        switch (reader.State)
                        {
                            case ODataReaderState.Stream:
                                var streamItem = reader.Item as ODataStreamItem;
                                if (streamItem == null)
                                {
                                    continue;
                                }

                                if (streamItem.PrimitiveTypeKind == EdmPrimitiveTypeKind.String)
                                {
                                    using (var textReader = reader.CreateTextReader())
                                    {
                                        textReader.ReadToEnd();
                                    }
                                }
                                else
                                {
                                    using (var readStream = reader.CreateReadStream())
                                    {
                                        var buffer = new byte[10];

                                        readStream.Read(buffer, 0, 10);
                                    }
                                }

                                break;
                        }
                    }
                }
            }
        }
    }

    public class InMemoryMessage : IODataRequestMessage, IODataResponseMessage, IODataRequestMessageAsync, IODataResponseMessageAsync, IDisposable
    {
        private readonly Dictionary<string, string> headers;

        public InMemoryMessage()
        {
            this.headers = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
        }

        public IEnumerable<KeyValuePair<string, string>> Headers
        {
            get { return this.headers; }
        }

        public int StatusCode { get; set; }

        public Uri Url { get; set; }

        public string Method { get; set; }

        public Stream Stream { get; set; }

        public IServiceProvider Container { get; set; }

        public string? GetHeader(string headerName)
        {
            if (headerName == null)
            {
                throw new ArgumentNullException(nameof(headerName));
            }

            return this.headers.TryGetValue(headerName, out string? headerValue) ? headerValue : null;
        }

        public void SetHeader(string headerName, string headerValue)
        {
            headers[headerName] = headerValue;
        }

        public Stream GetStream()
        {
            return this.Stream;
        }

        public Action DisposeAction { get; set; }

        public Task<Stream> GetStreamAsync()
        {
            TaskCompletionSource<Stream> taskCompletionSource = new TaskCompletionSource<Stream>();
            taskCompletionSource.SetResult(this.Stream);
            return taskCompletionSource.Task;
        }

        void IDisposable.Dispose()
        {
            if (this.DisposeAction != null)
            {
                this.DisposeAction();
            }
        }
    }

@@ -986,53 +986,54 @@ internal ODataJsonReaderNestedInfo ReadPropertyWithoutValue(IODataJsonReaderReso
ODataJsonReaderNestedInfo readerNestedInfo = null;
IEdmStructuredType resourceType = resourceState.ResourceType;
IEdmProperty edmProperty = this.ReaderValidator.ValidatePropertyDefined(propertyName, resourceType);
if (edmProperty != null && !edmProperty.Type.IsUntyped())
if (edmProperty == null || edmProperty.Type.IsUntyped())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why has this condition changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@habbes I meant to leave a comment to explain this change. If you look at ReadPropertyWithoutValueAsync method, we invert the condition expression to avoid the unnecessary if and else if. The logic in the if (edmProperty == null || edmProperty.Type.IsUntyped()) is what was previously in the else statement. The sync method now mirrors the async method and we get rid of unnecessary nesting

Copy link
Contributor

Choose a reason for hiding this comment

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

Had not noticed the inverted if-else blocks.

@habbes
Copy link
Contributor

habbes commented Jun 7, 2024

Requesting some clarifications on the description:

Unfortunately, since ODataResource.Properties is a collection of ODataPropertys, it's not possible to read/write metadata (such as annotations) for properties that don't have values.

I'm a bit confused by this statement. Is it the metadata (annotations) that don't have values, or properties that don't have values?

#2786 added the support for reading/writing annotations for missing properties (for example, an annotation saying why the property is missing). However, it requires reading and writing the property separately from the other properties of the resource.

And here when you say "it requires reading and writing the property separately from the other properties", does the property that's being written separately refer to the annotation, or the property that's missing?

@habbes
Copy link
Contributor

habbes commented Jun 7, 2024

After a first pass through the PR, I have a few questions and concerns:

This PR makes ODataResource.Properties a collection of ODataPropertyInfo, but it appears that in a lot of places we end of filtering and casting the collection for ODataProperty instances only (Properties.OfType<ODataProperty>()). If we do this in the happy path or common case, maybe it's an indication that ODataProperty and ODataPropertyInfo should belong together? I could be looking at things the wrong way I stand to be corrected.

Another concern is performance. It appears that now we'll pay the cost of casting and filtering in the common case even when the Properties collection doesn't contain other types of ODataPropertyInfo. I think we should evaluate the performance impact of this change.

If in the common case we have to remember to filter for ODataProperty, this introduces complexity to the user experience in my opinion and makes it less intuitive. What happens if a customer forgets to add that filter? It doesn't seem like the kind that kind of thing that they would naturally expect to have to do, until they run into an error or other issues.

@gathogojr
Copy link
Contributor Author

Requesting some clarifications on the description:

Unfortunately, since ODataResource.Properties is a collection of ODataPropertys, it's not possible to read/write metadata (such as annotations) for properties that don't have values.

I'm a bit confused by this statement. Is it the metadata (annotations) that don't have values, or properties that don't have values?

#2786 added the support for reading/writing annotations for missing properties (for example, an annotation saying why the property is missing). However, it requires reading and writing the property separately from the other properties of the resource.

And here when you say "it requires reading and writing the property separately from the other properties", does the property that's being written separately refer to the annotation, or the property that's missing?

Think of it more as a property without a value rather than a missing property.
Ordinarily, the way a property with value is represented in the payload is in the form "PropertyName": "PropertyValue".
For a property without a value, the way such a property would be represented is by means of a OData annotation, e.g., "PropertyName@odata.type":"#Edm.String" or even be my means of a custom annotation, e.g., "PropertyName@Is.Missing":true - where Is.Missing is a random name for a custom annotation.

The challenge we had before was that we could cleanly read or write such properties. Key reason being that ODataResource.Properties property type was IEnumerable<ODataProperty> and a value is required for an ODataProperty.

The ODataPropertyInfo type comes in handy in this scenario. If an ODataPropertyInfo is found in the Properties collection, the annotations (only) for such a property will be written to the payload. Conversely, if we're reading a payload and we come across a property annotation but no property value, we read it as an ODataPropertyInfo and stash it in the Properties collection

@gathogojr
Copy link
Contributor Author

After a first pass through the PR, I have a few questions and concerns:

This PR makes ODataResource.Properties a collection of ODataPropertyInfo, but it appears that in a lot of places we end of filtering and casting the collection for ODataProperty instances only (Properties.OfType<ODataProperty>()). If we do this in the happy path or common case, maybe it's an indication that ODataProperty and ODataPropertyInfo should belong together? I could be looking at things the wrong way I stand to be corrected.

Another concern is performance. It appears that now we'll pay the cost of casting and filtering in the common case even when the Properties collection doesn't contain other types of ODataPropertyInfo. I think we should evaluate the performance impact of this change.

If in the common case we have to remember to filter for ODataProperty, this introduces complexity to the user experience in my opinion and makes it less intuitive. What happens if a customer forgets to add that filter? It doesn't seem like the kind that kind of thing that they would naturally expect to have to do, until they run into an error or other issues.

It's not a lot of places actually. In the case of writing, the logic to a very large extent remains the same.
The OfType<ODataProperty>() is not blanketly applied. You don't filter out properties without values indiscriminately. But in scenarios where only properties with values are relevant, you could use of OfType<ODataProperty>() or do it in a more performant approach (e.g. in a hot path).
I was initially a bit concerned but a close scrutiny I realized that it's not that many places where apply the casting.
I'd be open to different ideas/suggestions on how differently we could implement this.

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch 2 times, most recently from 4b5f2a8 to 149daa8 Compare June 10, 2024 14:21
@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch 2 times, most recently from 3a9e707 to fb8cbd0 Compare June 12, 2024 09:26
{
return nonComputedProperties == null ? null : nonComputedProperties.Where(p =>
{
if (p.ODataValue is ODataStreamReferenceValue)
if (p is not ODataProperty property)
Copy link
Contributor

Choose a reason for hiding this comment

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

This method seems to filter out properties that are not of type ODataProperty. Should it return an IEnumerable<ODataPropertyInfo> or just return an IEnumerable<ODataProperty>?

@ElizabethOkerio
Copy link
Contributor

After a first pass through the PR, I have a few questions and concerns:

This PR makes ODataResource.Properties a collection of ODataPropertyInfo, but it appears that in a lot of places we end of filtering and casting the collection for ODataProperty instances only (Properties.OfType<ODataProperty>()). If we do this in the happy path or common case, maybe it's an indication that ODataProperty and ODataPropertyInfo should belong together? I could be looking at things the wrong way I stand to be corrected.

Another concern is performance. It appears that now we'll pay the cost of casting and filtering in the common case even when the Properties collection doesn't contain other types of ODataPropertyInfo. I think we should evaluate the performance impact of this change.

If in the common case we have to remember to filter for ODataProperty, this introduces complexity to the user experience in my opinion and makes it less intuitive. What happens if a customer forgets to add that filter? It doesn't seem like the kind that kind of thing that they would naturally expect to have to do, until they run into an error or other issues.

I tend to agree with this. Can whatever that's in ODataProperty be moved to ODataPropertyInfo and we do away with ODataProperty? What will be the implications?

@gathogojr
Copy link
Contributor Author

After a first pass through the PR, I have a few questions and concerns:
This PR makes ODataResource.Properties a collection of ODataPropertyInfo, but it appears that in a lot of places we end of filtering and casting the collection for ODataProperty instances only (Properties.OfType<ODataProperty>()). If we do this in the happy path or common case, maybe it's an indication that ODataProperty and ODataPropertyInfo should belong together? I could be looking at things the wrong way I stand to be corrected.
Another concern is performance. It appears that now we'll pay the cost of casting and filtering in the common case even when the Properties collection doesn't contain other types of ODataPropertyInfo. I think we should evaluate the performance impact of this change.
If in the common case we have to remember to filter for ODataProperty, this introduces complexity to the user experience in my opinion and makes it less intuitive. What happens if a customer forgets to add that filter? It doesn't seem like the kind that kind of thing that they would naturally expect to have to do, until they run into an error or other issues.

I tend to agree with this. Can whatever that's in ODataProperty be moved to ODataPropertyInfo and we do away with ODataProperty? What will be the implications?

Can whatever that's in ODataProperty be moved to ODataPropertyInfo and we do away with ODataProperty?
ODataPropertyInfo represents a property without value while ODataProperty represents a property with value.
If moving the properties in ODataProperty to ODataPropertyInfo was an option, we wouldn't need to make this change. In particular, ODataProperty has a Value property to serve it's purpose. The same concerns you and @habbes have are the same reason I hesitated merging this pull request and I did the presentation so everyone is aware about the implications of this change. I organized that presentation for the purposes of dissecting the same concerns being raised here. You can rewatch the recording.

@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from fb8cbd0 to 38a7854 Compare June 14, 2024 09:08
@habbes
Copy link
Contributor

habbes commented Jun 19, 2024

After running more tests, the evidence we have so far is that this will not have noticeable hit on performance for common case. I'd still prefer a design that doesn't require us to always filter out properties (multiple times) during serialization. But I understand the constraints of the features we want to support. If we can demonstrate will have minimal to no disruptions or require no changes from customers who don't need the additional feature, I'd be okay with this.

habbes
habbes previously approved these changes Jun 19, 2024
@gathogojr gathogojr force-pushed the odl-8/change-odata-resource-properties-type branch from 38a7854 to 53ca193 Compare June 20, 2024 06:24
@gathogojr gathogojr dismissed stale reviews from ElizabethOkerio and habbes via b189ecf June 21, 2024 06:05
@gathogojr gathogojr merged commit 8cb387e into OData:dev-8.x Jun 21, 2024
5 checks passed
@gathogojr gathogojr deleted the odl-8/change-odata-resource-properties-type branch June 21, 2024 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants