Enable updating revision drafts through commands & leverage with Scale Rules

src/commands/revision/chooseRevisionMode/chooseRevisionMode.ts

@@ -19,7 +19,7 @@ export async function chooseRevisionMode(context: IActionContext, item?: Contain
     item ??= await pickContainerApp(context);
 
     let hasRevisionDraft: boolean | undefined;
-    if (item instanceof ContainerAppItem) {
+    if (ContainerAppItem.isContainerAppItem(item)) {
         // A revision draft can exist but may be identical to the source, distinguishing the difference in single revisions mode
         // improves the user experience by allowing us to skip the confirm step, silently discarding drafts instead
         hasRevisionDraft = item.hasUnsavedChanges();

src/commands/revisionDraft/README.md

@@ -0,0 +1,9 @@
+## Detecting Unsaved Changes
+
+When comparing items for changes in single revision mode, prefer referencing the `containerApp` template.  When comparing items in multiple revisions mode, prefer referencing the `revision` template.
+
+Reason:  Even though the `containerApp` template is essentially equivalent to the `latest` revision template... sometimes there are micro-differences present.  Although they end up being functionally equivalent, they may not always be equivalent enough to consistently pass a deep copy test (which is how we are set up to detect unsaved changes).
+
+## Data Sources for Tree Items
+
+Until the addition of revision drafts, the view has always reflected only one source of truth - the latest deployed changes.  With the addition of revision drafts, the view now prioritizes showing the latest draft `Unsaved changes` when they are present.  Model properties `containerApp` and `revision` should be kept consistent with the latest deployed changes so that methods like `hasUnsavedChanges` always have a reliable data reference for deep copy comparison.

src/commands/revisionDraft/RevisionDraftFileSystem.ts

@@ -3,15 +3,14 @@
 *  Licensed under the MIT License. See License.txt in the project root for license information.
 *--------------------------------------------------------------------------------------------*/
 
-import type { Template } from "@azure/arm-appcontainers";
+import { KnownActiveRevisionsMode, type Template } from "@azure/arm-appcontainers";
 import { nonNullValueAndProp } from "@microsoft/vscode-azext-utils";
 import { Disposable, Event, EventEmitter, FileChangeEvent, FileChangeType, FileStat, FileSystemProvider, FileType, TextDocument, Uri, window, workspace } from "vscode";
 import { URI } from "vscode-uri";
 import { ext } from "../../extensionVariables";
 import { ContainerAppItem } from "../../tree/ContainerAppItem";
 import type { ContainerAppsItem } from "../../tree/ContainerAppsBranchDataProvider";
-import type { RevisionDraftItem } from "../../tree/revisionManagement/RevisionDraftItem";
-import type { RevisionItem } from "../../tree/revisionManagement/RevisionItem";
+import type { RevisionsItemModel } from "../../tree/revisionManagement/RevisionItem";
 import { localize } from "../../utils/localize";
 
 const notSupported: string = localize('notSupported', 'This operation is not currently supported.');
@@ -50,23 +49,15 @@ export class RevisionDraftFileSystem implements FileSystemProvider {
         return this.emitter.event;
     }
 
-    // Create
-    createRevisionDraft(item: ContainerAppItem | RevisionItem | RevisionDraftItem): void {
+    createRevisionDraft(item: ContainerAppItem | RevisionsItemModel): void {
         const uri: Uri = this.buildUriFromItem(item);
         if (this.draftStore.has(uri.path)) {
             return;
         }
 
+        // Branching path reasoning: https://github.com/microsoft/vscode-azurecontainerapps/blob/main/src/commands/revisionDraft/README.md
         let file: RevisionDraftFile | undefined;
-        if (item instanceof ContainerAppItem) {
-            /**
-             * Sometimes there are micro-differences present between the latest revision template and the container app template.
-             * They end up being essentially equivalent, but not so equivalent as to always pass a deep copy test (which is how we detect unsaved changes).
-             *
-             * Since only container app template data is shown in single revision mode, and since revision data is not directly present on
-             * the container app tree item without further changes, it is easier to just use the container app template
-             * as the primary source of truth when in single revision mode.
-             */
+        if (ContainerAppItem.isContainerAppItem(item) || item.containerApp.revisionsMode === KnownActiveRevisionsMode.Single) {
             const revisionContent: Uint8Array = Buffer.from(JSON.stringify(nonNullValueAndProp(item.containerApp, 'template'), undefined, 4));
             file = new RevisionDraftFile(revisionContent, item.containerApp.id, nonNullValueAndProp(item.containerApp, 'latestRevisionName'));
         } else {
@@ -78,8 +69,7 @@ export class RevisionDraftFileSystem implements FileSystemProvider {
         this.fireSoon({ type: FileChangeType.Created, uri });
     }
 
-    // Read
-    parseRevisionDraft<T extends ContainerAppsItem>(item: T): Template | undefined {
+    parseRevisionDraft(item: ContainerAppsItem): Template | undefined {
         const uri: URI = this.buildUriFromItem(item);
         if (!this.draftStore.has(uri.path)) {
             return undefined;
@@ -93,12 +83,12 @@ export class RevisionDraftFileSystem implements FileSystemProvider {
         return contents ? Buffer.from(contents) : Buffer.from('');
     }
 
-    doesContainerAppsItemHaveRevisionDraft<T extends ContainerAppsItem>(item: T): boolean {
+    doesContainerAppsItemHaveRevisionDraft(item: ContainerAppsItem): boolean {
         const uri: Uri = this.buildUriFromItem(item);
         return this.draftStore.has(uri.path);
     }
 
-    getRevisionDraftFile<T extends ContainerAppsItem>(item: T): RevisionDraftFile | undefined {
+    getRevisionDraftFile(item: ContainerAppsItem): RevisionDraftFile | undefined {
         const uri: Uri = this.buildUriFromItem(item);
         return this.draftStore.get(uri.path);
     }
@@ -118,8 +108,7 @@ export class RevisionDraftFileSystem implements FileSystemProvider {
         }
     }
 
-    // Update
-    async editRevisionDraft(item: ContainerAppItem | RevisionItem | RevisionDraftItem): Promise<void> {
+    async editRevisionDraft(item: ContainerAppItem | RevisionsItemModel): Promise<void> {
         const uri: Uri = this.buildUriFromItem(item);
         if (!this.draftStore.has(uri.path)) {
             this.createRevisionDraft(item);
@@ -146,8 +135,17 @@ export class RevisionDraftFileSystem implements FileSystemProvider {
         ext.state.notifyChildrenChanged(file.containerAppId);
     }
 
-    // Delete
-    discardRevisionDraft<T extends ContainerAppsItem>(item: T): void {
+    updateRevisionDraftWithTemplate(item: RevisionsItemModel, template: Template): void {
+        const uri: Uri = this.buildUriFromItem(item);
+        if (!this.draftStore.has(uri.path)) {
+            this.createRevisionDraft(item);
+        }
+
+        const newContent: Uint8Array = Buffer.from(JSON.stringify(template, undefined, 4));
+        this.writeFile(uri, newContent);
+    }
+
+    discardRevisionDraft(item: ContainerAppsItem): void {
         const uri: Uri = this.buildUriFromItem(item);
         if (!this.draftStore.has(uri.path)) {
             return;
@@ -161,8 +159,7 @@ export class RevisionDraftFileSystem implements FileSystemProvider {
         this.fireSoon({ type: FileChangeType.Deleted, uri });
     }
 
-    // Helper
-    private buildUriFromItem<T extends ContainerAppsItem>(item: T): Uri {
+    private buildUriFromItem(item: ContainerAppsItem): Uri {
         return URI.parse(`${RevisionDraftFileSystem.scheme}:/${item.containerApp.name}.json`);
     }
 

src/commands/revisionDraft/RevisionDraftUpdateBaseStep.ts

@@ -0,0 +1,41 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import type { Template } from "@azure/arm-appcontainers";
+import { AzureWizardExecuteStep, nonNullValueAndProp } from "@microsoft/vscode-azext-utils";
+import type { Progress } from "vscode";
+import { ext } from "../../extensionVariables";
+import type { RevisionsItemModel } from "../../tree/revisionManagement/RevisionItem";
+import type { IContainerAppContext } from "../IContainerAppContext";
+
+export abstract class RevisionDraftUpdateBaseStep<T extends IContainerAppContext> extends AzureWizardExecuteStep<T> {
+    /**
+     * This property holds the active template revisions used for updating the revision draft
+     */
+    protected revisionDraftTemplate: Template;
+
+    constructor(readonly baseItem: RevisionsItemModel) {
+        super();
+        this.revisionDraftTemplate = this.initRevisionDraftTemplate();
+    }
+
+    abstract execute(context: T, progress: Progress<{ message?: string | undefined; increment?: number | undefined }>): Promise<void>;
+    abstract shouldExecute(context: T): boolean;
+
+    /**
+     * Call this method to upload `revisionDraftTemplate` changes to the container app revision draft
+     */
+    protected updateRevisionDraftWithTemplate(): void {
+        ext.revisionDraftFileSystem.updateRevisionDraftWithTemplate(this.baseItem, this.revisionDraftTemplate);
+    }
+
+    private initRevisionDraftTemplate(): Template {
+        let template: Template | undefined = ext.revisionDraftFileSystem.parseRevisionDraft(this.baseItem);
+        if (!template) {
+            template = nonNullValueAndProp(this.baseItem.revision, 'template');
+        }
+        return template;
+    }
+}

src/commands/scaling/addScaleRule/AddScaleRuleStep.ts

@@ -3,55 +3,54 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import type { ScaleRule, Template } from "@azure/arm-appcontainers";
-import { AzureWizardExecuteStep, nonNullProp } from "@microsoft/vscode-azext-utils";
-import { Progress, window } from "vscode";
+import { type ScaleRule } from "@azure/arm-appcontainers";
+import { nonNullProp } from "@microsoft/vscode-azext-utils";
 import { ScaleRuleTypes } from "../../../constants";
 import { ext } from "../../../extensionVariables";
+import type { RevisionsItemModel } from "../../../tree/revisionManagement/RevisionItem";
 import { localize } from "../../../utils/localize";
-import { updateContainerApp } from "../../../utils/updateContainerApp";
+import { getParentResource } from "../../../utils/revisionDraftUtils";
+import { RevisionDraftUpdateBaseStep } from "../../revisionDraft/RevisionDraftUpdateBaseStep";
 import type { IAddScaleRuleContext } from "./IAddScaleRuleContext";
 
-export class AddScaleRuleStep extends AzureWizardExecuteStep<IAddScaleRuleContext> {
-    public priority: number = 100;
+export class AddScaleRuleStep<T extends IAddScaleRuleContext> extends RevisionDraftUpdateBaseStep<T> {
+    public priority: number = 200;
 
-    public async execute(context: IAddScaleRuleContext, _progress: Progress<{ message?: string | undefined; increment?: number | undefined }>): Promise<void> {
-        const adding = localize('addingScaleRule', 'Adding {0} rule "{1}" to "{2}"...', context.ruleType, context.ruleName, context.containerApp.name);
-        const added = localize('addedScaleRule', 'Successfully added {0} rule "{1}" to "{2}".', context.ruleType, context.ruleName, context.containerApp.name);
+    constructor(baseItem: RevisionsItemModel) {
+        super(baseItem);
+    }
 
-        const template: Template = context.containerApp.template || {};
-        template.scale = context.scale || {};
-        template.scale.rules = context.scaleRules || [];
+    public async execute(context: IAddScaleRuleContext): Promise<void> {
+        this.revisionDraftTemplate.scale ||= {};
+        this.revisionDraftTemplate.scale.rules ||= [];
 
-        const scaleRule: ScaleRule = this.buildRule(context);
-        this.integrateRule(context, template.scale.rules, scaleRule);
+        context.scaleRule = this.buildRule(context);
+        this.integrateRule(context, this.revisionDraftTemplate.scale.rules, context.scaleRule);
+        this.updateRevisionDraftWithTemplate();
 
-        ext.outputChannel.appendLog(adding);
-        await updateContainerApp(context, context.subscription, context.containerApp, { template });
-        context.scaleRule = scaleRule;
-        void window.showInformationMessage(added);
-        ext.outputChannel.appendLog(added);
+        const resourceName = getParentResource(context.containerApp, this.baseItem.revision).name;
+        ext.outputChannel.appendLog(localize('addedScaleRule', 'Added {0} rule "{1}" to "{2}" (draft)', context.newRuleType, context.newRuleName, resourceName));
     }
 
     public shouldExecute(context: IAddScaleRuleContext): boolean {
-        return context.ruleName !== undefined && context.ruleType !== undefined;
+        return !!context.newRuleName && !!context.newRuleType;
     }
 
     private buildRule(context: IAddScaleRuleContext): ScaleRule {
-        const scaleRule: ScaleRule = { name: context.ruleName };
-        switch (context.ruleType) {
+        const scaleRule: ScaleRule = { name: context.newRuleName };
+        switch (context.newRuleType) {
             case ScaleRuleTypes.HTTP:
                 scaleRule.http = {
                     metadata: {
-                        concurrentRequests: nonNullProp(context, 'concurrentRequests')
+                        concurrentRequests: nonNullProp(context, 'newHttpConcurrentRequests')
                     }
                 };
                 break;
             case ScaleRuleTypes.Queue:
                 scaleRule.azureQueue = {
-                    queueName: context.queueName,
-                    queueLength: context.queueLength,
-                    auth: [{ secretRef: context.secretRef, triggerParameter: context.triggerParameter }]
+                    queueName: context.newQueueName,
+                    queueLength: context.newQueueLength,
+                    auth: [{ secretRef: context.newQueueSecretRef, triggerParameter: context.newQueueTriggerParameter }]
                 }
                 break;
             default:
@@ -60,12 +59,12 @@ export class AddScaleRuleStep extends AzureWizardExecuteStep<IAddScaleRuleContex
     }
 
     private integrateRule(context: IAddScaleRuleContext, scaleRules: ScaleRule[], scaleRule: ScaleRule): void {
-        switch (context.ruleType) {
+        switch (context.newRuleType) {
             case ScaleRuleTypes.HTTP:
                 // Portal only allows one HTTP rule per revision
                 const idx: number = scaleRules.findIndex((rule) => rule.http);
                 if (idx !== -1) {
-                    scaleRules.splice(idx, 0);
+                    scaleRules.splice(idx, 1);
                 }
                 break;
             case ScaleRuleTypes.Queue:

src/commands/scaling/addScaleRule/IAddScaleRuleContext.ts

@@ -3,29 +3,32 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import type { Scale, ScaleRule } from "@azure/arm-appcontainers";
+import { ScaleRule } from "@azure/arm-appcontainers";
+import type { ExecuteActivityContext } from "@microsoft/vscode-azext-utils";
 import type { ContainerAppModel } from "../../../tree/ContainerAppItem";
 import type { IContainerAppContext } from "../../IContainerAppContext";
 
-export interface IAddScaleRuleContext extends IContainerAppContext {
+export interface IAddScaleRuleContext extends IContainerAppContext, ExecuteActivityContext {
     // Make containerApp _required_
     containerApp: ContainerAppModel;
 
-    scale: Scale;
-    scaleRules: ScaleRule[];
+    /**
+     * The name of the parent resource (`ContainerAppModel | Revision`)
+     */
+    parentResourceName: string;
 
     // Base Rule Properties
-    ruleName?: string;
-    ruleType?: string;
+    newRuleName?: string;
+    newRuleType?: string;
 
     // HTTP Rule Properties
-    concurrentRequests?: string;
+    newHttpConcurrentRequests?: string;
 
     // Queue Rule Properties
-    queueName?: string;
-    queueLength?: number;
-    secretRef?: string;
-    triggerParameter?: string;
+    newQueueName?: string;
+    newQueueLength?: number;
+    newQueueSecretRef?: string;
+    newQueueTriggerParameter?: string;
 
     scaleRule?: ScaleRule;
 }