Fix issue on SQL: Delete a container with blob, then create container/blob with same name, and delete container will fail. (#2572)

* Fix 2563: on SQL, create container/blob, delete container, then create container again, then delete will fail.

* fix issue 2563: delete a contain with blob, then create container/blob and delete container fail on sql

Author: blueww

ChangeLog.md

@@ -11,6 +11,7 @@ Blob:
 - Fixed issue of filtering blobs with correct multiple conditions on single tag (range queries). (issue #2514)
 - Added support for sealing append blobs. (issue #810)
 - Added support for delegation sas with version of 2015-07-05.
+- Fix issue on SQL: Delete a container with blob, then create container/blob with same name, and delete container will fail. (issue #2563)
 
 Table:
 

src/blob/persistence/SqlBlobMetadataStore.ts

@@ -8,7 +8,8 @@ import {
   Options as SequelizeOptions,
   Sequelize,
   TEXT,
-  Transaction
+  Transaction,
+  WhereOptions
 } from "sequelize";
 
 import uuid from "uuid/v4";
@@ -646,34 +647,19 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
         transaction: t
       });
 
-      // TODO: GC blobs under deleting status
-      await BlobsModel.update(
-        {
-          deleting: literal("deleting + 1")
+      await this.deleteBlobFromSQL({
+          accountName: account,
+          containerName: container
         },
-        {
-          where: {
-            accountName: account,
-            containerName: container
-          },
-          transaction: t
-        }
+        t
       );
 
-      // TODO: GC blocks under deleting status
-      await BlocksModel.update(
-        {
-          deleting: literal("deleting + 1")
+      await this.deleteBlockFromSQL({
+          accountName: account,
+          containerName: container
         },
-        {
-          where: {
-            accountName: account,
-            containerName: container
-          },
-          transaction: t
-        }
+        t
       );
-      /* Transaction ends */
     });
   }
 
@@ -1509,6 +1495,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
           blobName: block.blobName,
           blockName: block.name,
           size: block.size,
+          deleting: 0,
           persistency: this.serializeModelValue(block.persistency)
         },
         { transaction: t }
@@ -1977,51 +1964,33 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
         if (count > 1) {
           throw StorageErrorFactory.getSnapshotsPresent(context.contextId!);
         } else {
-          await BlobsModel.update(
-            {
-              deleting: literal("deleting + 1")
+          await this.deleteBlobFromSQL({
+              accountName: account,
+              containerName: container,
+              blobName: blob
             },
-            {
-              where: {
-                accountName: account,
-                containerName: container,
-                blobName: blob
-              },
-              transaction: t
-            }
+            t
           );
 
-          await BlocksModel.update(
-            {
-              deleting: literal("deleting + 1")
+          await this.deleteBlockFromSQL({
+              accountName: account,
+              containerName: container,
+              blobName: blob
             },
-            {
-              where: {
-                accountName: account,
-                containerName: container,
-                blobName: blob
-              },
-              transaction: t
-            }
+            t
           );
         }
       }
 
       // Scenario: Delete one snapshot only
       if (!againstBaseBlob) {
-        await BlobsModel.update(
-          {
-            deleting: literal("deleting + 1")
-          },
-          {
-            where: {
+        await this.deleteBlobFromSQL({
               accountName: account,
               containerName: container,
               blobName: blob,
               snapshot: blobModel.snapshot
             },
-            transaction: t
-          }
+            t
         );
       }
 
@@ -2030,32 +1999,19 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
         againstBaseBlob &&
         options.deleteSnapshots === Models.DeleteSnapshotsOptionType.Include
       ) {
-        await BlobsModel.update(
-          {
-            deleting: literal("deleting + 1")
+        await this.deleteBlobFromSQL({
+            accountName: account,
+            containerName: container,
+            blobName: blob
           },
-          {
-            where: {
-              accountName: account,
-              containerName: container,
-              blobName: blob
-            },
-            transaction: t
-          }
+          t
         );
 
-        await BlocksModel.update(
-          {
-            deleting: literal("deleting + 1")
-          },
-          {
-            where: {
-              accountName: account,
-              containerName: container,
-              blobName: blob
-            },
-            transaction: t
-          }
+        await this.deleteBlockFromSQL({
+            accountName: account,
+            containerName: container,
+            blobName: blob
+          },t
         );
       }
 
@@ -2064,19 +2020,13 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
         againstBaseBlob &&
         options.deleteSnapshots === Models.DeleteSnapshotsOptionType.Only
       ) {
-        await BlobsModel.update(
-          {
-            deleting: literal("deleting + 1")
+        await this.deleteBlobFromSQL({
+            accountName: account,
+            containerName: container,
+            blobName: blob,
+            snapshot: { [Op.gt]: "" }
           },
-          {
-            where: {
-              accountName: account,
-              containerName: container,
-              blobName: blob,
-              snapshot: { [Op.gt]: "" }
-            },
-            transaction: t
-          }
+          t
         );
       }
     });
@@ -3545,6 +3495,62 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
       return Models.AccessTier.Cold;
     }
     return undefined;
+  }  
+
+  /**
+   * Delete blob from SQL database.
+   * For performance, we used to mark deleting+1, instead of really delete. But this take issue like #2563. So change to real delete.
+   *
+   * @private
+   * @param {WhereOptions<any>} where
+   * @param {Transaction} [t]
+   * @returns {Promise<void>}
+   * @memberof SqlBlobMetadataStore
+   */
+  private async deleteBlobFromSQL(where: WhereOptions<any>, t?: Transaction): Promise<void> {
+    await BlobsModel.destroy({
+      where,
+      transaction: t
+    });
+    
+    // // TODO: GC blobs under deleting status
+    // await BlobsModel.update(
+    //   {
+    //     deleting: literal("deleting + 1")
+    //   },
+    //   {
+    //     where,
+    //     transaction: t
+    //   }
+    // );
+  }
+
+    /**
+   * Delete block from SQL database.
+   * For performance, we used to mark deleting+1, instead of really delete. But this take issue like #2563. So change to real delete.
+   *
+   * @private
+   * @param {WhereOptions<any>} where
+   * @param {Transaction} [t]
+   * @returns {Promise<void>}
+   * @memberof SqlBlobMetadataStore
+   */
+  private async deleteBlockFromSQL(where: WhereOptions<any>, t?: Transaction): Promise<void> {
+     await BlocksModel.destroy({
+      where,
+      transaction: t
+    });
+
+    // TODO: GC blocks under deleting status
+    // await BlocksModel.update(
+    //   {
+    //     deleting: literal("deleting + 1")
+    //   },
+    //   {
+    //     where,
+    //     transaction: t
+    //   }
+    // );
   }
 
   /**

tests/blob/apis/container.test.ts

@@ -1675,4 +1675,41 @@ describe("ContainerAPIs", () => {
       await blob.delete();
     }
   });
+  
+  it("Delete a container with block blob, then create container/blob with same name, and delete container should success. @loki @sql", async function () {
+    //create container and block blob
+    const containerName = getUniqueName("container1");
+    const containerClient = serviceClient.getContainerClient(containerName);
+    await containerClient.create();
+
+    const blobName1 = getUniqueName("blobname1");
+    const blockBlobClient = containerClient.getBlockBlobClient(blobName1);    
+    const body = "HelloWorld";
+    await blockBlobClient.stageBlock(base64encode("1"), body, body.length);
+    await blockBlobClient.stageBlock(base64encode("2"), body, body.length);
+    await blockBlobClient.commitBlockList(
+      [base64encode("1"), base64encode("2")]
+    );
+
+    // delete container
+    await containerClient.delete();
+
+    assert.strictEqual(false, await containerClient.exists());
+    assert.strictEqual(false, await blockBlobClient.exists());
+
+    //recreate
+    await containerClient.create();    
+    
+    await blockBlobClient.stageBlock(base64encode("1"), body, body.length);
+    await blockBlobClient.stageBlock(base64encode("2"), body, body.length);
+    await blockBlobClient.commitBlockList(
+      [base64encode("1"), base64encode("2")]
+    );
+    
+    // delete container
+    await containerClient.delete();
+
+    assert.strictEqual(false, await containerClient.exists());
+    assert.strictEqual(false, await blockBlobClient.exists());
+  });
 });