Skip to content

Conversation

@sungam3r
Copy link
Member

@sungam3r sungam3r commented Nov 1, 2020

Rework of #62 . No breaking changes. Fixes #61 , slight style changes.

fixes #61
heap allocation optimizations
null argument checks
add comments
formatting
remove MVC from example
… cleanup

# Conflicts:
#	README.md
#	appveyor.yml
#	src/GraphQL.Authorization.Tests/AuthenticatedUserRequirementTests.cs
#	src/GraphQL.Authorization.Tests/AuthorizationEvaluatorTests.cs
#	src/GraphQL.Authorization.Tests/AuthorizationSchemaBuilderTests.cs
#	src/GraphQL.Authorization.Tests/AuthorizationSettingsTests.cs
#	src/GraphQL.Authorization.Tests/AuthorizationValidationRuleTests.cs
#	src/GraphQL.Authorization.Tests/ClaimAuthorizationRequirementTests.cs
#	src/GraphQL.Authorization.Tests/GraphQL.Authorization.Tests.csproj
#	src/GraphQL.Authorization.Tests/ValidationTestBase.cs
#	src/GraphQL.Authorization.sln
#	src/GraphQL.Authorization/AuthorizationEvaluator.cs
#	src/GraphQL.Authorization/AuthorizationMetadataExtensions.cs
#	src/GraphQL.Authorization/AuthorizationPolicyBuilder.cs
#	src/GraphQL.Authorization/AuthorizationValidationRule.cs
#	src/GraphQL.Authorization/GraphQL.Authorization.csproj
#	src/GraphQL.Authorization/Requirements/ClaimAuthorizationRequirement.cs
#	src/Harness/GraphQLAuthExtensions.cs
#	src/Harness/Harness.csproj
#	src/Harness/Program.cs
#	src/Harness/Properties/launchSettings.json
#	src/Harness/Startup.cs
@github-actions github-actions bot added the test Pull request that adds new or changes existing tests label Nov 1, 2020
@sungam3r sungam3r requested review from Shane32 and joemcbride and removed request for Shane32 and joemcbride November 1, 2020 18:22
@Shane32
Copy link
Member

Shane32 commented Nov 1, 2020

I have not used this repo @sungam3r

@sungam3r
Copy link
Member Author

sungam3r commented Nov 1, 2020

Don't I bother you with the review anymore?

@sungam3r sungam3r added code style Pull request that applies code style rules bugfix Pull request that fixes a bug labels Nov 1, 2020
@sungam3r sungam3r requested a review from joemcbride November 1, 2020 20:36
@Shane32
Copy link
Member

Shane32 commented Nov 1, 2020

I don’t mind if you tag me of course. But I don’t have the knowledge or time to give a comprehensive review. When I’m able, I’ll skim over the changes and give it the 👍 but I won’t really know how any of the changes affect users.

(Basically I only focus on changes to the main repo at present)

@sungam3r
Copy link
Member Author

sungam3r commented Nov 1, 2020

@joemcbride PR is ready for review, no breaking changes, one bugfix, many xml docs and some memory optimizations. I have reduced the number of changed files. After this PR is merged I will continue working on #62. There are backward incompatible changes. Maybe I'll make develop branch for version 4.0.0.

@sungam3r
Copy link
Member Author

sungam3r commented Nov 2, 2020

@joemcbride OK to merge?

Copy link
Member

@Shane32 Shane32 left a comment

Choose a reason for hiding this comment

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

I think the "optimizations" within AuthorizationPolicy should be reverted, as the performance should be better with the old code; and, this code is more complicated.

@sungam3r sungam3r merged commit 4821115 into master Nov 3, 2020
@sungam3r sungam3r deleted the small-cleanup branch November 3, 2020 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes a bug code style Pull request that applies code style rules performance test Pull request that adds new or changes existing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NullReferenceException in AuthorizationValidationRule when query/mutation/parameter name is invalid

4 participants