Skip to content

525 warn on mismatch between app and schema versions#1196

Merged
waldekmastykarz merged 17 commits intodotnet:mainfrom
bartizan:525_warn-on-mismatch-between-app-and-schema-versions
Jun 2, 2025
Merged

525 warn on mismatch between app and schema versions#1196
waldekmastykarz merged 17 commits intodotnet:mainfrom
bartizan:525_warn-on-mismatch-between-app-and-schema-versions

Conversation

@bartizan
Copy link
Copy Markdown
Contributor

@bartizan bartizan commented May 20, 2025

@bartizan bartizan marked this pull request as ready for review May 20, 2025 11:12
@bartizan bartizan requested a review from a team as a code owner May 20, 2025 11:12
@waldekmastykarz waldekmastykarz linked an issue May 20, 2025 that may be closed by this pull request
Warn on incompatible schemas #525
Closed
@waldekmastykarz
Copy link
Copy Markdown
Collaborator

waldekmastykarz commented May 20, 2025

Thank you for the submission! We'll review it asap

@waldekmastykarz
Copy link
Copy Markdown
Collaborator

waldekmastykarz commented May 22, 2025

Hey @bartizan, could we also please check if this works with beta and doesn't give the same issue as #1201?

@waldekmastykarz
Copy link
Copy Markdown
Collaborator

waldekmastykarz commented May 22, 2025

PS: Seems I botched the merge conflict resolution. Sorry, do you want me to fix it @bartizan or are you looking at the code anyway for #1201? Sorry for the trouble.

@bartizan
Copy link
Copy Markdown
Contributor Author

bartizan commented May 22, 2025

PS: Seems I botched the merge conflict resolution. Sorry, do you want me to fix it @bartizan or are you looking at the code anyway for #1201? Sorry for the trouble.

let me fix it.
no worry.

@bartizan
Copy link
Copy Markdown
Contributor Author

bartizan commented May 22, 2025

could we also please check if this works with beta and doesn't give the same issue as #1201?

@waldekmastykarz , fixed. So schema version format is never a pre-release version.

@waldekmastykarz
Copy link
Copy Markdown
Collaborator

waldekmastykarz commented May 22, 2025

Cool! Thank you for such a quick turnaround. Will check asap. Preparing for a conference next week so might not be immediate.

@bartizan
Copy link
Copy Markdown
Contributor Author

bartizan commented May 23, 2025 •
edited
Loading

@dotnet-policy-service agree

@waldekmastykarz waldekmastykarz requested a review from Copilot May 26, 2025 17:35
Copilot AI reviewed May 26, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR centralizes semantic version comparison and adds schema version validation across proxy components to warn on mismatches between the application and JSON schema versions.

  • Moved and unified CompareSemVer and introduced NormalizeVersion in ProxyUtils.
  • Added ValidateSchemaVersion calls in loaders, plugin handlers, and notification logic.
  • Updated snippet and release version handling to use normalized versions.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
dev-proxy/UpdateNotification.cs Normalize product version and use centralized CompareSemVer
dev-proxy/PluginLoader.cs Extract $schema URL and validate its version
dev-proxy/CommandHandlers/ConfigNewCommandHandler.cs Use NormalizeVersion for snippet URL
dev-proxy-abstractions/ProxyUtils.cs Added ValidateSchemaVersion, relocated CompareSemVer, implemented NormalizeVersion
dev-proxy-abstractions/BaseProxyPlugin.cs Invoke ValidateSchemaVersion before JSON schema validation
dev-proxy-abstractions/BaseLoader.cs Extract schema element, call ValidateSchemaVersion, then validate JSON
Comments suppressed due to low confidence (1)

dev-proxy-abstractions/ProxyUtils.cs:522

  • There are no existing unit tests for the new ValidateSchemaVersion logic; consider adding tests for valid URLs, mismatched versions, invalid URLs, and empty inputs.
public static void ValidateSchemaVersion(string schemaUrl, ILogger logger)

Comment thread dev-proxy-abstractions/ProxyUtils.cs Outdated
Comment thread dev-proxy-abstractions/ProxyUtils.cs
Comment thread dev-proxy-abstractions/ProxyUtils.cs
Comment thread dev-proxy-abstractions/ProxyUtils.cs Outdated
@waldekmastykarz waldekmastykarz self-assigned this May 30, 2025
waldekmastykarz
waldekmastykarz requested changes May 30, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@waldekmastykarz waldekmastykarz left a comment

Choose a reason for hiding this comment

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

Nice job, and keen eye on the extra fixes. Let's update a few things and we'll get it in.

Comment thread dev-proxy-abstractions/ProxyUtils.cs Outdated
Comment thread dev-proxy/UpdateNotification.cs Outdated
@waldekmastykarz waldekmastykarz marked this pull request as draft May 30, 2025 10:00
@waldekmastykarz
Copy link
Copy Markdown
Collaborator

waldekmastykarz commented May 30, 2025

Marked the PR as draft so that we don't merge it accidentally

@bartizan bartizan marked this pull request as ready for review May 31, 2025 14:26
waldekmastykarz
waldekmastykarz approved these changes Jun 2, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@waldekmastykarz waldekmastykarz left a comment

Choose a reason for hiding this comment

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

Awesome! No further comments

@waldekmastykarz waldekmastykarz merged commit ef80955 into dotnet:main Jun 2, 2025
4 checks passed
@bartizan bartizan deleted the 525_warn-on-mismatch-between-app-and-schema-versions branch June 3, 2025 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@waldekmastykarz waldekmastykarz waldekmastykarz approved these changes

Assignees

@waldekmastykarz waldekmastykarz

Labels

breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Warn on incompatible schemas

3 participants

@bartizan @waldekmastykarz