From 6e03a71d70b1151455e4deda07bc7f2c4b0af8ff Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Feb 2026 22:40:21 +0100 Subject: [PATCH] fix: Prevent revision deletion from removing entire API 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: 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. --- tools/code/publisher/Api.cs | 80 +++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/tools/code/publisher/Api.cs b/tools/code/publisher/Api.cs index 683ba240..ae221f6e 100644 --- a/tools/code/publisher/Api.cs +++ b/tools/code/publisher/Api.cs @@ -590,6 +590,7 @@ await getPublisherFiles() private static void ConfigureDeleteApi(IHostApplicationBuilder builder) { ConfigureFindApiInformationFileDto(builder); + ConfigureIsApiNameInSourceControl(builder); ConfigureDeleteApiFromApim(builder); builder.Services.TryAddSingleton(GetDeleteApi); @@ -598,6 +599,7 @@ private static void ConfigureDeleteApi(IHostApplicationBuilder builder) private static DeleteApi GetDeleteApi(IServiceProvider provider) { var findDto = provider.GetRequiredService(); + var isNameInSourceControl = provider.GetRequiredService(); var deleteFromApim = provider.GetRequiredService(); var activitySource = provider.GetRequiredService(); var logger = provider.GetRequiredService(); @@ -623,36 +625,62 @@ await taskDictionary.GetOrAdd(name, async ValueTask deleteApiInner(ApiName name, CancellationToken cancellationToken) => await ApiName.TryParseRevisionedName(name) .Map(async api => await processRevisionedApi(api.RootName, api.RevisionNumber, cancellationToken)) - .IfLeft(async _ => await processRootApi(name, cancellationToken)); + .IfLeft(async _ => await deleteFromApim(name, cancellationToken)); - async ValueTask processRootApi(ApiName name, CancellationToken cancellationToken) => - await deleteFromApim(name, cancellationToken); + /// + /// Handles deletion of a revisioned API. Implements fix for issue #709. + /// + /// When deleting a revision, we need to: + /// 1. Check if the root API still exists in source control + /// 2. If not, delete the entire API (all revisions) + /// 3. If yes, only delete the specific revision if it's not the current one + /// + async ValueTask processRevisionedApi(ApiName rootName, ApiRevisionNumber revisionNumber, CancellationToken cancellationToken) + { + if (isNameInSourceControl(rootName) is false) + { + logger.LogInformation("Root API {RootApiName} not found in source control. Deleting entire API.", rootName); + await deleteFromApim(rootName, cancellationToken); + return; + } - async ValueTask processRevisionedApi(ApiName name, ApiRevisionNumber revisionNumber, CancellationToken cancellationToken) + await deleteRevisionIfNotCurrent(rootName, revisionNumber, cancellationToken); + } + + async ValueTask deleteRevisionIfNotCurrent(ApiName rootName, ApiRevisionNumber revisionNumber, CancellationToken cancellationToken) { - var rootName = ApiName.GetRootName(name); var currentRevisionNumberOption = await tryGetRevisionNumberInSourceControl(rootName, cancellationToken); - - await currentRevisionNumberOption.Match(// If the current revision in source control has a different revision number, delete this revision. - // We don't want to delete a revision if it was just made current. For instance: - // 1. Dev has apiA with revision 1 (current) and revision 2. Artifacts folder has: - // - /apis/apiA/apiInformation.json with revision 1 as current - // - /apis/apiA;rev=2/apiInformation.json - // 2. User makes revision 2 current in dev APIM. - // 3. User runs extractor for dev APIM. Artifacts folder has: - // - /apis/apiA/apiInformation.json with revision 2 as current - // - /apis/apiA;rev=1/apiInformation.json - // - /apis/apiA;rev=2 folder gets deleted. - // 4. User runs publisher to prod APIM. We don't want to handle the deletion of folder /apis/apiA;rev=2, as it's the current revision. - async currentRevisionNumber => - { - if (currentRevisionNumber != revisionNumber) - { - await deleteFromApim(name, cancellationToken); - } - }, - // If there is no current revision in source control, process the root API deletion - async () => await deleteApi(rootName, cancellationToken)); + var revisionedName = ApiName.GetRevisionedName(rootName, revisionNumber); + + // If the current revision in source control has a different revision number, delete this revision. + // We don't want to delete a revision if it was just made current. For instance: + // 1. Dev has apiA with revision 1 (current) and revision 2. Artifacts folder has: + // - /apis/apiA/apiInformation.json with revision 1 as current + // - /apis/apiA;rev=2/apiInformation.json + // 2. User makes revision 2 current in dev APIM. + // 3. User runs extractor for dev APIM. Artifacts folder has: + // - /apis/apiA/apiInformation.json with revision 2 as current + // - /apis/apiA;rev=1/apiInformation.json + // - /apis/apiA;rev=2 folder gets deleted. + // 4. User runs publisher to prod APIM. We don't want to handle the deletion of + // folder /apis/apiA;rev=2, as it's the current revision. + await currentRevisionNumberOption.Match( + async currentRevisionNumber => + { + if (currentRevisionNumber != revisionNumber) + { + logger.LogInformation("Deleting revision {RevisionNumber} of API {ApiName}...", revisionNumber, rootName); + await deleteFromApim(revisionedName, cancellationToken); + } + }, + // The root API exists but we couldn't read its revision number (file read issue). + // This is an edge case - see issue #709. Previously this would delete the entire API. + // Now we safely delete only the specific revision that was removed from source control. + async () => + { + logger.LogWarning("Root API {RootApiName} exists in source control but could not read current revision. Safely deleting only revision {RevisionNumber}.", rootName, revisionNumber); + await deleteFromApim(revisionedName, cancellationToken); + }); } async ValueTask> tryGetRevisionNumberInSourceControl(ApiName name, CancellationToken cancellationToken)