fix: Prevent revision deletion from removing entire API#833
Open
smithimage wants to merge 1 commit intoAzure:mainfrom
Open
fix: Prevent revision deletion from removing entire API#833smithimage wants to merge 1 commit intoAzure:mainfrom
smithimage wants to merge 1 commit intoAzure:mainfrom
Conversation
This fix addresses issue Azure#709 where deleting an API revision from source control would incorrectly delete the entire API (all revisions) instead of just the specific revision that was removed. Root Cause: When processing a deleted revision folder (e.g., apis/myApi;rev=4), the code would call tryGetRevisionNumberInSourceControl(rootName) to check if the root API still exists. This function uses TryGetFileContents to read the root API's apiInformation.json. However, TryGetFileContents can fail to read files from the git commit in certain scenarios (e.g., path resolution issues), returning None even when the file actually exists in the repository. When None was returned, the old code assumed the entire API had been deleted and would call deleteApi(rootName), which cascades to DeleteAllRevisions, removing the entire API including all its revisions. Fix: 1. Added IsApiNameInSourceControl check before the revision number lookup. This function uses GetArtifactFiles (Git.GetExistingFilesInCommit) which returns ALL files in the commit tree, making it more reliable for existence checks. 2. If the root API exists in source control (via isNameInSourceControl) but tryGetRevisionNumberInSourceControl returns None (file read issue), we now safely delete only the specific revision instead of the entire API. 3. If the root API does NOT exist in source control, we correctly delete the entire API as intended. Code Improvements: - Refactored processRevisionedApi method for better readability - Extracted deleteRevisionIfNotCurrent into a separate method - Removed processRootApi wrapper (directly call deleteFromApim) - Added comprehensive XML documentation comments - Improved inline comments for better clarity - Fixed log message to include revision number when deleting This ensures that revision deletions only affect the specific revision, while still allowing full API deletion when the entire API folder is removed.
Author
|
@microsoft-github-policy-service agree |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This fix addresses issue #709 where deleting an API revision from source control would incorrectly delete the entire API (all revisions) instead of just the specific revision that was removed.
Root Cause:
When processing a deleted revision folder (e.g., apis/myApi;rev=4), the code would call tryGetRevisionNumberInSourceControl(rootName) to check if the root API still exists. This function uses TryGetFileContents to read the root API's apiInformation.json. However, TryGetFileContents can fail to read files from the git commit in certain scenarios (e.g., path resolution issues), returning None even when the file actually exists in the repository.
When None was returned, the old code assumed the entire API had been deleted and would call deleteApi(rootName), which cascades to DeleteAllRevisions, removing the entire API including all its revisions.
Fix:
Added IsApiNameInSourceControl check before the revision number lookup. This function uses GetArtifactFiles (Git.GetExistingFilesInCommit) which returns ALL files in the commit tree, making it more reliable for existence checks.
If the root API exists in source control (via isNameInSourceControl) but tryGetRevisionNumberInSourceControl returns None (file read issue), we now safely delete only the specific revision instead of the entire API.
If the root API does NOT exist in source control, we correctly delete the entire API as intended.
Code Improvements:
This ensures that revision deletions only affect the specific revision, while still allowing full API deletion when the entire API folder is removed.