Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix namespace collision for string and CollectionUtils method IsNullOrEmpty #2471

Merged
merged 6 commits into from
Nov 27, 2024

Conversation

abhishekkumams
Copy link
Contributor

@abhishekkumams abhishekkumams commented Nov 20, 2024

Why make this change?

What is this change?

  • MsSqlQueryBuilder class had a check in the Build method where it was using IsNullOrEmpty for Strings but it was refrencing CollectionUtils Class instead
  • Removed Collection.Utilities class from Cli and renamed it in the Core project to EnumerableUtilities to avoid any future namespace collision.
  • Also updated all the references of IsNullOrEmpty

How was this tested?

  • manual tests
  • current existing tests

Notes:
Work Around shared here: AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#1722

Copy link
Contributor

@aaronburtle aaronburtle left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this one, I will follow up with another PR that fixes any additional usages.

@aaronburtle
Copy link
Contributor

Actually, after searching the codebase there was only 1 more instance where string was being null checked wrong, so just added it in here, hope that's OK @abhishekkumams

@abhishekkumams
Copy link
Contributor Author

/azp run

@aaronburtle
Copy link
Contributor

/azp run

Copy link
Contributor

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Need to remove unnecessary CollectionUtils class

@abhishekkumams
Copy link
Contributor Author

abhishekkumams commented Nov 25, 2024

Need to remove unnecessary CollectionUtils class

It's still being used for the reason pointed out by Sean: #2471 (comment)

@abhishekkumams
Copy link
Contributor Author

/azp run

Copy link
Contributor

@sezal98 sezal98 left a comment

Choose a reason for hiding this comment

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

lgtm

@abhishekkumams abhishekkumams dismissed Aniruddh25’s stale review November 27, 2024 10:15

updated as suggested

@abhishekkumams abhishekkumams merged commit 26a694c into main Nov 27, 2024
7 checks passed
@abhishekkumams abhishekkumams deleted the dev/abhishekkuma/fix-namespace-collision branch November 27, 2024 10:16
abhishekkumams added a commit that referenced this pull request Nov 28, 2024
…OrEmpty` (#2471)

## Why make this change?

- Closes #2469 
- There is a namespace collision for the `IsNullOrEmpty` method in the
`MsSqlQueryBuilder` class between String and CollectionUtils class.

## What is this change?

- `MsSqlQueryBuilder` class had a check in the `Build` method where it
was using `IsNullOrEmpty` for Strings but it was refrencing
CollectionUtils Class instead
- Removed `Collection.Utilities` class from Cli and renamed it in the
Core project to `EnumerableUtilities` to avoid any future namespace
collision.
- Also updated all the references of `IsNullOrEmpty`

## How was this tested?

- [x] manual tests
- [x] current existing tests

Notes:
Work Around shared here:
AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#1722

---------

Co-authored-by: Aaron Burtle <aaronburtle@microsoft.com>
Co-authored-by: aaronburtle <93220300+aaronburtle@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Conflicting namespace for method isNullOrEmpty causing update mutation to break
6 participants