-
Notifications
You must be signed in to change notification settings - Fork 468
Reduce string allocations by comparing issuers in-place #2597
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
Merged
Merged
Changes from 11 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
294b614
compare issuers without string allocations
kllysng dc24e70
add changelog details for this bug
kllysng ee637d4
comment update
kllysng e5ff68a
use spans
kllysng bd9bf1a
Merge branch 'dev' into kellysong/string-replace
kllysng 61fe257
fix concat error
kllysng a47bc59
Merge branch 'kellysong/string-replace' of https://github.com/AzureAD…
kllysng d91c75f
change request
kllysng a585032
Delay creation of issuer until necessary + remove unecessary indexOf …
kllysng c04a235
Add unit tests for new methods
kllysng 52da7cd
Forgot to push - update internal method
kllysng 27d0708
Change requests
kllysng 825d528
Delay calling indexOf
kllysng b84ad93
change requests
kllysng cd85053
Merge branch 'dev' of https://github.com/AzureAD/azure-activedirector…
kllysng 747dd39
Add additional test case for IsValidIssuer
kllysng File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
46 changes: 46 additions & 0 deletions
46
test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
| using System; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.IdentityModel.Validators.Tests | ||
| { | ||
| public class AadIssuerValidatorTests | ||
| { | ||
| [Fact] | ||
| public static void IssuersWithTemplatesAreEqualTests_EqualIssuers() | ||
| { | ||
| // arrange | ||
| var issuer1Template = "{tenantId}"; | ||
| var issuer1 = ValidatorConstants.AadInstance + issuer1Template; | ||
| var issuer2Template = ValidatorConstants.TenantIdAsGuid; | ||
| var issuer2 = ValidatorConstants.AadInstance + issuer2Template; | ||
| int templateStartIndex = issuer1.IndexOf(issuer1Template); | ||
|
|
||
| // act | ||
| var result = AadIssuerValidator.IssuersWithTemplatesAreEqual( | ||
| issuer1.AsSpan(), issuer1Template.AsSpan(), templateStartIndex, issuer2.AsSpan(), issuer2Template.AsSpan()); | ||
|
|
||
| // assert | ||
| Assert.True(result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public static void IssuersWithTemplatesAreEqualTests_UnequalIssuers() | ||
| { | ||
| // arrange | ||
| var issuer1Template = "{tenantId}"; | ||
| var issuer1 = ValidatorConstants.AadInstancePPE + issuer1Template; | ||
| var issuer2Template = ValidatorConstants.TenantIdAsGuid; | ||
| var issuer2 = ValidatorConstants.AadInstance + issuer2Template; | ||
| int templateStartIndex = issuer1.IndexOf(issuer1Template); | ||
|
|
||
| // act | ||
| var result = AadIssuerValidator.IssuersWithTemplatesAreEqual( | ||
| issuer1.AsSpan(), issuer1Template.AsSpan(), templateStartIndex, issuer2.AsSpan(), issuer2Template.AsSpan()); | ||
|
|
||
| // assert | ||
| Assert.False(result); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.