PR For Bulk Operation and ODataBind Review for Beta Release by Sreejithpin · Pull Request #2524 · OData/WebApi

Sreejithpin · 2021-07-08T18:23:27Z

Issues

PR For Bulk Operation and ODataBind Review for Beta Release
This pull request fixes issue #xxx.

Description

Briefly describe the changes of this pull request.

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.

* Bulk operations6 (#2) * BulkInsert1 draft * Bulk insert changes * deleted old test * updates * Update ODataResourceSetWrapper.cs * updates * Updated addressing comments * updates * updates updates * Updates * Updates * Minor updates * comments * updates * update publicapi for core * Address comments * Cleanup and additional tests Cleanup and additional tests * Updated code * BulkInsert1 draft * Bulk insert changes * deleted old test * updates * updates * Updated addressing comments * updates * updates updates * Updates * Updates * Minor updates * updates * Address comments * Cleanup and additional tests Cleanup and additional tests * DataAnnotationException updates * comments * small updates * updates * small update * updates * Updates * Update DeltaSetOfT.cs * Updates with Patch * updates * updates * Update WebHostTestFixture.cs * updates * Update DeltaOfTStructuralType.cs * Updates * Updates for serializer etc * Update WebHostTestFixture.cs * updates * updates * updates * Bulk Operations Updates * Review comments addressed * Updates * updated for deltaentity * Update Microsoft.AspNet.OData.Test.csproj * Update Microsoft.AspNet.OData.PublicApi.bsl * update public api * test fix * Update EdmStructuredObject.cs * Update EdmStructuredObject.cs * Update BulkInsertTest.cs * update * public api * review comments * Review comments updates * smalll update * updates * updates * updates

* support for odata.bind * support for odata.bind

xuzhg · 2021-07-14T19:47:12Z

src/Microsoft.AspNet.OData.Shared/Routing/Conventions/EntitySetRoutingConvention.cs

                }
+                else if (ODataRequestMethod.Patch == controllerContext.Request.Method)
+                {
+                    // e.g. Try PatchCustomer first, then fall back to Patch action name


PatchCustomer

PatchCustomers #Resolved

xuzhg · 2021-07-14T19:48:05Z

                    "PostFrom" + entityType.Name);

do we support patch from cast?

PATCH ~/Customers/NS.VipCustomer

Refers to: src/Microsoft.AspNet.OData.Shared/Routing/Conventions/EntitySetRoutingConvention.cs:83 in fbe081a. [](commit_id = fbe081a, deletion_comment = False)

xuzhg · 2021-07-14T19:49:27Z

src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerHelper.cs

+using Org.OData.Core.V1;
+using System;
+using System.Collections.Generic;
+using System.Text;


sort #Resolved

xuzhg · 2021-07-14T19:51:03Z

src/Microsoft.AspNet.OData.Shared/Formatter/Serialization/ODataSerializerHelper.cs

+    /// Helper class for OData Serialization
+    /// </summary>
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1053:StaticHolderTypesShouldNotHaveConstructors")]
+    public class ODataSerializerHelper


ODataSerializerHelper

if it's an internal class (I noticed no public API in this class), why don't mark the class as interal, and mark API as public.

In current design, we have a public class, but nothing in the public class from customer perspetive. #Resolved

xuzhg · 2021-07-14T19:56:08Z

src/Microsoft.AspNet.OData.Shared/Formatter/Deserialization/ODataResourceWrapper.cs

-        /// Gets the wrapped <see cref="ODataResource"/>.
+        /// Gets the wrapped <see cref="ODataResource"/>. This will return null for deleted resources.
        /// </summary>
+        [Obsolete("Please use ResourceBase instead")]


[Obsolete("Please use ResourceBase instead")]

This obsolete seems unnecessary.

The idea was to make it obselete but not to remove, right @mikepizzo . but I guess in 8.0 its removed right?

In 8.0 the Resource property should be typed as ODataResourceBase.

Sreejithpin · 2021-07-15T17:01:34Z

                    "PostFrom" + entityType.Name);

Normal Patch (Delta) supports inheritance/casting right?

In reply to: 880162857

Refers to: src/Microsoft.AspNet.OData.Shared/Routing/Conventions/EntitySetRoutingConvention.cs:83 in fbe081a. [](commit_id = fbe081a, deletion_comment = False)

mikepizzo · 2021-07-27T03:31:31Z

src/Microsoft.AspNet.OData.Shared/EdmPatchMethodHandler.cs

+    /// <summary>
+    /// Handler Class to handle users methods for create, delete and update
+    /// </summary>
+    public abstract class EdmPatchMethodHandler


EdmPatchMethodHandler

Since this is not just used for Patch, should we consider renaming to something like "UpdateHandler" or "DataModificationHandler"?

mikepizzo · 2021-07-27T03:34:43Z

src/Microsoft.AspNet.OData.Shared/PatchMethodHandler.cs

+        /// </summary>
+        NotFound
+    }
+


This should be in a separate file #Resolved

mikepizzo · 2021-07-27T03:36:39Z

src/Microsoft.AspNet.OData.Shared/PatchMethodHandler.cs

+    /// <summary>
+    /// Base Interface for PatchMethodHandler
+    /// </summary>
+    public interface IPatchMethodHandler


IPatchMethodHandler

I'm sure I asked this before, but why do we need an empty marker interface? Can you add to the comments so that it's clear to others in the future?

Also, this should be in a separate file so that it is easy to find. #Resolved

mikepizzo · 2021-07-27T03:39:02Z

src/Microsoft.AspNet.OData.Shared/DefaultPatchHandler.cs

+
+namespace Microsoft.AspNet.OData
+{    
+    internal class DefaultPatchHandler<TStructuralType> : PatchMethodHandler<TStructuralType> where TStructuralType :class


DefaultPatchHandler

Add comments describing this class, its behavior, and where/when it is appropriate to use. #Resolved

mikepizzo · 2021-07-27T03:40:24Z

src/Microsoft.AspNet.OData.Shared/EdmPatchMethodHandler.cs

+{    
+
+    /// <summary>
+    /// Handler Class to handle users methods for create, delete and update


Handler Class to handle users methods for create, delete and update

Add that this is for the case where there is no CLR type -- PatchMethodHandler is used when there is a clr type. #Resolved

mikepizzo · 2021-07-27T03:41:02Z

src/Microsoft.AspNet.OData.Shared/PatchMethodHandler.cs

+    }
+
+    /// <summary>
+    /// Handler Class to handle users methods for create, delete and update


Handler Class to handle users methods for create, delete and update

add to the description that this is for the clr-backed objects; EdmPatchMethodHandler is used for the non-CLR backed case. #Resolved

mikepizzo · 2021-07-27T03:50:06Z

src/Microsoft.AspNet.OData.Shared/IDeltaSet.cs

+namespace Microsoft.AspNet.OData
+{
+    /// <summary>
+    /// Basic interface to reperesent a deltaset which is a collection of Deltas.


Basic interface to reperesent a deltaset which is a collection of Deltas.

Please describe why we have this marker interface with no members. #Resolved

mikepizzo · 2021-07-27T04:02:23Z

src/Microsoft.AspNet.OData.Shared/DeltaSetOfT.cs

+        /// <returns>DeltaSet response</returns>
+        public DeltaSet<TStructuralType> Patch(IPatchMethodHandler patchHandler)
+        {            
+            return CopyChangedValues(patchHandler as PatchMethodHandler<TStructuralType>);


patchHandler as PatchMethodHandler

what happens if this cast fails? #Resolved

* Add [EnableNestedPaths] attribute that transforms result query based on path * Add NestedPathsRoutingConvention for .netcore * Make it possible to access action descriptor from action map * Fix bug in WebApiActionDescriptor.GetCustomAttributes() * Add tests for NestedPathsRoutingConvention * Add routing tests * Add tests for ODataPathQueryBuilder * Fix tests * Fix build errors is AspNet * Minor cleanup * Fix build error * Reuse existing ExpressionHelpers methods in ODataPathQueryBuilder * Minor code cleanup based on review comments * Remove var keyword * Fix some failing tests * Fix failing tests * Better support single results when [EnableQuery] is set and better support for EF Core * Add e2e tests * Add EF e2e tests * Refactoring * Update core 3.x public API * Minor refactors * Fix build errors in aspnet * Fix aspnet classic tests * Update projitems * Fix failing tests * Refactoring * Address PR comments and refactor * Conditionally-compile code only supported in net core * Minor clean up

Bumps System.Text.RegularExpressions from 4.3.0 to 4.3.1. --- updated-dependencies: - dependency-name: System.Text.RegularExpressions dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Co-authored-by: John Gathogo <jogathogo@microsoft.com>

* Bulk operations6 (#2) * BulkInsert1 draft * Bulk insert changes * deleted old test * updates * Update ODataResourceSetWrapper.cs * updates * Updated addressing comments * updates * updates updates * Updates * Updates * Minor updates * comments * updates * update publicapi for core * Address comments * Cleanup and additional tests Cleanup and additional tests * Updated code * BulkInsert1 draft * Bulk insert changes * deleted old test * updates * updates * Updated addressing comments * updates * updates updates * Updates * Updates * Minor updates * updates * Address comments * Cleanup and additional tests Cleanup and additional tests * DataAnnotationException updates * comments * small updates * updates * small update * updates * Updates * Update DeltaSetOfT.cs * Updates with Patch * updates * updates * Update WebHostTestFixture.cs * updates * Update DeltaOfTStructuralType.cs * Updates * Updates for serializer etc * Update WebHostTestFixture.cs * updates * updates * updates * Bulk Operations Updates * Review comments addressed * Updates * updated for deltaentity * Update Microsoft.AspNet.OData.Test.csproj * Update Microsoft.AspNet.OData.PublicApi.bsl * update public api * test fix * Update EdmStructuredObject.cs * Update EdmStructuredObject.cs * Update BulkInsertTest.cs * update * public api * review comments * Review comments updates * smalll update * updates * updates * updates

* support for odata.bind * support for odata.bind

…nto BulkOps_Odatabind

pull-request-quantifier-deprecated · 2021-09-07T05:06:51Z

This PR has 7513 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       : +6890 -623
Percentile : 100%

Total files changed: 1868

Change summary by file extension:
.md : +7 -3
.txt : +2 -1
.yml : +226 -0
.cs : +5975 -426
.csproj : +103 -43
.config : +118 -103
.resx : +18 -0
.projitems : +30 -0
.nuspec : +26 -18
.bsl : +381 -24
.proj : +0 -1
.targets : +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 detetcted.
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.

pull-request-quantifier-deprecated bot added the Extra Large label Jul 8, 2021

Sreejithpin requested a review from mikepizzo July 8, 2021 18:23

Sreejithpin and others added 3 commits July 8, 2021 11:29

Feature/new add support for odata bind (#2506)

a6c6763

* support for odata.bind * support for odata.bind

updates

6ff81ed

Sreejithpin force-pushed the BulkOps_Odatabind branch from 2561d7c to 6ff81ed Compare July 8, 2021 18:51

KenitoInc and others added 4 commits July 9, 2021 08:46

Enable constant parameterization in DateTime constants (#2515)

3a9f4bc

small update

fbe081a

Add support for dollar count segment in filter (#2521)

e03cd30

Fixed Enum try parse issue. This closes #2518 (#2525)

693edca

xuzhg reviewed Jul 14, 2021

View reviewed changes

Fix behaviour on status code requests. (#2512)

11600fe

mikepizzo reviewed Jul 27, 2021

View reviewed changes

Sreejithpin and others added 5 commits July 27, 2021 07:26

Code review comments

6acac50

Fix bug in Filter Any within Expand (#2530)

0e7a74f

dxrdxr and others added 28 commits August 25, 2021 11:12

Delta does not allow UpdatableProperties to be changed (#2483)

4699051

Update version to 7.5.10 (#2554)

67f82a6

Co-authored-by: John Gathogo <jogathogo@microsoft.com>

updates

4839a77

updates to use only factory

c0485de

updates

b82cd97

Feature/new add support for odata bind (#2506)

8e5b66a

* support for odata.bind * support for odata.bind

updates

a7afb1c

small update

44a3f24

Code review comments

83d7e49

update for odataid _temp

2771b28

updates

945f79d

updates to use only factory

768a789

updates

3b2c682

Update DeltaOfTStructuralType.cs

a648a4a

Merge branch 'BulkOps_Odatabind' of https://github.com/OData/WebApi i…

1c9a45a

…nto BulkOps_Odatabind

Update DeltaOfTStructuralType.cs

7402621

publicapi

3471989

test change

7f0eddc

Update Microsoft.AspNetCore.OData.Test.csproj

f183d8e

Update Microsoft.Test.E2E.AspNetCore3x.OData.csproj

49ba7a7

updates

40c5ca9

Update WebStack.versions.settings.targets

5ddec3e

Update WebStack.versions.settings.targets

1bfc01a

Update GetNugetPackageMetadata.proj

b2aa235

Update WebStack.versions.settings.targets

1e7fff9

For testing

2a1216a

update

8bcbd40

Sreejithpin mentioned this pull request Sep 9, 2021

PR For Bulk Operation and ODataBind Review for Beta Release #2567

Open

2 tasks

Conversation

Sreejithpin commented Jul 8, 2021

Issues

Description

Checklist (Uncheck if it is not completed)

Additional work necessary

Uh oh!

xuzhg Jul 14, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuzhg commented Jul 14, 2021

Uh oh!

xuzhg Jul 14, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuzhg Jul 14, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuzhg Jul 14, 2021

Choose a reason for hiding this comment

Uh oh!

Sreejithpin Jul 15, 2021

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

Sreejithpin commented Jul 15, 2021

Uh oh!

mikepizzo Jul 27, 2021

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jul 27, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jul 27, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jul 27, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jul 27, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jul 27, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jul 27, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikepizzo Jul 27, 2021 • edited by Sreejithpin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pull-request-quantifier-deprecated bot commented Sep 7, 2021

What can I do to optimize my changes

How to interpret the change counts in git diff output

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

xuzhg Jul 14, 2021 •

edited by Sreejithpin

Loading

xuzhg Jul 14, 2021 •

edited by Sreejithpin

Loading

xuzhg Jul 14, 2021 •

edited by Sreejithpin

Loading

mikepizzo Jul 27, 2021 •

edited by Sreejithpin

Loading

mikepizzo Jul 27, 2021 •

edited by Sreejithpin

Loading

mikepizzo Jul 27, 2021 •

edited by Sreejithpin

Loading

mikepizzo Jul 27, 2021 •

edited by Sreejithpin

Loading

mikepizzo Jul 27, 2021 •

edited by Sreejithpin

Loading

mikepizzo Jul 27, 2021 •

edited by Sreejithpin

Loading

mikepizzo Jul 27, 2021 •

edited by Sreejithpin

Loading