From 90b6fcda7604949dfef29b09d2252424aadf9fbd Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Wed, 12 Aug 2026 16:40:26 +0100 Subject: [PATCH 01/10] Add blob versioning support (Loki metadata store) Implements blob versioning for the LokiJS metadata store, opt in per storage account. Fixes #665. Versioning is an ARM management plane setting in Azure Storage (Microsoft.Storage/storageAccounts/blobServices/default#isVersioningEnabled), not part of the data plane REST API that Azurite emulates. Since Azurite has no management plane, the setting is supplied at start up through two new mutually exclusive options, --accountConfigFile and --accountConfig , rather than by inventing a data plane API for it. The account model lives in src/common so queue and table can reuse it, and is persisted in its own Loki collection. The wire contract needed no code generation changes: the existing swagger already defines the versionid query parameter on 15 operations and VersionId / IsCurrentVersion on BlobItemInternal, so no generated artifact is hand edited. Behaviour with versioning enabled for an account: - Put Blob, Put Block List and Copy Blob retain the overwritten content as a previous version and return x-ms-version-id - Get Blob and Get Blob Properties accept ?versionid=, and return x-ms-is-current-version only for the current version - List Blobs supports include=versions, ordering versions of a blob oldest first with the current version last - Delete Blob with ?versionid= removes a single version; deleting the current version retains previous versions rather than failing with SnapshotsPresent. Snapshots still block base blob deletion as before - A version is restored by copying it over the current version, so a ?versionid= qualified copy source is now resolved - ?snapshot= together with ?versionid= returns 400 InvalidQueryParameterValue, matching the real service error code Previous versions share the base blob's snapshot value in the blobs collection, so every lookup that means "the blob itself" now excludes them. Blobs written before versioning was enabled have no isCurrentVersion field, so those queries match isCurrentVersion !== false and continue to resolve unchanged. Switching versioning on or off against an existing workspace is rejected at start up: the persisted configuration is compared with the supplied one and merged when there is no conflict. The resolved configuration is written to the debug log to help diagnose issue reports. Not implemented, and documented as such rather than partially emulated: the SQL metadata store (configuring versioning with AZURITE_DB fails at start up, and a versionid request returns a not-implemented error instead of silently reading the current version), blob version SAS (sr=bv and the x permission), soft delete interactions, blob expiration, Get Block List and Get Page Ranges with a version ID (the storage swagger does not define versionid on either). Adds tests/blob/apis/blob.versioning.test.ts covering the enabled and disabled paths with List Blobs assertions alongside each version assertion, and tests/common/AccountModel.test.ts for configuration parsing and validation. Documents the feature in README.md and docs/designs/blob-versioning.md, and moves Blob Versions out of the unsupported list in the support matrix. Co-Authored-By: Claude --- ChangeLog.md | 4 + README.md | 81 ++- docs/designs/blob-versioning.md | 125 +++++ package.json | 16 + src/blob/BlobConfiguration.ts | 2 + src/blob/BlobEnvironment.ts | 17 + src/blob/BlobServer.ts | 4 +- src/blob/BlobServerFactory.ts | 11 + src/blob/IBlobEnvironment.ts | 3 + src/blob/handlers/BlobHandler.ts | 53 +- src/blob/handlers/BlockBlobHandler.ts | 8 +- src/blob/handlers/ContainerHandler.ts | 14 +- src/blob/persistence/IBlobMetadataStore.ts | 15 +- src/blob/persistence/LokiBlobMetadataStore.ts | 511 ++++++++++++++++-- src/blob/persistence/SqlBlobMetadataStore.ts | 41 +- src/blob/utils/utils.ts | 31 ++ src/common/AccountModel.ts | 183 +++++++ src/common/Environment.ts | 17 + src/common/EnvironmentFunctions.ts | 67 +++ src/common/VSCEnvironment.ts | 11 + tests/BlobTestServerFactory.ts | 24 +- tests/blob/apis/blob.versioning.test.ts | 398 ++++++++++++++ tests/common/AccountModel.test.ts | 162 ++++++ 23 files changed, 1714 insertions(+), 84 deletions(-) create mode 100644 docs/designs/blob-versioning.md create mode 100644 src/common/AccountModel.ts create mode 100644 src/common/EnvironmentFunctions.ts create mode 100644 tests/blob/apis/blob.versioning.test.ts create mode 100644 tests/common/AccountModel.test.ts diff --git a/ChangeLog.md b/ChangeLog.md index 19dc485a5..cf67fc82f 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -4,6 +4,10 @@ ## Upcoming Release +Blob: + +- Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options rather than through a data plane API. With versioning enabled for an account: overwriting a blob retains the previous content as a version and returns `x-ms-version-id`; `Get Blob` / `Get Blob Properties` accept `?versionid=` and return `x-ms-is-current-version` for the current version; `List Blobs` supports `include=versions`; `Delete Blob` with `?versionid=` removes a single version, and deleting the current version retains previous versions instead of failing with `SnapshotsPresent`; a version can be restored by copying it over the current version. Combining `?snapshot=` with `?versionid=` returns 400 `InvalidQueryParameterValue`. The setting is persisted with the workspace and cannot be changed against existing data. Versioning is not implemented for the SQL metadata store, and blob version SAS (`sr=bv`, the `x` permission), soft delete interactions, and blob expiration interactions are out of scope - see `docs/designs/blob-versioning.md`. + General: - Raised the minimum supported Node.js runtime from 21 to 22 because Node.js 21 has reached end of life. diff --git a/README.md b/README.md index 149e59a46..a1a4d18a3 100644 --- a/README.md +++ b/README.md @@ -208,6 +208,8 @@ Following extension configurations are supported: - `azurite.inMemoryPersistence` Disable persisting any data to disk. If the Azurite process is terminated, all data is lost. - `azurite.extentMemoryLimit` When using in-memory persistence, limit the total size of extents (blob and queue content) to a specific number of megabytes. This does not limit blob, queue, or table metadata. Defaults to 50% of total memory. - `azurite.disableTelemetry` Disable telemetry data collection of this Azurite execution. By default, Azurite will collect telemetry data to help improve the product. +- `azurite.accountConfigFile` Path to a JSON file with account level (management plane) configuration, for example to enable blob versioning. Mutually exclusive with `azurite.accountConfig`. +- `azurite.accountConfig` Inline JSON string with account level (management plane) configuration, for example to enable blob versioning. Mutually exclusive with `azurite.accountConfigFile`. ### [DockerHub](https://hub.docker.com/_/microsoft-azure-storage-azurite) @@ -240,7 +242,7 @@ docker run -p 10000:10000 -p 10001:10001 -v c:/azurite:/data mcr.microsoft.com/a #### Customize all Azurite V3 supported parameters for docker image ```bash -docker run -p 7777:7777 -p 8888:8888 -p 9999:9999 -v c:/azurite:/workspace mcr.microsoft.com/azure-storage/azurite azurite -l /workspace -d /workspace/debug.log --blobPort 7777 --blobHost 0.0.0.0 --blobKeepAliveTimeout 5 --queuePort 8888 --queueHost 0.0.0.0 --queueKeepAliveTimeout 5 --tablePort 9999 --tableHost 0.0.0.0 --tableKeepAliveTimeout 5 --loose --skipApiVersionCheck --disableProductStyleUrl --disableTelemetry +docker run -p 7777:7777 -p 8888:8888 -p 9999:9999 -v c:/azurite:/workspace mcr.microsoft.com/azure-storage/azurite azurite -l /workspace -d /workspace/debug.log --blobPort 7777 --blobHost 0.0.0.0 --blobKeepAliveTimeout 5 --queuePort 8888 --queueHost 0.0.0.0 --queueKeepAliveTimeout 5 --tablePort 9999 --tableHost 0.0.0.0 --tableKeepAliveTimeout 5 --loose --skipApiVersionCheck --disableProductStyleUrl --disableTelemetry --accountConfigFile /workspace/myAccountConfig.json ``` Above command will try to start Azurite image with configurations: @@ -275,6 +277,8 @@ Above command will try to start Azurite image with configurations: `--azurite.disableTelemetry` disable telemetry data collection of this Azurite execution. By default, Azurite will collect telemetry data to help improve the product. +`--accountConfigFile` / `--accountConfig` supply account level (management plane) configuration such as enabling blob versioning. See [Blob versioning](#blob-versioning). + > If you use customized azurite parameters for docker image, `--blobHost 0.0.0.0`, `--queueHost 0.0.0.0` are required parameters. > In above sample, you need to use **double first forward slash** for location and debug path parameters to avoid a [known issue](https://stackoverflow.com/questions/48427366/docker-build-command-add-c-program-files-git-to-the-path-passed-as-build-argu) for Git on Windows. @@ -458,6 +462,78 @@ Optional. By default, Azurite will collect telemetry data to help improve the pr --disableTelemetry ``` +### Blob versioning + +Optional. Enable [blob versioning](https://learn.microsoft.com/en-us/azure/storage/blobs/versioning-overview) +for one or more storage accounts. When versioning is enabled for an account, overwriting +a blob preserves the previous content as a version addressable by its version ID. + +In Azure Storage, versioning is an account level setting configured through the ARM +management plane (`Microsoft.Storage/storageAccounts/blobServices/default`, property +`isVersioningEnabled`), not through the data plane REST API that Azurite emulates. +Azurite has no management plane, so the setting is supplied at start up with one of two +mutually exclusive options - a JSON file, or an inline JSON string: + +```cmd +--accountConfigFile +``` + +```cmd +--accountConfig +``` + +For example, with `./myAccountConfig.json`: + +```json +{ + "accounts": [ + { + "name": "devstoreaccount1", + "blobService": { + "isVersioningEnabled": true + } + } + ] +} +``` + +```cmd +azurite --accountConfigFile ./myAccountConfig.json +``` + +Or inline, without a file: + +```cmd +azurite --accountConfig "{\"accounts\":[{\"name\":\"devstoreaccount1\",\"blobService\":{\"isVersioningEnabled\":true}}]}" +``` + +A single account can also be given directly, without the `accounts` wrapper: + +```json +{ "name": "devstoreaccount1", "blobService": { "isVersioningEnabled": true } } +``` + +Account names are matched case insensitively, and any account not listed keeps the +default of versioning disabled - so this configuration only opts accounts in, it never +changes behaviour for accounts you did not name. If you run Azurite with +[multiple accounts](#customized-storage-accounts--keys), list each account that needs +versioning; a single configuration is not applied to all of them implicitly. The resolved +configuration is written to the debug log at start up. + +Versioning changes how blob writes are persisted, so it cannot be switched on or off +against a workspace that already holds data. The setting is persisted alongside the +metadata, and Azurite fails at start up if the configuration conflicts with the previous +run. To change it, use a clean workspace (a different `--location`, or remove the +existing one). + +Versioning is implemented for the default LokiJS metadata store only. Configuring it +together with the SQL based metadata implementation (via `AZURITE_DB`) is rejected at +start up. + +See [docs/designs/blob-versioning.md](docs/designs/blob-versioning.md) for the data model, +the supported operations, and the interactions that are not implemented (blob version +SAS, soft delete, blob expiration). + ### Use in-memory storage Optional. Disable persisting any data to disk and only store data in-memory. If the Azurite process is terminated, all @@ -1073,6 +1149,7 @@ Detailed support matrix: - Copy Blob (Only supports copy within same Azurite instance) - Abort Copy Blob (Only supports copy within same Azurite instance) - Copy Blob From URL (Only supports copy within same Azurite instance, only on Loki) + - Blob Versions (Only on Loki, opt in per account, see [Blob versioning](#blob-versioning)) - Access control based on conditional headers - Following features or REST APIs are NOT supported or limited supported in this release (will support more features per customers feedback in future releases) - SharedKey Lite @@ -1081,7 +1158,7 @@ Detailed support matrix: - Soft delete & Undelete Blob - Incremental Copy Blob - Blob Query - - Blob Versions + - Blob Version SAS (the `x` permission and the `sr=bv` signed resource) - Blob Last Access Time - Concurrent Append - Blob Expiry diff --git a/docs/designs/blob-versioning.md b/docs/designs/blob-versioning.md new file mode 100644 index 000000000..cd3d17cff --- /dev/null +++ b/docs/designs/blob-versioning.md @@ -0,0 +1,125 @@ +# Blob Versioning + +Tracking issue: [#665](https://github.com/Azure/Azurite/issues/665) + +Reference behaviour: [Blob versioning](https://learn.microsoft.com/en-us/azure/storage/blobs/versioning-overview) + +## Summary + +When blob versioning is enabled for an account, every write to a blob preserves the +previous content as a read-only *version* identified by a version ID. The version ID is +an RFC 3339 timestamp with 7 digit fractional seconds, for example +`2026-08-12T10:00:00.0000000Z`. + +## How versioning is enabled + +Azure Storage configures versioning through the **ARM management plane** +(`Microsoft.Storage/storageAccounts/blobServices/default`, property +`isVersioningEnabled`). It is not part of the data plane REST API that Azurite +emulates - `Set Blob Service Properties` has no `Versioning` element, and adding one +would mean emulating an API that does not exist in the real service. + +Azurite has no management plane, so account level settings are supplied at start up +instead, with two mutually exclusive options: + +```bash +# From a file +azurite --accountConfigFile ./myAccountConfig.json + +# Inline +azurite --accountConfig '{"accounts":[{"name":"devstoreaccount1","blobService":{"isVersioningEnabled":true}}]}' +``` + +`./myAccountConfig.json`: + +```json +{ + "accounts": [ + { + "name": "devstoreaccount1", + "blobService": { + "isVersioningEnabled": true + } + } + ] +} +``` + +A single account may also be supplied directly, without the `accounts` wrapper: + +```json +{ "name": "devstoreaccount1", "blobService": { "isVersioningEnabled": true } } +``` + +Account names are matched case insensitively. An account that is not listed uses the +defaults, which is versioning disabled - so this configuration only ever opts accounts +in, it never changes behaviour for accounts you did not mention. This composes with +[customized storage accounts](../../README.md#customized-storage-accounts--keys): list +each account that needs versioning in the configuration. + +The resolved configuration is written to the debug log at start up, so an Azurite issue +report shows which account settings were actually in effect. + +### Changing the setting on an existing workspace + +Versioning changes how writes are persisted, so switching it on or off against a +workspace that already contains blobs would leave the metadata store in a state that +matches neither setting. The configuration is therefore persisted in the metadata store +and reconciled at start up: + +1. Read the configuration persisted by the previous run. +2. Compare it with the configuration supplied on the command line. +3. If there is no conflict, run with the previous configuration merged with the new + input, and persist the result. +4. If there is a conflict, fail at start up with a message naming the account. + +To flip the setting, start Azurite against a clean workspace (a different `--location`, +or remove the existing one). + +## Data model + +Versions live in the same Loki collection as the blob they belong to +(`$BLOBS_COLLECTION$`), keyed by `accountName`, `containerName`, `name`, `snapshot` and +`versionId`. Exactly one document per blob has `isCurrentVersion: true`, or none once the +current version has been deleted. + +Blobs written before versioning was enabled have no `versionId` and no +`isCurrentVersion` field. Every query that means "the blob itself" therefore matches +`isCurrentVersion: { $ne: false }` rather than `isCurrentVersion: true`, so those blobs +continue to resolve. + +Account configuration lives in its own collection (`$ACCOUNTS_COLLECTION$`) rather than +alongside blob documents, so that the queue and table services can reuse it when they +need account level settings. + +## Behaviour + +| Operation | Behaviour with versioning enabled | +| --- | --- | +| Put Blob, Put Block List, Copy Blob | Overwriting retains the previous content as a version; the response carries `x-ms-version-id` | +| Get Blob, Get Blob Properties | `?versionid=` addresses one version; without it the current version is addressed. `x-ms-is-current-version: true` is returned only for the current version | +| List Blobs | `include=versions` returns previous versions with `VersionId` and `IsCurrentVersion`; they are hidden otherwise. Versions of the same blob are ordered oldest first, current last | +| Delete Blob with `?versionid=` | Deletes just that version; `x-ms-delete-snapshots` cannot be combined with it | +| Delete Blob without `?versionid=` | Deletes the current version and leaves previous versions in place - it does **not** fail with `SnapshotsPresent` because versions exist | +| Snapshots | Continue to work as before, and still block deleting the base blob with `SnapshotsPresent`. Using versioning and snapshots together is supported but, as in production, not recommended | +| Restore a version | No dedicated API: copy the version over the current version, `Copy Blob` with a `?versionid=` qualified source | +| `?snapshot=` and `?versionid=` together | 400 `InvalidQueryParameterValue` | + +## Not implemented + +These are deliberately out of scope, and are tracked separately rather than partially +emulated: + +- **SQL metadata store.** Versioning is implemented for the Loki store only. Configuring + versioning together with `AZURITE_DB` fails at start up, and a request carrying + `versionid` against the SQL store returns a not-implemented error rather than silently + reading the current version. +- **Blob soft delete**, which Azurite does not support at all, so none of the + soft-delete/versioning interactions (`include=deletedwithversions`, `HasVersionsOnly`, + permanent delete) are emulated. +- **Blob version SAS.** Azurite supports SAS in general, but the `x` (delete version) + permission and the `sr=bv` (blob version) signed resource are not implemented, so a SAS + cannot be used to address a specific version. +- **Get Block List** and **Get Page Ranges** with a version ID. The current storage + swagger does not define `versionid` on either operation, matching Azure. +- **Blob expiration** and object replication interactions. diff --git a/package.json b/package.json index 2c8d44869..81712d8b6 100644 --- a/package.json +++ b/package.json @@ -275,6 +275,22 @@ "type": "boolean", "default": false, "description": "Disable telemetry data collection of this Azurite execution. By default, Azurite will collect telemetry data to help improve the product." + }, + "azurite.accountConfigFile": { + "type": [ + "string", + "null" + ], + "default": null, + "description": "Path to a JSON file with account level (management plane) configuration, for example to enable blob versioning. Mutually exclusive with azurite.accountConfig." + }, + "azurite.accountConfig": { + "type": [ + "string", + "null" + ], + "default": null, + "description": "Inline JSON string with account level (management plane) configuration, for example to enable blob versioning. Mutually exclusive with azurite.accountConfigFile." } } } diff --git a/src/blob/BlobConfiguration.ts b/src/blob/BlobConfiguration.ts index b77f94a4d..a3453e3e1 100644 --- a/src/blob/BlobConfiguration.ts +++ b/src/blob/BlobConfiguration.ts @@ -1,3 +1,4 @@ +import { IAccountModel } from "../common/AccountModel"; import ConfigurationBase from "../common/ConfigurationBase"; import { StoreDestinationArray } from "../common/persistence/IExtentStore"; import { MemoryExtentChunkStore } from "../common/persistence/MemoryExtentStore"; @@ -45,6 +46,7 @@ export default class BlobConfiguration extends ConfigurationBase { disableProductStyleUrl: boolean = false, public readonly isMemoryPersistence: boolean = false, public readonly memoryStore?: MemoryExtentChunkStore, + public readonly accountModel?: IAccountModel, ) { super( host, diff --git a/src/blob/BlobEnvironment.ts b/src/blob/BlobEnvironment.ts index d046a3773..14642da24 100644 --- a/src/blob/BlobEnvironment.ts +++ b/src/blob/BlobEnvironment.ts @@ -2,6 +2,8 @@ import args from "args"; import { access, ensureDir } from "fs-extra"; import { dirname } from "path"; +import { IAccountModel } from "../common/AccountModel"; +import { resolveAccountModel } from "../common/EnvironmentFunctions"; import IBlobEnvironment from "./IBlobEnvironment"; import { DEFAULT_BLOB_LISTENING_PORT, @@ -70,6 +72,14 @@ if (!(args as any).config.name) { .option( ["", "disableTelemetry"], "Optional. Disable telemetry data collection of this Azurite execution. By default, Azurite will collect telemetry data to help improve the product." + ) + .option( + ["", "accountConfigFile"], + "Optional. Path to a JSON file with account level (management plane) configuration, for example to enable blob versioning. Mutually exclusive with --accountConfig" + ) + .option( + ["", "accountConfig"], + "Optional. Inline JSON string with account level (management plane) configuration, for example to enable blob versioning. Mutually exclusive with --accountConfigFile" ); (args as any).config.name = "azurite-blob"; @@ -170,6 +180,13 @@ export default class BlobEnvironment implements IBlobEnvironment { return this.flags.extentMemoryLimit; } + public async accountModel(): Promise { + return resolveAccountModel( + this.flags.accountConfigFile, + this.flags.accountConfig + ); + } + public async debug(): Promise { if (typeof this.flags.debug === "string") { // Enable debug log to file diff --git a/src/blob/BlobServer.ts b/src/blob/BlobServer.ts index fcf5952eb..c78d8dfc1 100644 --- a/src/blob/BlobServer.ts +++ b/src/blob/BlobServer.ts @@ -79,7 +79,9 @@ export default class BlobServer extends ServerBase implements ICleaner { // and replace the default LokiBlobMetadataStore const metadataStore: IBlobMetadataStore = new LokiBlobMetadataStore( configuration.metadataDBPath, - configuration.isMemoryPersistence + configuration.isMemoryPersistence, + configuration.accountModel, + logger ); const extentMetadataStore: IExtentMetadataStore = diff --git a/src/blob/BlobServerFactory.ts b/src/blob/BlobServerFactory.ts index 158456476..845d7dbda 100644 --- a/src/blob/BlobServerFactory.ts +++ b/src/blob/BlobServerFactory.ts @@ -25,6 +25,7 @@ export class BlobServerFactory { const env = blobEnvironment ? blobEnvironment : new BlobEnvironment(); const location = await env.location(); const debugFilePath = await env.debug(); + const accountModel = await env.accountModel(); if (typeof debugFilePath === "boolean") { throw RangeError( @@ -48,6 +49,14 @@ export class BlobServerFactory { if (env.extentMemoryLimit() !== undefined) { throw new Error(`The --extentMemoryLimit option is not supported when using SQL-based metadata storage.`) } + if ( + accountModel !== undefined && + accountModel.accounts.some( + (account) => account.blobService.isVersioningEnabled + ) + ) { + throw new Error(`Blob versioning is not supported when using SQL-based metadata storage.`) + } const config = new SqlBlobConfiguration( env.blobHost(), @@ -90,6 +99,8 @@ export class BlobServerFactory { env.oauth(), env.disableProductStyleUrl(), env.inMemoryPersistence(), + undefined, + accountModel, ); return new BlobServer(config); diff --git a/src/blob/IBlobEnvironment.ts b/src/blob/IBlobEnvironment.ts index a57700759..10b9cba75 100644 --- a/src/blob/IBlobEnvironment.ts +++ b/src/blob/IBlobEnvironment.ts @@ -1,3 +1,5 @@ +import { IAccountModel } from "../common/AccountModel"; + export default interface IBlobEnvironment { blobHost(): string | undefined; blobPort(): number | undefined; @@ -15,4 +17,5 @@ export default interface IBlobEnvironment { inMemoryPersistence(): boolean; extentMemoryLimit(): number | undefined; disableTelemetry(): boolean; + accountModel(): Promise; } diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts index 1ee4b9bc6..c48eb1fd8 100644 --- a/src/blob/handlers/BlobHandler.ts +++ b/src/blob/handlers/BlobHandler.ts @@ -29,7 +29,8 @@ import { deserializePageBlobRangeHeader, deserializeRangeHeader, getBlobTagsCount, - validateBlobTag + validateBlobTag, + validateSnapshotAndVersionId } from "../utils/utils"; import BaseHandler from "./BaseHandler"; import IPageBlobRangesManager from "./IPageBlobRangesManager"; @@ -70,6 +71,12 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const containerName = blobCtx.container!; const blobName = blobCtx.blob!; + validateSnapshotAndVersionId( + options.snapshot, + options.versionId, + context.contextId + ); + const blob = await this.metadataStore.downloadBlob( context, accountName, @@ -77,7 +84,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { blobName, options.snapshot, options.leaseAccessConditions, - options.modifiedAccessConditions + options.modifiedAccessConditions, + options.versionId ); if (blob.properties.accessTier === Models.AccessTier.Archive) { @@ -111,6 +119,12 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const account = blobCtx.account!; const container = blobCtx.container!; const blob = blobCtx.blob!; + validateSnapshotAndVersionId( + options.snapshot, + options.versionId, + context.contextId + ); + const res = await this.metadataStore.getBlobProperties( context, account, @@ -118,7 +132,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { blob, options.snapshot, options.leaseAccessConditions, - options.modifiedAccessConditions + options.modifiedAccessConditions, + options.versionId ); // TODO: Create get metadata specific request in swagger @@ -158,6 +173,11 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { contentLanguage: context.request!.getQuery("rscl") ?? res.properties.contentLanguage, contentType: context.request!.getQuery("rsct") ?? res.properties.contentType, tagCount: res.properties.tagCount, + versionId: res.versionId, + // x-ms-is-current-version is only returned for the current version, it is + // absent when a previous version is addressed. + isCurrentVersion: + res.isCurrentVersion === true ? true : undefined, }; return response; @@ -179,6 +199,13 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const account = blobCtx.account!; const container = blobCtx.container!; const blob = blobCtx.blob!; + + validateSnapshotAndVersionId( + options.snapshot, + options.versionId, + context.contextId + ); + await this.metadataStore.deleteBlob( context, account, @@ -649,6 +676,10 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { sourceBlob ] = extractStoragePartsFromPath(url.hostname, url.pathname, blobCtx.disableProductStyleUrl); const snapshot = url.searchParams.get("snapshot") || ""; + // A previous version can be used as a copy source, which is how a blob is restored + // from one of its versions. + const sourceVersionId = url.searchParams.get("versionid") || undefined; + validateSnapshotAndVersionId(snapshot, sourceVersionId, context.contextId); if ( sourceAccount === undefined || @@ -674,7 +705,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { account: sourceAccount, container: sourceContainer, blob: sourceBlob, - snapshot + snapshot, + versionId: sourceVersionId }, { account, container, blob }, copySource, @@ -848,6 +880,10 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { sourceBlob ] = extractStoragePartsFromPath(url.hostname, url.pathname, blobCtx.disableProductStyleUrl); const snapshot = url.searchParams.get("snapshot") || ""; + // A previous version can be used as a copy source, which is how a blob is restored + // from one of its versions. + const sourceVersionId = url.searchParams.get("versionid") || undefined; + validateSnapshotAndVersionId(snapshot, sourceVersionId, context.contextId); if ( sourceAccount === undefined || @@ -877,7 +913,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { account: sourceAccount, container: sourceContainer, blob: sourceBlob, - snapshot + snapshot, + versionId: sourceVersionId }, { account, container, blob }, copySource, @@ -1123,6 +1160,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { blob.properties.blobType === Models.BlobType.AppendBlob ? (blob.committedBlocksInOrder || []).length : undefined, + versionId: blob.versionId, + isCurrentVersion: blob.isCurrentVersion === true ? true : undefined, }; return response; @@ -1255,7 +1294,9 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { tagCount: getBlobTagsCount(blob.blobTags), isServerEncrypted: true, creationTime: blob.properties.creationTime, - clientRequestId: options.requestId + clientRequestId: options.requestId, + versionId: blob.versionId, + isCurrentVersion: blob.isCurrentVersion === true ? true : undefined }; return response; diff --git a/src/blob/handlers/BlockBlobHandler.ts b/src/blob/handlers/BlockBlobHandler.ts index 666a067f0..38dcfd92b 100644 --- a/src/blob/handlers/BlockBlobHandler.ts +++ b/src/blob/handlers/BlockBlobHandler.ts @@ -152,7 +152,9 @@ export default class BlockBlobHandler version: BLOB_API_VERSION, date, isServerEncrypted: true, - clientRequestId: options.requestId + clientRequestId: options.requestId, + // Set by the metadata store when versioning is enabled for the account + versionId: blob.versionId }; return response; @@ -381,7 +383,9 @@ export default class BlockBlobHandler version: BLOB_API_VERSION, date: blobCtx.startTime, isServerEncrypted: true, - clientRequestId: options.requestId + clientRequestId: options.requestId, + // Set by the metadata store when versioning is enabled for the account + versionId: blob.versionId }; return response; } diff --git a/src/blob/handlers/ContainerHandler.ts b/src/blob/handlers/ContainerHandler.ts index fe5ec6adb..4c5a31389 100644 --- a/src/blob/handlers/ContainerHandler.ts +++ b/src/blob/handlers/ContainerHandler.ts @@ -644,6 +644,7 @@ export default class ContainerHandler extends BaseHandler let includeUncommittedBlobs: boolean = false; let includeTags: boolean = false; let includeMetadata: boolean = false; + let includeVersions: boolean = false; if (options.include !== undefined) { options.include.forEach(element => { if (Models.ListBlobsIncludeItem.Snapshots.toLowerCase() === element.toLowerCase()) { @@ -658,6 +659,9 @@ export default class ContainerHandler extends BaseHandler if (Models.ListBlobsIncludeItem.Metadata.toLowerCase() === element.toLowerCase()) { includeMetadata = true; } + if (Models.ListBlobsIncludeItem.Versions.toLowerCase() === element.toLowerCase()) { + includeVersions = true; + } }) } if ( @@ -677,7 +681,8 @@ export default class ContainerHandler extends BaseHandler options.maxresults, marker, includeSnapshots, - includeUncommittedBlobs + includeUncommittedBlobs, + includeVersions ); const serviceEndpoint = `${request.getEndpoint()}/${accountName}`; @@ -749,6 +754,7 @@ export default class ContainerHandler extends BaseHandler let includeUncommittedBlobs: boolean = false; let includeTags: boolean = false; let includeMetadata: boolean = false; + let includeVersions: boolean = false; if (options.include !== undefined) { options.include.forEach(element => { if (Models.ListBlobsIncludeItem.Snapshots.toLowerCase() === element.toLowerCase()) { @@ -763,6 +769,9 @@ export default class ContainerHandler extends BaseHandler if (Models.ListBlobsIncludeItem.Metadata.toLowerCase() === element.toLowerCase()) { includeMetadata = true; } + if (Models.ListBlobsIncludeItem.Versions.toLowerCase() === element.toLowerCase()) { + includeVersions = true; + } } ) } @@ -783,7 +792,8 @@ export default class ContainerHandler extends BaseHandler options.maxresults, marker, includeSnapshots, - includeUncommittedBlobs + includeUncommittedBlobs, + includeVersions ); const serviceEndpoint = `${request.getEndpoint()}/${accountName}`; diff --git a/src/blob/persistence/IBlobMetadataStore.ts b/src/blob/persistence/IBlobMetadataStore.ts index fb933f8df..3af01023b 100644 --- a/src/blob/persistence/IBlobMetadataStore.ts +++ b/src/blob/persistence/IBlobMetadataStore.ts @@ -151,6 +151,8 @@ interface IGetBlobPropertiesRes { properties: Models.BlobPropertiesInternal; metadata?: Models.BlobMetadata; blobCommittedBlockCount?: number; // AppendBlobOnly + versionId?: string; + isCurrentVersion?: boolean; } export type GetBlobPropertiesRes = IGetBlobPropertiesRes; @@ -181,6 +183,7 @@ interface IBlobId { container: string; blob: string; snapshot?: string; + versionId?: string; } export type BlobId = IBlobId; @@ -495,14 +498,16 @@ export interface IBlobMetadataStore maxResults?: number, marker?: string, includeSnapshots?: boolean, - includeUncommittedBlobs?: boolean + includeUncommittedBlobs?: boolean, + includeVersions?: boolean ): Promise<[BlobModel[], BlobPrefixModel[], string | undefined]>; listAllBlobs( maxResults?: number, marker?: string, includeSnapshots?: boolean, - includeUncommittedBlobs?: boolean + includeUncommittedBlobs?: boolean, + includeVersions?: boolean ): Promise<[BlobModel[], string | undefined]>; filterBlobs( @@ -575,7 +580,8 @@ export interface IBlobMetadataStore blob: string, snapshot: string | undefined, leaseAccessConditions?: Models.LeaseAccessConditions, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise; /** @@ -598,7 +604,8 @@ export interface IBlobMetadataStore blob: string, snapshot: string | undefined, leaseAccessConditions: Models.LeaseAccessConditions | undefined, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise; /** diff --git a/src/blob/persistence/LokiBlobMetadataStore.ts b/src/blob/persistence/LokiBlobMetadataStore.ts index d0e3f62d6..1fa4ff5db 100644 --- a/src/blob/persistence/LokiBlobMetadataStore.ts +++ b/src/blob/persistence/LokiBlobMetadataStore.ts @@ -2,7 +2,14 @@ import { stat } from "fs"; import Loki from "lokijs"; import { randomUUID as uuid } from "crypto"; +import { + AccountConfigError, + getAccountBlobServiceConfig, + IAccountConfig, + IAccountModel +} from "../../common/AccountModel"; import IGCExtentProvider from "../../common/IGCExtentProvider"; +import ILogger from "../../common/ILogger"; import { convertDateTimeStringMsTo7Digital, rimrafAsync @@ -110,12 +117,22 @@ export default class LokiBlobMetadataStore private readonly CONTAINERS_COLLECTION = "$CONTAINERS_COLLECTION$"; private readonly BLOBS_COLLECTION = "$BLOBS_COLLECTION$"; private readonly BLOCKS_COLLECTION = "$BLOCKS_COLLECTION$"; + private readonly ACCOUNTS_COLLECTION = "$ACCOUNTS_COLLECTION$"; private readonly pageBlobRangesManager = new PageBlobRangesManager(); + /** + * Account level configuration in effect for this run, resolved during init() from the + * configuration supplied on the command line merged with the configuration persisted + * by the previous run. + */ + private accountConfigs: Map = new Map(); + public constructor( public readonly lokiDBPath: string, - inMemory: boolean + inMemory: boolean, + private readonly inputAccountModel?: IAccountModel, + private readonly logger?: ILogger ) { this.db = new Loki( lokiDBPath, @@ -190,6 +207,17 @@ export default class LokiBlobMetadataStore }); } + // Create account configuration collection if not exists. Kept in its own + // collection (rather than alongside blob documents) so that queue and table can + // reuse the same account configuration when they need account level settings. + if (this.db.getCollection(this.ACCOUNTS_COLLECTION) === null) { + this.db.addCollection(this.ACCOUNTS_COLLECTION, { + unique: ["name"] + }); + } + + this.resolveAccountConfigs(); + await new Promise((resolve, reject) => { this.db.saveDatabase((err) => { if (err) { @@ -204,6 +232,154 @@ export default class LokiBlobMetadataStore this.closed = false; } + /** + * Resolve the account level configuration for this run. + * + * Blob versioning changes how blob writes are persisted, so switching it on or off + * against an existing workspace would leave the metadata store in a state that does + * not match either setting. The resolution rules are therefore: + * + * 1. Read the configuration persisted by the previous run. + * 2. Compare it with the configuration supplied on the command line. + * 3. If there is no conflict, run with the previous configuration merged with the + * new input, and persist the result. + * 4. If there is a conflict, fail at start up with an actionable message. + * + * @private + * @memberof LokiBlobMetadataStore + */ + private resolveAccountConfigs(): void { + const coll = this.db.getCollection(this.ACCOUNTS_COLLECTION); + const persisted: IAccountConfig[] = coll.find({}).map((doc: any) => ({ + name: doc.name, + blobService: { ...doc.blobService } + })); + + const resolved = new Map(); + for (const account of persisted) { + resolved.set(account.name, account); + } + + for (const incoming of this.inputAccountModel?.accounts ?? []) { + const previous = resolved.get(incoming.name); + + if ( + previous !== undefined && + previous.blobService.isVersioningEnabled !== + incoming.blobService.isVersioningEnabled + ) { + throw new AccountConfigError( + `Account "${incoming.name}" was previously started with blob versioning ` + + `${previous.blobService.isVersioningEnabled ? "enabled" : "disabled"} ` + + `but is now configured with blob versioning ` + + `${incoming.blobService.isVersioningEnabled ? "enabled" : "disabled"}. ` + + `Changing this setting against an existing workspace is not supported. ` + + `Either keep the previous setting, or start Azurite against a clean ` + + `workspace (a different --location, or remove the existing one).` + ); + } + + resolved.set(incoming.name, incoming); + + if (previous === undefined) { + coll.insert({ name: incoming.name, blobService: incoming.blobService }); + } + } + + this.accountConfigs = resolved; + + // Print the resolved configuration so that Azurite issue reports include the + // account settings that were actually in effect. + if (this.logger !== undefined) { + if (resolved.size === 0) { + this.logger.debug( + `LokiBlobMetadataStore:resolveAccountConfigs() No account level configuration supplied or persisted, using defaults (blob versioning disabled).` + ); + } else { + this.logger.debug( + `LokiBlobMetadataStore:resolveAccountConfigs() Account level configuration in effect: ${JSON.stringify( + [...resolved.values()] + )}` + ); + } + } + } + + /** + * Whether blob versioning is enabled for the given account. + * + * @private + * @param {string} account + * @returns {boolean} + * @memberof LokiBlobMetadataStore + */ + private isVersioningEnabled(account: string): boolean { + const config = this.accountConfigs.get(account.toLowerCase()); + return config !== undefined + ? config.blobService.isVersioningEnabled + : getAccountBlobServiceConfig(undefined, account).isVersioningEnabled; + } + + /** + * Generate a version ID for a blob write. + * + * Matches the Azure Storage format: an RFC 3339 timestamp with 7 digit fractional + * seconds, for example "2026-08-12T10:00:00.0000000Z". Version IDs must be unique + * and increasing per blob, so when two writes land inside the same millisecond the + * timestamp is advanced until it is free. + * + * @private + * @param {Context} context + * @param {string} account + * @param {string} container + * @param {string} blob + * @returns {string} + * @memberof LokiBlobMetadataStore + */ + private generateVersionId( + context: Context, + account: string, + container: string, + blob: string + ): string { + const coll = this.db.getCollection(this.BLOBS_COLLECTION); + let candidateTime = context.startTime!.getTime(); + + // eslint-disable-next-line no-constant-condition + while (true) { + const candidate = convertDateTimeStringMsTo7Digital( + new Date(candidateTime).toISOString() + ); + const clash = coll.findOne({ + accountName: account, + containerName: container, + name: blob, + versionId: candidate + }); + if (clash === null || clash === undefined) { + return candidate; + } + candidateTime += 1; + } + } + + /** + * Build a Loki query that matches only the current version of a blob. + * + * Previous versions live in the same collection as the blob they belong to and share + * the base blob's `snapshot` value, so every query that means "the blob itself" has + * to exclude them. Blobs written before versioning was enabled have no + * `isCurrentVersion` field at all, hence `$ne: false` rather than `$eq: true`. + * + * @private + * @param {*} query + * @returns {*} + * @memberof LokiBlobMetadataStore + */ + private currentVersionQuery(query: any): any { + return { ...query, isCurrentVersion: { $ne: false } }; + } + /** * Close loki DB. * @@ -919,7 +1095,8 @@ export default class LokiBlobMetadataStore maxResults: number = DEFAULT_LIST_BLOBS_MAX_RESULTS, marker: string = "", includeSnapshots?: boolean, - includeUncommittedBlobs?: boolean + includeUncommittedBlobs?: boolean, + includeVersions?: boolean ): Promise<[BlobModel[], BlobPrefixModel[], string | undefined]> { const query: any = {}; if (prefix !== "") { @@ -954,8 +1131,19 @@ export default class LokiBlobMetadataStore .where((obj) => { return includeUncommittedBlobs ? true : obj.isCommitted; }) + .where((obj) => { + return includeVersions ? true : obj.isCurrentVersion !== false; + }) .sort((obj1, obj2) => { - if (obj1.name === obj2.name) return 0; + // Versions of the same blob are returned together, oldest first, with the + // current version last. This matches the ordering List Blobs uses when + // include=versions is requested. + if (obj1.name === obj2.name) { + const version1 = obj1.versionId ?? ""; + const version2 = obj2.versionId ?? ""; + if (version1 === version2) return 0; + return version1 > version2 ? 1 : -1; + } if (obj1.name > obj2.name) return 1; return -1; }) @@ -992,7 +1180,8 @@ export default class LokiBlobMetadataStore maxResults: number = DEFAULT_LIST_BLOBS_MAX_RESULTS, marker: string = "", includeSnapshots?: boolean, - includeUncommittedBlobs?: boolean + includeUncommittedBlobs?: boolean, + includeVersions?: boolean ): Promise<[BlobModel[], string | undefined]> { const coll = this.db.getCollection(this.BLOBS_COLLECTION); @@ -1007,6 +1196,9 @@ export default class LokiBlobMetadataStore .where((obj) => { return includeUncommittedBlobs ? true : obj.isCommitted; }) + .where((obj) => { + return includeVersions ? true : obj.isCurrentVersion !== false; + }) .simplesort("name") .limit(maxResults + 1) .data(); @@ -1049,12 +1241,14 @@ export default class LokiBlobMetadataStore blob.containerName ); const coll = this.db.getCollection(this.BLOBS_COLLECTION); - const blobDoc = coll.findOne({ - name: blob.name, - accountName: blob.accountName, - containerName: blob.containerName, - snapshot: blob.snapshot - }); + const blobDoc = coll.findOne( + this.currentVersionQuery({ + name: blob.name, + accountName: blob.accountName, + containerName: blob.containerName, + snapshot: blob.snapshot + }) + ); validateWriteConditions(context, modifiedAccessConditions, blobDoc); @@ -1067,6 +1261,12 @@ export default class LokiBlobMetadataStore throw StorageErrorFactory.getBlobAlreadyExists(context.contextId); } + // Versioning only applies to the base blob, snapshots keep their existing + // behaviour of being addressed by snapshot timestamp. + const versioningEnabled = + this.isVersioningEnabled(blob.accountName) && + (blob.snapshot === "" || blob.snapshot === undefined); + if (blobDoc) { LeaseFactory.createLeaseState(new BlobLeaseAdapter(blobDoc), context) .validate(new BlobWriteLeaseValidator(leaseAccessConditions)) @@ -1078,12 +1278,68 @@ export default class LokiBlobMetadataStore ) { throw StorageErrorFactory.getBlobArchived(context.contextId); } - coll.remove(blobDoc); + + if (versioningEnabled) { + // Retain the overwritten content as a previous version instead of removing it. + this.demoteToPreviousVersion(coll, blobDoc); + } else { + coll.remove(blobDoc); + } } + + if (versioningEnabled) { + blob.versionId = this.generateVersionId( + context, + blob.accountName, + blob.containerName, + blob.name + ); + blob.isCurrentVersion = true; + } + delete (blob as any).$loki; return coll.insert(blob); } + /** + * Turn the current version of a blob into a previous version, in place. + * + * A blob that was written before versioning was enabled on the account has no version + * ID of its own. Azure assigns one when such a blob is first overwritten, so do the + * same here using the blob's last modified time, which is the closest thing we have to + * the time the content was created. + * + * @private + * @param {Collection} coll + * @param {*} doc + * @memberof LokiBlobMetadataStore + */ + private demoteToPreviousVersion(coll: Collection, doc: any): void { + if (doc.versionId === undefined) { + const lastModified: Date | undefined = doc.properties?.lastModified; + doc.versionId = convertDateTimeStringMsTo7Digital( + (lastModified !== undefined + ? new Date(lastModified) + : new Date(0) + ).toISOString() + ); + } + doc.isCurrentVersion = false; + + // A previous version never holds a lease of its own. + new BlobLeaseSyncer(doc).sync({ + leaseId: undefined, + leaseExpireTime: undefined, + leaseDurationSeconds: undefined, + leaseBreakTime: undefined, + leaseDurationType: undefined, + leaseState: undefined, + leaseStatus: undefined + }); + + coll.update(doc); + } + /** * Create snapshot. * @@ -1191,7 +1447,8 @@ export default class LokiBlobMetadataStore blob: string, snapshot: string = "", leaseAccessConditions?: Models.LeaseAccessConditions, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise { const doc = await this.getBlobWithLeaseUpdated( account, @@ -1200,7 +1457,8 @@ export default class LokiBlobMetadataStore snapshot, context, false, - true + true, + versionId ); validateReadConditions(context, modifiedAccessConditions, doc); @@ -1236,12 +1494,14 @@ export default class LokiBlobMetadataStore snapshot: string = "" ): Promise { const coll = this.db.getCollection(this.BLOBS_COLLECTION); - const blobDoc = coll.findOne({ - name: blob, - accountName: account, - containerName: container, - snapshot - }); + const blobDoc = coll.findOne( + this.currentVersionQuery({ + name: blob, + accountName: account, + containerName: container, + snapshot + }) + ); if (blobDoc) { const blobModel = blobDoc as BlobModel; @@ -1274,7 +1534,8 @@ export default class LokiBlobMetadataStore blob: string, snapshot: string = "", leaseAccessConditions: Models.LeaseAccessConditions | undefined, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise { const doc = await this.getBlobWithLeaseUpdated( account, @@ -1283,7 +1544,8 @@ export default class LokiBlobMetadataStore snapshot, context, false, - true + true, + versionId ); validateReadConditions(context, modifiedAccessConditions, doc); @@ -1306,7 +1568,9 @@ export default class LokiBlobMetadataStore blobCommittedBlockCount: doc.properties.blobType === Models.BlobType.AppendBlob ? (doc.committedBlocksInOrder || []).length - : undefined + : undefined, + versionId: doc.versionId, + isCurrentVersion: doc.isCurrentVersion }; } @@ -1331,13 +1595,24 @@ export default class LokiBlobMetadataStore const coll = this.db.getCollection(this.BLOBS_COLLECTION); await this.checkContainerExist(context, account, container); + // x-ms-delete-snapshots cannot be combined with a version ID, the request addresses + // a single version which has no snapshots of its own. + if (options.versionId !== undefined && options.deleteSnapshots !== undefined) { + throw StorageErrorFactory.getInvalidOperation( + context.contextId!, + "Invalid operation against a blob version." + ); + } + const doc = await this.getBlobWithLeaseUpdated( account, container, blob, options.snapshot, context, - false + false, + undefined, + options.versionId ); validateWriteConditions(context, options.modifiedAccessConditions, doc); @@ -1361,21 +1636,40 @@ export default class LokiBlobMetadataStore context ); - // Scenario: Delete base blob only - if (againstBaseBlob && options.deleteSnapshots === undefined) { - const count = coll.count({ + // Scenario: Delete a single blob version. Other versions of the blob, and the + // current version, are unaffected. + if (options.versionId !== undefined && options.versionId !== "") { + coll.findAndRemove({ accountName: account, containerName: container, - name: blob + name: blob, + versionId: options.versionId }); - if (count > 1) { + return; + } + + // Snapshots of a blob still block deleting the base blob, but previous versions do + // not - with versioning enabled, deleting the current version leaves the previous + // versions in place. + const snapshotCount = coll.count({ + accountName: account, + containerName: container, + name: blob, + snapshot: { $gt: "" } + }); + + // Scenario: Delete base blob only + if (againstBaseBlob && options.deleteSnapshots === undefined) { + if (snapshotCount > 0) { throw StorageErrorFactory.getSnapshotsPresent(context.contextId!); } else { - coll.findAndRemove({ - accountName: account, - containerName: container, - name: blob - }); + coll.findAndRemove( + this.currentVersionQuery({ + accountName: account, + containerName: container, + name: blob + }) + ); } } @@ -1389,7 +1683,8 @@ export default class LokiBlobMetadataStore }); } - // Scenario: Delete base blob and snapshots + // Scenario: Delete base blob and snapshots. Previous versions are retained, they + // are removed only by an explicit delete against their version ID. if ( againstBaseBlob && options.deleteSnapshots === Models.DeleteSnapshotsOptionType.Include @@ -1397,8 +1692,16 @@ export default class LokiBlobMetadataStore coll.findAndRemove({ accountName: account, containerName: container, - name: blob + name: blob, + snapshot: { $gt: "" } }); + coll.findAndRemove( + this.currentVersionQuery({ + accountName: account, + containerName: container, + name: blob + }) + ); } // Scenario: Delete all snapshots only @@ -1816,12 +2119,14 @@ export default class LokiBlobMetadataStore await this.checkContainerExist(context, account, container); const coll = this.db.getCollection(this.BLOBS_COLLECTION); - const doc = coll.findOne({ - name: blob, - accountName: account, - containerName: container, - snapshot - }); + const doc = coll.findOne( + this.currentVersionQuery({ + name: blob, + accountName: account, + containerName: container, + snapshot + }) + ); if (!doc) { const requestId = context ? context.contextId : undefined; @@ -1850,12 +2155,14 @@ export default class LokiBlobMetadataStore { blobType: Models.BlobType | undefined; isCommitted: boolean } | undefined > { const coll = this.db.getCollection(this.BLOBS_COLLECTION); - const doc = coll.findOne({ - name: blob, - accountName: account, - containerName: container, - snapshot - }); + const doc = coll.findOne( + this.currentVersionQuery({ + name: blob, + accountName: account, + containerName: container, + snapshot + }) + ); if (!doc) { return undefined; } @@ -1892,7 +2199,8 @@ export default class LokiBlobMetadataStore source.snapshot, context, true, - true + true, + source.versionId ); options.sourceModifiedAccessConditions = @@ -2051,9 +2359,28 @@ export default class LokiBlobMetadataStore }); } + // A copy that overwrites an existing blob creates a new version of the destination, + // retaining the overwritten content as a previous version. + const versioningEnabled = this.isVersioningEnabled(destination.account); + if (destBlob) { - coll.remove(destBlob); + if (versioningEnabled) { + this.demoteToPreviousVersion(coll, destBlob); + } else { + coll.remove(destBlob); + } + } + + if (versioningEnabled) { + copiedBlob.versionId = this.generateVersionId( + context, + destination.account, + destination.container, + destination.blob + ); + copiedBlob.isCurrentVersion = true; } + coll.insert(copiedBlob); return copiedBlob.properties; } @@ -2088,7 +2415,8 @@ export default class LokiBlobMetadataStore source.snapshot, context, true, - true + true, + source.versionId ); options.sourceModifiedAccessConditions = @@ -2244,9 +2572,28 @@ export default class LokiBlobMetadataStore }); } + // A copy that overwrites an existing blob creates a new version of the destination, + // retaining the overwritten content as a previous version. + const versioningEnabled = this.isVersioningEnabled(destination.account); + if (destBlob) { - coll.remove(destBlob); + if (versioningEnabled) { + this.demoteToPreviousVersion(coll, destBlob); + } else { + coll.remove(destBlob); + } + } + + if (versioningEnabled) { + copiedBlob.versionId = this.generateVersionId( + context, + destination.account, + destination.container, + destination.blob + ); + copiedBlob.isCurrentVersion = true; } + coll.insert(copiedBlob); return copiedBlob.properties; } @@ -2615,7 +2962,32 @@ export default class LokiBlobMetadataStore } } - if (doc) { + // With versioning enabled, committing a block list over an existing committed blob + // retains the previous content as a version rather than updating it in place. An + // uncommitted doc is not a blob yet, so it is committed normally. + const versioningEnabled = + this.isVersioningEnabled(blob.accountName) && + (blob.snapshot === "" || blob.snapshot === undefined); + + if (versioningEnabled && doc && doc.isCommitted) { + this.demoteToPreviousVersion(coll, doc); + + blob.committedBlocksInOrder = selectedBlockList; + blob.properties.contentLength = selectedBlockList + .map((block) => block.size) + .reduce((total, val) => { + return total + val; + }, 0); + blob.versionId = this.generateVersionId( + context, + blob.accountName, + blob.containerName, + blob.name + ); + blob.isCurrentVersion = true; + delete (blob as any).$loki; + coll.insert(blob); + } else if (doc) { // Commit block list doc.properties.blobType = blob.properties.blobType; doc.properties.lastModified = blob.properties.lastModified; @@ -2651,6 +3023,15 @@ export default class LokiBlobMetadataStore .reduce((total, val) => { return total + val; }, 0); + if (versioningEnabled) { + blob.versionId = this.generateVersionId( + context, + blob.accountName, + blob.containerName, + blob.name + ); + blob.isCurrentVersion = true; + } coll.insert(blob); } @@ -3340,7 +3721,8 @@ export default class LokiBlobMetadataStore snapshot: string | undefined, context: Context, forceExist?: true, - forceCommitted?: boolean + forceCommitted?: boolean, + versionId?: string ): Promise; /** @@ -3365,7 +3747,8 @@ export default class LokiBlobMetadataStore snapshot: string | undefined, context: Context, forceExist: false, - forceCommitted?: boolean + forceCommitted?: boolean, + versionId?: string ): Promise; private async getBlobWithLeaseUpdated( @@ -3375,17 +3758,24 @@ export default class LokiBlobMetadataStore snapshot: string = "", context: Context, forceExist?: boolean, - forceCommitted?: boolean + forceCommitted?: boolean, + versionId?: string ): Promise { await this.checkContainerExist(context, account, container); const coll = this.db.getCollection(this.BLOBS_COLLECTION); - const doc = coll.findOne({ + const baseQuery = { name: blob, accountName: account, containerName: container, snapshot - }); + }; + // An explicit version ID addresses exactly one version, current or not. Without + // one the request addresses the current version only. + const doc = + versionId === undefined || versionId === "" + ? coll.findOne(this.currentVersionQuery(baseQuery)) + : coll.findOne({ ...baseQuery, versionId }); // Force exist if parameter forceExist is undefined or true if (forceExist === undefined || forceExist === true) { @@ -3416,8 +3806,11 @@ export default class LokiBlobMetadataStore ); } - // Snapshot doesn't have lease - if (snapshot !== undefined && snapshot !== "") { + // Neither a snapshot nor a previous version holds a lease + if ( + (snapshot !== undefined && snapshot !== "") || + doc.isCurrentVersion === false + ) { new BlobLeaseSyncer(doc).sync({ leaseId: undefined, leaseExpireTime: undefined, diff --git a/src/blob/persistence/SqlBlobMetadataStore.ts b/src/blob/persistence/SqlBlobMetadataStore.ts index 8443360b7..f5c90f110 100644 --- a/src/blob/persistence/SqlBlobMetadataStore.ts +++ b/src/blob/persistence/SqlBlobMetadataStore.ts @@ -1175,8 +1175,13 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { blob: string, snapshot: string = "", leaseAccessConditions?: Models.LeaseAccessConditions, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise { + // Blob versioning is not implemented for the SQL metadata store. Reject the + // request explicitly rather than silently ignoring versionId and returning the + // current version, which would look like a successful read of the wrong content. + this.assertVersioningNotRequested(context, versionId); return this.sequelize.transaction(async (t) => { await this.assertContainerExists(context, account, container, t); @@ -1300,8 +1305,10 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { maxResults: number = DEFAULT_LIST_BLOBS_MAX_RESULTS, marker?: string, includeSnapshots?: boolean, - includeUncommittedBlobs?: boolean + includeUncommittedBlobs?: boolean, + includeVersions?: boolean ): Promise<[BlobModel[], BlobPrefixModel[], any | undefined]> { + this.assertVersioningNotRequested(context, includeVersions ? "versions" : undefined); return this.sequelize.transaction(async (t) => { await this.assertContainerExists(context, account, container, t); @@ -1372,7 +1379,8 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { maxResults: number = DEFAULT_LIST_BLOBS_MAX_RESULTS, marker?: string, includeSnapshots?: boolean, - includeUncommittedBlobs?: boolean + includeUncommittedBlobs?: boolean, + includeVersions?: boolean ): Promise<[BlobModel[], any | undefined]> { const whereQuery: any = {}; if (marker !== undefined) { @@ -1763,8 +1771,13 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { blob: string, snapshot: string = "", leaseAccessConditions?: Models.LeaseAccessConditions, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise { + // Blob versioning is not implemented for the SQL metadata store. Reject the + // request explicitly rather than silently ignoring versionId and returning the + // current version, which would look like a successful read of the wrong content. + this.assertVersioningNotRequested(context, versionId); return this.sequelize.transaction(async (t) => { await this.assertContainerExists(context, account, container, t); @@ -1903,6 +1916,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { blob: string, options: Models.BlobDeleteMethodOptionalParams ): Promise { + this.assertVersioningNotRequested(context, options.versionId); await this.sequelize.transaction(async (t) => { await this.assertContainerExists(context, account, container, t); @@ -2921,6 +2935,25 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { return new BlobReferredExtentsAsyncIterator(this); } + /** + * Blob versioning is only implemented for the Loki metadata store. Fail loudly when a + * request asks for version specific behaviour against the SQL metadata store, so that + * it cannot be mistaken for a successful read or delete of the wrong content. + * + * @private + * @param {Context} context + * @param {string} [versionId] + * @memberof SqlBlobMetadataStore + */ + private assertVersioningNotRequested( + context: Context, + versionId?: string + ): void { + if (versionId !== undefined && versionId !== "") { + throw new NotImplementedinSQLError(context.contextId); + } + } + private async assertContainerExists( context: Context, account: string, diff --git a/src/blob/utils/utils.ts b/src/blob/utils/utils.ts index 53e00e1a4..58858876c 100644 --- a/src/blob/utils/utils.ts +++ b/src/blob/utils/utils.ts @@ -394,3 +394,34 @@ export function toBlobTags(input: TagContent[]): BlobTag[] { } }); } + +/** + * Validate the `snapshot` and `versionId` query parameters of a blob request. + * + * A request may address a snapshot or a version, but not both. Azure Storage rejects + * the combination with 400 InvalidQueryParameterValue. + * + * @export + * @param {string} [snapshot] + * @param {string} [versionId] + * @param {string} [contextID] + */ +export function validateSnapshotAndVersionId( + snapshot?: string, + versionId?: string, + contextID?: string +): void { + if ( + snapshot !== undefined && + snapshot !== "" && + versionId !== undefined && + versionId !== "" + ) { + throw StorageErrorFactory.getInvalidQueryParameterValue( + contextID, + "versionid", + versionId, + "The snapshot and versionid query parameters are mutually exclusive." + ); + } +} diff --git a/src/common/AccountModel.ts b/src/common/AccountModel.ts new file mode 100644 index 000000000..94810df9d --- /dev/null +++ b/src/common/AccountModel.ts @@ -0,0 +1,183 @@ +/** + * Azurite account (management plane) configuration model. + * + * Azure Storage exposes account level settings such as blob versioning through the + * ARM management plane (Microsoft.Storage/storageAccounts/blobServices), NOT through + * the data plane REST API that Azurite emulates. Azurite has no management plane, so + * these settings are supplied at start up instead - either as a JSON file + * (--accountConfigFile) or as an inline JSON string (--accountConfig). + * + * Keeping this model in `src/common` (rather than under `src/blob`) so queue and table + * can reuse the same account configuration when they need account level settings. + */ + +/** + * Account level settings for the blob service. + */ +export interface IAccountBlobServiceConfig { + /** + * Whether blob versioning is enabled for the account. + * + * Equivalent to the ARM property + * Microsoft.Storage/storageAccounts/blobServices/default#isVersioningEnabled + */ + isVersioningEnabled: boolean; +} + +/** + * Configuration for a single storage account. + */ +export interface IAccountConfig { + /** + * Storage account name, for example "devstoreaccount1". + */ + name: string; + + blobService: IAccountBlobServiceConfig; +} + +/** + * Root of the account configuration document. + */ +export interface IAccountModel { + accounts: IAccountConfig[]; +} + +export const DEFAULT_ACCOUNT_BLOB_SERVICE_CONFIG: IAccountBlobServiceConfig = { + isVersioningEnabled: false +}; + +/** + * Thrown when the supplied account configuration cannot be used. Surfaced to the user + * as a start up failure rather than a request failure. + */ +export class AccountConfigError extends Error { + public constructor(message: string) { + super(message); + this.name = "AccountConfigError"; + } +} + +/** + * Parse and validate an account configuration document. + * + * Accepts either the full document shape: + * { "accounts": [ { "name": "devstoreaccount1", + * "blobService": { "isVersioningEnabled": true } } ] } + * + * or the shorthand where a single account object is supplied directly: + * { "name": "devstoreaccount1", "blobService": { "isVersioningEnabled": true } } + * + * @param raw JSON text + * @param source Description of where the JSON came from, used in error messages + */ +export function parseAccountModel(raw: string, source: string): IAccountModel { + let parsed: any; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new AccountConfigError( + `Account configuration from ${source} is not valid JSON: ${ + (err as Error).message + }` + ); + } + + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new AccountConfigError( + `Account configuration from ${source} must be a JSON object.` + ); + } + + const rawAccounts = parsed.accounts !== undefined ? parsed.accounts : [parsed]; + + if (!Array.isArray(rawAccounts)) { + throw new AccountConfigError( + `Account configuration from ${source} must have an "accounts" array.` + ); + } + + if (rawAccounts.length === 0) { + throw new AccountConfigError( + `Account configuration from ${source} must configure at least one account.` + ); + } + + const accounts: IAccountConfig[] = []; + const seen = new Set(); + + for (const rawAccount of rawAccounts) { + if ( + rawAccount === null || + typeof rawAccount !== "object" || + Array.isArray(rawAccount) + ) { + throw new AccountConfigError( + `Account configuration from ${source} contains an account entry that is not a JSON object.` + ); + } + + if (typeof rawAccount.name !== "string" || rawAccount.name === "") { + throw new AccountConfigError( + `Account configuration from ${source} contains an account without a non-empty "name".` + ); + } + + // Account names are case insensitive in Azure Storage, and Azurite resolves them + // in lower case, so normalize here to keep lookups predictable. + const name = rawAccount.name.toLowerCase(); + + if (seen.has(name)) { + throw new AccountConfigError( + `Account configuration from ${source} configures account "${name}" more than once.` + ); + } + seen.add(name); + + const blobService = rawAccount.blobService ?? {}; + if ( + blobService === null || + typeof blobService !== "object" || + Array.isArray(blobService) + ) { + throw new AccountConfigError( + `Account "${name}" has a "blobService" value that is not a JSON object.` + ); + } + + const isVersioningEnabled = + blobService.isVersioningEnabled ?? + DEFAULT_ACCOUNT_BLOB_SERVICE_CONFIG.isVersioningEnabled; + + if (typeof isVersioningEnabled !== "boolean") { + throw new AccountConfigError( + `Account "${name}" has a non-boolean "blobService.isVersioningEnabled" value.` + ); + } + + accounts.push({ name, blobService: { isVersioningEnabled } }); + } + + return { accounts }; +} + +/** + * Look up the blob service configuration for an account, falling back to the + * defaults when the account is not configured. + */ +export function getAccountBlobServiceConfig( + model: IAccountModel | undefined, + account: string +): IAccountBlobServiceConfig { + if (model === undefined) { + return DEFAULT_ACCOUNT_BLOB_SERVICE_CONFIG; + } + + const match = model.accounts.find( + (candidate) => candidate.name === account.toLowerCase() + ); + + return match === undefined + ? DEFAULT_ACCOUNT_BLOB_SERVICE_CONFIG + : match.blobService; +} diff --git a/src/common/Environment.ts b/src/common/Environment.ts index 44c25c237..6a241de1b 100644 --- a/src/common/Environment.ts +++ b/src/common/Environment.ts @@ -18,6 +18,8 @@ import { DEFAULT_TABLE_SERVER_HOST_NAME } from "../table/utils/constants"; +import { IAccountModel } from "./AccountModel"; +import { resolveAccountModel } from "./EnvironmentFunctions"; import IEnvironment from "./IEnvironment"; import { shouldSkipApiVersionCheck } from "./utils/environment"; @@ -111,6 +113,14 @@ args .option( ["", "disableTelemetry"], "Optional. Disable telemtry collection of Azurite. If not specify this parameter Azurite will collect telemetry data by default." + ) + .option( + ["", "accountConfigFile"], + "Optional. Path to a JSON file with account level (management plane) configuration, for example to enable blob versioning. Mutually exclusive with --accountConfig" + ) + .option( + ["", "accountConfig"], + "Optional. Inline JSON string with account level (management plane) configuration, for example to enable blob versioning. Mutually exclusive with --accountConfigFile" ); (args as any).config.name = "azurite"; @@ -223,6 +233,13 @@ export default class Environment implements IEnvironment { return this.flags.extentMemoryLimit; } + public async accountModel(): Promise { + return resolveAccountModel( + this.flags.accountConfigFile, + this.flags.accountConfig + ); + } + public disableTelemetry(): boolean { if (this.flags.disableTelemetry !== undefined) { return true; diff --git a/src/common/EnvironmentFunctions.ts b/src/common/EnvironmentFunctions.ts new file mode 100644 index 000000000..c5bfd395d --- /dev/null +++ b/src/common/EnvironmentFunctions.ts @@ -0,0 +1,67 @@ +import { readFile } from "fs-extra"; + +import { + AccountConfigError, + IAccountModel, + parseAccountModel +} from "./AccountModel"; + +/** + * Shared helpers for reading environment/command line options that are used by more + * than one Azurite entry point (azurite, azurite-blob, and the VS Code extension). + */ + +/** + * Resolve the account (management plane) configuration from the --accountConfigFile + * and --accountConfig options. + * + * The two options are mutually exclusive. When neither is supplied, undefined is + * returned and Azurite falls back to the configuration persisted from the previous + * run, or to the defaults when there is none. + * + * @param accountConfigFile Value of the --accountConfigFile option + * @param accountConfig Value of the --accountConfig option + */ +export async function resolveAccountModel( + accountConfigFile: string | undefined, + accountConfig: string | undefined +): Promise { + if (accountConfigFile !== undefined && accountConfig !== undefined) { + throw new AccountConfigError( + `The --accountConfigFile and --accountConfig options are mutually exclusive, please provide only one of them.` + ); + } + + if (accountConfigFile !== undefined) { + if (typeof accountConfigFile !== "string" || accountConfigFile === "") { + throw new AccountConfigError( + `Must provide a file path for the --accountConfigFile option.` + ); + } + + let raw: string; + try { + raw = await readFile(accountConfigFile, "utf8"); + } catch (err) { + throw new AccountConfigError( + `Could not read the account configuration file "${accountConfigFile}": ${ + (err as Error).message + }` + ); + } + + return parseAccountModel(raw, `file "${accountConfigFile}"`); + } + + if (accountConfig !== undefined) { + if (typeof accountConfig !== "string" || accountConfig === "") { + throw new AccountConfigError( + `Must provide a JSON string for the --accountConfig option.` + ); + } + + return parseAccountModel(accountConfig, "the --accountConfig option"); + } + + return undefined; +} diff --git a/src/common/VSCEnvironment.ts b/src/common/VSCEnvironment.ts index 0bcff08f5..02037fce0 100644 --- a/src/common/VSCEnvironment.ts +++ b/src/common/VSCEnvironment.ts @@ -2,6 +2,8 @@ import { access, ensureDir } from "fs-extra"; import { isAbsolute, resolve } from "path"; import { window, workspace, WorkspaceFolder } from "vscode"; +import { IAccountModel } from "./AccountModel"; +import { resolveAccountModel } from "./EnvironmentFunctions"; import IEnvironment from "./IEnvironment"; export default class VSCEnvironment implements IEnvironment { @@ -130,6 +132,15 @@ export default class VSCEnvironment implements IEnvironment { return this.workspaceConfiguration.get("extentMemoryLimit"); } + public async accountModel(): Promise { + // VS Code returns null for an unset setting, normalize it so that "not configured" + // is not mistaken for an empty value. + return resolveAccountModel( + this.workspaceConfiguration.get("accountConfigFile") || undefined, + this.workspaceConfiguration.get("accountConfig") || undefined + ); + } + public disableTelemetry(): boolean { return ( this.workspaceConfiguration.get("disableTelemetry") || false diff --git a/tests/BlobTestServerFactory.ts b/tests/BlobTestServerFactory.ts index 0867b07cb..4b5bd747b 100644 --- a/tests/BlobTestServerFactory.ts +++ b/tests/BlobTestServerFactory.ts @@ -5,6 +5,7 @@ import SqlBlobServer from "../src/blob/SqlBlobServer"; import { StoreDestinationArray } from "../src/common/persistence/IExtentStore"; import { DEFAULT_SQL_OPTIONS } from "../src/common/utils/constants"; import { DEFAULT_BLOB_KEEP_ALIVE_TIMEOUT } from "../src/blob/utils/constants"; +import { IAccountModel } from "../src/common/AccountModel"; import { LIVE_TEST_MODE } from "./testutils"; /** @@ -24,7 +25,8 @@ export default class BlobTestServerFactory { loose: boolean = false, skipApiVersionCheck: boolean = false, https: boolean = false, - oauth?: string + oauth?: string, + accountModel?: IAccountModel ): BlobServer | SqlBlobServer | LiveModeStubServer { if (LIVE_TEST_MODE) { return new LiveModeStubServer(); @@ -49,6 +51,14 @@ export default class BlobTestServerFactory { if (inMemoryPersistence) { throw new Error(`The in-memory persistence settings is not supported when using SQL-based metadata.`) } + if ( + accountModel !== undefined && + accountModel.accounts.some( + (account) => account.blobService.isVersioningEnabled + ) + ) { + throw new Error(`Blob versioning is not supported when using SQL-based metadata.`) + } const config = new SqlBlobConfiguration( host, @@ -72,8 +82,12 @@ export default class BlobTestServerFactory { return new SqlBlobServer(config); } else { - const lokiMetadataDBPath = "__test_db_blob__.json"; - const lokiExtentDBPath = "__test_db_blob_extent__.json"; + // Blob versioning cannot be switched on or off against an existing workspace, so + // suites that configure it need their own metadata DB. + const suffix = + accountModel !== undefined ? `_${accountModel.accounts.map((a) => `${a.name}-${a.blobService.isVersioningEnabled}`).join("_")}` : ""; + const lokiMetadataDBPath = `__test_db_blob${suffix}__.json`; + const lokiExtentDBPath = `__test_db_blob_extent${suffix}__.json`; const config = new BlobConfiguration( host, port, @@ -92,7 +106,9 @@ export default class BlobTestServerFactory { undefined, oauth, undefined, - inMemoryPersistence + inMemoryPersistence, + undefined, + accountModel ); return new BlobServer(config); } diff --git a/tests/blob/apis/blob.versioning.test.ts b/tests/blob/apis/blob.versioning.test.ts new file mode 100644 index 000000000..3a647cf8a --- /dev/null +++ b/tests/blob/apis/blob.versioning.test.ts @@ -0,0 +1,398 @@ +import { + BlobServiceClient, + newPipeline, + StorageSharedKeyCredential +} from "@azure/storage-blob"; +import * as assert from "assert"; + +import { IAccountModel } from "../../../src/common/AccountModel"; +import { configLogger } from "../../../src/common/Logger"; +import BlobTestServerFactory from "../../BlobTestServerFactory"; +import { + bodyToString, + EMULATOR_ACCOUNT_KEY, + EMULATOR_ACCOUNT_NAME, + getTestServerBaseURL, + getUniqueName +} from "../../testutils"; + +// Set true to enable debug log +configLogger(false); + +const VERSIONING_ENABLED_ACCOUNT_MODEL: IAccountModel = { + accounts: [ + { + name: EMULATOR_ACCOUNT_NAME, + blobService: { isVersioningEnabled: true } + } + ] +}; + +describe("BlobVersioningAPIs", () => { + const factory = new BlobTestServerFactory(); + const server = factory.createServer( + false, + false, + false, + undefined, + VERSIONING_ENABLED_ACCOUNT_MODEL + ); + + const baseURL = getTestServerBaseURL(server); + const serviceClient = new BlobServiceClient( + baseURL, + newPipeline( + new StorageSharedKeyCredential( + EMULATOR_ACCOUNT_NAME, + EMULATOR_ACCOUNT_KEY + ), + { + retryOptions: { maxTries: 1 }, + keepAliveOptions: { enable: false } + } + ) + ); + + let containerName: string = getUniqueName("container"); + let containerClient = serviceClient.getContainerClient(containerName); + let blobName: string = getUniqueName("blob"); + let blockBlobClient = containerClient.getBlockBlobClient(blobName); + + before(async () => { + await server.start(); + }); + + after(async () => { + await server.close(); + await server.clean(); + }); + + beforeEach(async () => { + containerName = getUniqueName("container"); + containerClient = serviceClient.getContainerClient(containerName); + await containerClient.create(); + blobName = getUniqueName("blob"); + blockBlobClient = containerClient.getBlockBlobClient(blobName); + }); + + afterEach(async () => { + await containerClient.delete(); + }); + + /** + * List every version of a blob, oldest first. + */ + async function listVersions(name: string) { + const items = []; + for await (const item of containerClient.listBlobsFlat({ + includeVersions: true + })) { + if (item.name === name) { + items.push(item); + } + } + return items; + } + + it("Upload should return a version ID @loki", async () => { + const upload = await blockBlobClient.upload("version1", 8); + + assert.notStrictEqual( + upload.versionId, + undefined, + "Expected x-ms-version-id on the upload response" + ); + // RFC 3339 with 7 digit fractional seconds, as Azure returns + assert.ok( + /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{7}Z$/.test(upload.versionId!), + `Unexpected version ID format: ${upload.versionId}` + ); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 1); + assert.strictEqual(versions[0].versionId, upload.versionId); + assert.strictEqual(versions[0].isCurrentVersion, true); + }); + + it("Overwrite should preserve the previous content as a version @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + const second = await blockBlobClient.upload("version2", 8); + + assert.notStrictEqual(first.versionId, second.versionId); + + // The current version has the new content + const current = await blockBlobClient.download(); + assert.strictEqual(await bodyToString(current, 8), "version2"); + + // The previous version still has the old content + const previous = await blockBlobClient + .withVersion(first.versionId!) + .download(); + assert.strictEqual(await bodyToString(previous, 8), "version1"); + + // Both versions are listed, oldest first, and exactly one is current + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 2); + assert.strictEqual(versions[0].versionId, first.versionId); + assert.strictEqual(versions[1].versionId, second.versionId); + assert.notStrictEqual(versions[0].isCurrentVersion, true); + assert.strictEqual(versions[1].isCurrentVersion, true); + }); + + it("List blobs should hide previous versions unless requested @loki", async () => { + await blockBlobClient.upload("version1", 8); + await blockBlobClient.upload("version2", 8); + + const withoutVersions = []; + for await (const item of containerClient.listBlobsFlat()) { + withoutVersions.push(item); + } + + assert.strictEqual( + withoutVersions.length, + 1, + "Only the current version should be listed by default" + ); + assert.strictEqual(withoutVersions[0].name, blobName); + + const withVersions = await listVersions(blobName); + assert.strictEqual(withVersions.length, 2); + }); + + it("Get properties should report the current version @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + const second = await blockBlobClient.upload("version2", 8); + + // Without a version ID the request addresses the current version + const current = await blockBlobClient.getProperties(); + assert.strictEqual(current.versionId, second.versionId); + assert.strictEqual(current.isCurrentVersion, true); + + // Explicitly addressing the current version behaves the same + const currentByVersion = await blockBlobClient + .withVersion(second.versionId!) + .getProperties(); + assert.strictEqual(currentByVersion.versionId, second.versionId); + assert.strictEqual(currentByVersion.isCurrentVersion, true); + + // x-ms-is-current-version is absent for a previous version + const previous = await blockBlobClient + .withVersion(first.versionId!) + .getProperties(); + assert.strictEqual(previous.versionId, first.versionId); + assert.notStrictEqual(previous.isCurrentVersion, true); + }); + + it("Download of an unknown version should fail with 404 @loki", async () => { + await blockBlobClient.upload("version1", 8); + + let error; + try { + await blockBlobClient + .withVersion("2020-01-01T00:00:00.0000000Z") + .download(); + } catch (err) { + error = err; + } + + assert.notStrictEqual(error, undefined); + assert.strictEqual((error as any).statusCode, 404); + }); + + it("Deleting a single version should leave the others intact @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + const second = await blockBlobClient.upload("version2", 8); + + await blockBlobClient.withVersion(first.versionId!).delete(); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 1); + assert.strictEqual(versions[0].versionId, second.versionId); + + // The current version is still readable + const current = await blockBlobClient.download(); + assert.strictEqual(await bodyToString(current, 8), "version2"); + }); + + it("Deleting the current version should retain previous versions @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + await blockBlobClient.upload("version2", 8); + + // Deleting the blob does not fail even though previous versions exist, and it does + // not remove them. This differs from snapshots, which return SnapshotsPresent. + await blockBlobClient.delete(); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 1); + assert.strictEqual(versions[0].versionId, first.versionId); + assert.notStrictEqual(versions[0].isCurrentVersion, true); + + // The blob itself is gone + assert.strictEqual(await blockBlobClient.exists(), false); + + // But the previous version is still readable by version ID + const previous = await blockBlobClient + .withVersion(first.versionId!) + .download(); + assert.strictEqual(await bodyToString(previous, 8), "version1"); + }); + + it("Restore should be a copy of a previous version over the current one @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + await blockBlobClient.upload("version2", 8); + + const sourceUrl = blockBlobClient.withVersion(first.versionId!).url; + const poller = await blockBlobClient.beginCopyFromURL(sourceUrl); + await poller.pollUntilDone(); + + const restored = await blockBlobClient.download(); + assert.strictEqual(await bodyToString(restored, 8), "version1"); + }); + + it("Snapshot and version ID together should fail with 400 @loki", async () => { + const upload = await blockBlobClient.upload("version1", 8); + const snapshot = await blockBlobClient.createSnapshot(); + + // withSnapshot()/withVersion() both rewrite the URL, so chaining them produces a + // request carrying the snapshot and versionid query parameters at the same time. + const both = blockBlobClient + .withSnapshot(snapshot.snapshot!) + .withVersion(upload.versionId!); + + let error; + try { + await both.download(); + } catch (err) { + error = err; + } + + assert.notStrictEqual( + error, + undefined, + "Expected the snapshot and versionid combination to be rejected" + ); + assert.strictEqual((error as any).statusCode, 400); + assert.strictEqual( + (error as any).code, + "InvalidQueryParameterValue", + "Error code should match the real service" + ); + }); + + it("Commit block list should create a version @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + + const blockId = Buffer.from("block-1").toString("base64"); + await blockBlobClient.stageBlock(blockId, "version2", 8); + const commit = await blockBlobClient.commitBlockList([blockId]); + + assert.notStrictEqual(commit.versionId, undefined); + assert.notStrictEqual(commit.versionId, first.versionId); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 2); + assert.strictEqual(versions[0].versionId, first.versionId); + assert.strictEqual(versions[1].versionId, commit.versionId); + assert.strictEqual(versions[1].isCurrentVersion, true); + + const previous = await blockBlobClient + .withVersion(first.versionId!) + .download(); + assert.strictEqual(await bodyToString(previous, 8), "version1"); + }); + + it("Snapshots should still block deleting the base blob @loki", async () => { + await blockBlobClient.upload("version1", 8); + await blockBlobClient.createSnapshot(); + + let error; + try { + await blockBlobClient.delete(); + } catch (err) { + error = err; + } + + assert.notStrictEqual( + error, + undefined, + "Expected SnapshotsPresent when snapshots exist" + ); + assert.strictEqual((error as any).statusCode, 409); + }); +}); + +describe("BlobVersioningDisabledAPIs", () => { + const factory = new BlobTestServerFactory(); + const server = factory.createServer(); + + const baseURL = getTestServerBaseURL(server); + const serviceClient = new BlobServiceClient( + baseURL, + newPipeline( + new StorageSharedKeyCredential( + EMULATOR_ACCOUNT_NAME, + EMULATOR_ACCOUNT_KEY + ), + { + retryOptions: { maxTries: 1 }, + keepAliveOptions: { enable: false } + } + ) + ); + + let containerName: string = getUniqueName("container"); + let containerClient = serviceClient.getContainerClient(containerName); + let blobName: string = getUniqueName("blob"); + let blockBlobClient = containerClient.getBlockBlobClient(blobName); + + before(async () => { + await server.start(); + }); + + after(async () => { + await server.close(); + await server.clean(); + }); + + beforeEach(async () => { + containerName = getUniqueName("container"); + containerClient = serviceClient.getContainerClient(containerName); + await containerClient.create(); + blobName = getUniqueName("blob"); + blockBlobClient = containerClient.getBlockBlobClient(blobName); + }); + + afterEach(async () => { + await containerClient.delete(); + }); + + it("Upload should not return a version ID when versioning is disabled @loki", async () => { + const upload = await blockBlobClient.upload("version1", 8); + assert.strictEqual(upload.versionId, undefined); + }); + + it("Overwrite should replace the blob when versioning is disabled @loki", async () => { + await blockBlobClient.upload("version1", 8); + await blockBlobClient.upload("version2", 8); + + const items = []; + for await (const item of containerClient.listBlobsFlat({ + includeVersions: true + })) { + items.push(item); + } + + assert.strictEqual(items.length, 1, "No versions should be retained"); + assert.strictEqual(items[0].versionId, undefined); + + const current = await blockBlobClient.download(); + assert.strictEqual(await bodyToString(current, 8), "version2"); + }); + + it("Get properties should not report version information @loki", async () => { + await blockBlobClient.upload("version1", 8); + const properties = await blockBlobClient.getProperties(); + assert.strictEqual(properties.versionId, undefined); + assert.strictEqual(properties.isCurrentVersion, undefined); + }); +}); diff --git a/tests/common/AccountModel.test.ts b/tests/common/AccountModel.test.ts new file mode 100644 index 000000000..f32214f2e --- /dev/null +++ b/tests/common/AccountModel.test.ts @@ -0,0 +1,162 @@ +import * as assert from "assert"; + +import { + AccountConfigError, + getAccountBlobServiceConfig, + parseAccountModel +} from "../../src/common/AccountModel"; +import { resolveAccountModel } from "../../src/common/EnvironmentFunctions"; + +describe("AccountModel @loki", () => { + it("parses the full document shape", () => { + const model = parseAccountModel( + JSON.stringify({ + accounts: [ + { + name: "devstoreaccount1", + blobService: { isVersioningEnabled: true } + }, + { + name: "devstoreaccount2", + blobService: { isVersioningEnabled: false } + } + ] + }), + "test" + ); + + assert.strictEqual(model.accounts.length, 2); + assert.strictEqual( + model.accounts[0].blobService.isVersioningEnabled, + true + ); + assert.strictEqual( + model.accounts[1].blobService.isVersioningEnabled, + false + ); + }); + + it("parses the single account shorthand", () => { + const model = parseAccountModel( + JSON.stringify({ + name: "devstoreaccount1", + blobService: { isVersioningEnabled: true } + }), + "test" + ); + + assert.strictEqual(model.accounts.length, 1); + assert.strictEqual(model.accounts[0].name, "devstoreaccount1"); + assert.strictEqual(model.accounts[0].blobService.isVersioningEnabled, true); + }); + + it("normalizes account names to lower case", () => { + const model = parseAccountModel( + JSON.stringify({ name: "DevStoreAccount1" }), + "test" + ); + assert.strictEqual(model.accounts[0].name, "devstoreaccount1"); + }); + + it("defaults versioning to disabled when blobService is omitted", () => { + const model = parseAccountModel( + JSON.stringify({ name: "devstoreaccount1" }), + "test" + ); + assert.strictEqual(model.accounts[0].blobService.isVersioningEnabled, false); + }); + + it("rejects malformed JSON", () => { + assert.throws( + () => parseAccountModel("{not json", "test"), + AccountConfigError + ); + }); + + it("rejects an empty accounts array", () => { + assert.throws( + () => parseAccountModel(JSON.stringify({ accounts: [] }), "test"), + AccountConfigError + ); + }); + + it("rejects an account without a name", () => { + assert.throws( + () => + parseAccountModel( + JSON.stringify({ accounts: [{ blobService: {} }] }), + "test" + ), + AccountConfigError + ); + }); + + it("rejects duplicate account names", () => { + assert.throws( + () => + parseAccountModel( + JSON.stringify({ + accounts: [{ name: "a" }, { name: "A" }] + }), + "test" + ), + AccountConfigError + ); + }); + + it("rejects a non-boolean isVersioningEnabled", () => { + assert.throws( + () => + parseAccountModel( + JSON.stringify({ + name: "a", + blobService: { isVersioningEnabled: "yes" } + }), + "test" + ), + AccountConfigError + ); + }); + + it("falls back to defaults for unconfigured accounts", () => { + const model = parseAccountModel( + JSON.stringify({ name: "a", blobService: { isVersioningEnabled: true } }), + "test" + ); + + assert.strictEqual( + getAccountBlobServiceConfig(model, "a").isVersioningEnabled, + true + ); + assert.strictEqual( + getAccountBlobServiceConfig(model, "A").isVersioningEnabled, + true + ); + assert.strictEqual( + getAccountBlobServiceConfig(model, "other").isVersioningEnabled, + false + ); + assert.strictEqual( + getAccountBlobServiceConfig(undefined, "a").isVersioningEnabled, + false + ); + }); + + it("rejects supplying both --accountConfigFile and --accountConfig", async () => { + await assert.rejects( + () => resolveAccountModel("some/path.json", "{}"), + AccountConfigError + ); + }); + + it("returns undefined when neither option is supplied", async () => { + assert.strictEqual(await resolveAccountModel(undefined, undefined), undefined); + }); + + it("reports an unreadable account configuration file", async () => { + await assert.rejects( + () => resolveAccountModel("does/not/exist.json", undefined), + AccountConfigError + ); + }); +}); From df167ff8718c58e4c68a1a7c1544e23b03b8d2ea Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 08:34:54 +0100 Subject: [PATCH 02/10] Fix List Blobs pagination across versions of one blob The list continuation token was the blob name alone, which cannot express "resume from version N of blob X". Because listBlobs filtered with `obj.name > marker`, following a continuation token skipped every remaining version of the blob the previous page stopped inside. Reproduced with five versions of a single blob and maxPageSize 2: the listing returned 2 of 5 versions across 2 pages and then stopped, with no error - a silently short result. PageWithDelimiter now tracks a [name, secondaryKey] page key, where the secondary key is the version ID when listing versions and empty otherwise, and serializes it into the continuation token. A token with no secondary key keeps the historical plain blob name format, so listings that do not involve versions are unchanged and tokens issued by earlier Azurite versions stay valid. Anything not recognizable as a composite token is read as a plain blob name. Equal page keys are tolerated rather than rejected, because items sharing a name do not always carry a secondary key - snapshots do not - so snapshot listing keeps its existing behaviour. listBlobs and listAllBlobs decode the incoming token, compare items against it with the composite ordering, and sort by name then version ID then snapshot for a deterministic order. Adds three List Blobs pagination tests (versions of one blob, versions across several blobs, and a non-versioned listing to show it is unaffected) and eight unit tests for token encoding, decoding, and ordering. The pre-existing PageWithDelimiter tests pass unchanged, which is what shows the non-versioned path still behaves as before. Co-Authored-By: Claude --- ChangeLog.md | 1 + docs/designs/blob-versioning.md | 12 ++ src/blob/persistence/LokiBlobMetadataStore.ts | 55 +++++-- src/blob/persistence/PageWithDelimiter.ts | 136 ++++++++++++++++-- tests/blob/apis/blob.versioning.test.ts | 91 ++++++++++++ tests/blob/pagewithdelimiter.test.ts | 94 +++++++++++- 6 files changed, 366 insertions(+), 23 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index cf67fc82f..1dc8f1bcb 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -7,6 +7,7 @@ Blob: - Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options rather than through a data plane API. With versioning enabled for an account: overwriting a blob retains the previous content as a version and returns `x-ms-version-id`; `Get Blob` / `Get Blob Properties` accept `?versionid=` and return `x-ms-is-current-version` for the current version; `List Blobs` supports `include=versions`; `Delete Blob` with `?versionid=` removes a single version, and deleting the current version retains previous versions instead of failing with `SnapshotsPresent`; a version can be restored by copying it over the current version. Combining `?snapshot=` with `?versionid=` returns 400 `InvalidQueryParameterValue`. The setting is persisted with the workspace and cannot be changed against existing data. Versioning is not implemented for the SQL metadata store, and blob version SAS (`sr=bv`, the `x` permission), soft delete interactions, and blob expiration interactions are out of scope - see `docs/designs/blob-versioning.md`. +- Blob list continuation tokens now carry a secondary key so that a `List Blobs` page can resume part way through one blob's versions. Tokens for listings that do not involve versions keep the previous plain blob name format, so existing tokens remain valid. General: diff --git a/docs/designs/blob-versioning.md b/docs/designs/blob-versioning.md index cd3d17cff..b6ccd9aca 100644 --- a/docs/designs/blob-versioning.md +++ b/docs/designs/blob-versioning.md @@ -92,6 +92,18 @@ Account configuration lives in its own collection (`$ACCOUNTS_COLLECTION$`) rath alongside blob documents, so that the queue and table services can reuse it when they need account level settings. +### List continuation tokens + +A blob name is not a sufficient continuation token once versions exist, because every +version of a blob shares its name - resuming from a name alone would skip the remaining +versions of the blob the previous page stopped inside. Tokens therefore carry a secondary +key (the version ID) alongside the name. + +Tokens without a secondary key keep Azurite's historical format, the plain blob name, so +listings that do not involve versions are unchanged and tokens issued by earlier versions +of Azurite remain valid. Anything not recognizable as a composite token is interpreted as +a plain blob name. + ## Behaviour | Operation | Behaviour with versioning enabled | diff --git a/src/blob/persistence/LokiBlobMetadataStore.ts b/src/blob/persistence/LokiBlobMetadataStore.ts index 1fa4ff5db..48a93a82f 100644 --- a/src/blob/persistence/LokiBlobMetadataStore.ts +++ b/src/blob/persistence/LokiBlobMetadataStore.ts @@ -69,7 +69,12 @@ import IBlobMetadataStore, { ServicePropertiesModel, SetContainerAccessPolicyOptions } from "./IBlobMetadataStore"; -import PageWithDelimiter from "./PageWithDelimiter"; +import PageWithDelimiter, { + decodePageMarker, + encodePageMarker, + isAfterPageMarker, + PageItemKey +} from "./PageWithDelimiter"; import FilterBlobPage from "./FilterBlobPage"; import { generateQueryBlobWithTagsWhereFunction } from "./QueryInterpreter/QueryInterpreter"; import { @@ -1118,12 +1123,18 @@ export default class LokiBlobMetadataStore delimiter, prefix ); + // Every version of a blob shares its name, so a continuation token has to be able to + // resume part way through one blob's versions. + const decodedMarker = decodePageMarker(marker!); + const secondaryKeyOf = (item: BlobModel) => + includeVersions ? item.versionId ?? "" : ""; + const readPage = async (offset: number): Promise => { return await coll .chain() .find(query) .where((obj) => { - return obj.name > marker!; + return isAfterPageMarker([obj.name, secondaryKeyOf(obj)], decodedMarker); }) .where((obj) => { return includeSnapshots ? true : obj.snapshot.length === 0; @@ -1138,22 +1149,27 @@ export default class LokiBlobMetadataStore // Versions of the same blob are returned together, oldest first, with the // current version last. This matches the ordering List Blobs uses when // include=versions is requested. - if (obj1.name === obj2.name) { - const version1 = obj1.versionId ?? ""; - const version2 = obj2.versionId ?? ""; - if (version1 === version2) return 0; + if (obj1.name !== obj2.name) { + return obj1.name > obj2.name ? 1 : -1; + } + const version1 = obj1.versionId ?? ""; + const version2 = obj2.versionId ?? ""; + if (version1 !== version2) { return version1 > version2 ? 1 : -1; } - if (obj1.name > obj2.name) return 1; - return -1; + // Keep snapshots of the same blob in a stable order too + const snapshot1 = obj1.snapshot ?? ""; + const snapshot2 = obj2.snapshot ?? ""; + if (snapshot1 === snapshot2) return 0; + return snapshot1 > snapshot2 ? 1 : -1; }) .offset(offset) .limit(maxResults) .data(); }; - const nameItem = (item: BlobModel) => { - return item.name; + const nameItem = (item: BlobModel): PageItemKey => { + return [item.name, secondaryKeyOf(item)]; }; const [blobItems, blobPrefixes, nextMarker] = await page.fill( @@ -1185,10 +1201,14 @@ export default class LokiBlobMetadataStore ): Promise<[BlobModel[], string | undefined]> { const coll = this.db.getCollection(this.BLOBS_COLLECTION); + const decodedMarker = decodePageMarker(marker!); + const secondaryKeyOf = (item: BlobModel) => + includeVersions ? item.versionId ?? "" : ""; + const docs = await coll .chain() .where((obj) => { - return obj.name > marker!; + return isAfterPageMarker([obj.name, secondaryKeyOf(obj)], decodedMarker); }) .where((obj) => { return includeSnapshots ? true : obj.snapshot.length === 0; @@ -1199,7 +1219,15 @@ export default class LokiBlobMetadataStore .where((obj) => { return includeVersions ? true : obj.isCurrentVersion !== false; }) - .simplesort("name") + .sort((obj1, obj2) => { + if (obj1.name !== obj2.name) { + return obj1.name > obj2.name ? 1 : -1; + } + const key1 = secondaryKeyOf(obj1); + const key2 = secondaryKeyOf(obj2); + if (key1 === key2) return 0; + return key1 > key2 ? 1 : -1; + }) .limit(maxResults + 1) .data(); @@ -1213,7 +1241,8 @@ export default class LokiBlobMetadataStore if (docs.length <= maxResults) { return [docs, undefined]; } else { - const nextMarker = docs[docs.length - 2].name; + const last = docs[docs.length - 2] as BlobModel; + const nextMarker = encodePageMarker([last.name, secondaryKeyOf(last)]); docs.pop(); return [docs, nextMarker]; } diff --git a/src/blob/persistence/PageWithDelimiter.ts b/src/blob/persistence/PageWithDelimiter.ts index 05e3bf280..c6d73f413 100644 --- a/src/blob/persistence/PageWithDelimiter.ts +++ b/src/blob/persistence/PageWithDelimiter.ts @@ -1,5 +1,110 @@ import { BlobPrefixModel } from "./IBlobMetadataStore"; +/** + * The sort key of an item on a page: the blob name, plus a secondary key that + * distinguishes items sharing that name. + * + * A blob name alone is not enough to resume a listing once versions are involved, + * because every version of a blob shares its name. The secondary key is the version ID + * when listing versions, and empty otherwise. + */ +export type PageItemKey = [string, string]; + +/** + * Marker prefix identifying a continuation token that carries a secondary key. + * + * Continuation tokens are opaque to clients, but Azurite has always used the plain blob + * name, so tokens without a secondary key keep that form. That keeps listings which do + * not involve versions byte for byte unchanged, and keeps tokens issued by older Azurite + * versions readable. + */ +const COMPOSITE_MARKER_PREFIX = "2!"; + +/** + * A continuation token decoded back into its parts. + */ +export interface IDecodedPageMarker { + name: string; + secondaryKey: string; + /** + * True when the token carried a secondary key. When false the token addresses a whole + * blob name and every item sharing that name has already been returned. + */ + isComposite: boolean; +} + +/** + * Encode a page key into a continuation token. + */ +export function encodePageMarker(key: PageItemKey): string { + const [name, secondaryKey] = key; + if (secondaryKey === "") { + return name; + } + return ( + COMPOSITE_MARKER_PREFIX + + Buffer.from(JSON.stringify([name, secondaryKey]), "utf8").toString("base64") + ); +} + +/** + * Decode a continuation token supplied by a client. + * + * Anything that is not recognizable as a composite token is treated as a plain blob + * name, which is both the historical Azurite format and the safe interpretation of a + * token we did not issue. + */ +export function decodePageMarker(marker: string): IDecodedPageMarker { + if (!marker.startsWith(COMPOSITE_MARKER_PREFIX)) { + return { name: marker, secondaryKey: "", isComposite: false }; + } + + try { + const decoded = JSON.parse( + Buffer.from( + marker.slice(COMPOSITE_MARKER_PREFIX.length), + "base64" + ).toString("utf8") + ); + if ( + Array.isArray(decoded) && + decoded.length === 2 && + typeof decoded[0] === "string" && + typeof decoded[1] === "string" + ) { + return { + name: decoded[0], + secondaryKey: decoded[1], + isComposite: true + }; + } + } catch { + // Fall through and treat the token as a plain blob name + } + + return { name: marker, secondaryKey: "", isComposite: false }; +} + +/** + * Whether an item with the given key sorts after a decoded continuation token, and so + * belongs on a later page. + */ +export function isAfterPageMarker( + key: PageItemKey, + marker: IDecodedPageMarker +): boolean { + const [name, secondaryKey] = key; + if (name > marker.name) { + return true; + } + if (name < marker.name) { + return false; + } + // Same blob name. A plain token means the whole name was already returned; a composite + // token means we stopped part way through it. + return marker.isComposite ? secondaryKey > marker.secondaryKey : false; +} + /** * This implements a page of blob results taking delimiters into account. * @@ -18,7 +123,7 @@ export default class PageWithDelimiter { blobItems: BlobType[] = []; blobPrefixes: Set = new Set(); - latestMarker: string = ""; + latestMarker: PageItemKey = ["", ""]; // isFull indicates we could only (maybe) add a prefix private isFull: boolean = false; @@ -46,7 +151,7 @@ export default class PageWithDelimiter { this.blobPrefixes.clear(); this.isFull = false; this.isExhausted = false; - this.latestMarker = ""; + this.latestMarker = ["", ""]; } private updateFull() { @@ -104,14 +209,21 @@ export default class PageWithDelimiter { * * Return the number of items added */ - private add(name: string, item: BlobType): boolean { + private add(key: PageItemKey, item: BlobType): boolean { if (this.isExhausted) { return false; } - if (name < this.latestMarker) { + const [name, secondaryKey] = key; + if (name < this.latestMarker[0]) { throw new Error("add received unsorted item. add must be called on sorted data"); } - const marker = (name > this.latestMarker) ? name : this.latestMarker; + // Items sharing a name are not required to carry a secondary key - snapshots, for + // example, do not - so equal keys are tolerated and simply do not advance the marker. + const marker: PageItemKey = + name > this.latestMarker[0] || + (name === this.latestMarker[0] && secondaryKey > this.latestMarker[1]) + ? [name, secondaryKey] + : this.latestMarker; let added: boolean = false; if (this.delimiter !== undefined) { const delimiterPosAfterPrefix = name.indexOf( @@ -137,10 +249,16 @@ export default class PageWithDelimiter { /** * Iterate over an array blobs read from a source and add them until the page cannot accept new items */ - private processList(docs: BlobType[], nameFn: (item: BlobType) => string): number { + private processList( + docs: BlobType[], + nameFn: (item: BlobType) => string | PageItemKey + ): number { let added: number = 0; for (const item of docs) { - if (this.add(nameFn(item), item)) { + const named = nameFn(item); + const key: PageItemKey = + typeof named === "string" ? [named, ""] : named; + if (this.add(key, item)) { added++; } if (this.isExhausted) break; @@ -161,7 +279,7 @@ export default class PageWithDelimiter { */ public async fill( reader: (offset: number) => Promise, - namer: (item: BlobType) => string, + namer: (item: BlobType) => string | PageItemKey, ): Promise<[BlobType[], BlobPrefixModel[], string]> { let offset: number = 0; let docs = await reader(offset); @@ -177,7 +295,7 @@ export default class PageWithDelimiter { return [ this.blobItems, this.prefixes(), - added < docs.length ? this.latestMarker : "" + added < docs.length ? encodePageMarker(this.latestMarker) : "" ]; } diff --git a/tests/blob/apis/blob.versioning.test.ts b/tests/blob/apis/blob.versioning.test.ts index 3a647cf8a..c14911d81 100644 --- a/tests/blob/apis/blob.versioning.test.ts +++ b/tests/blob/apis/blob.versioning.test.ts @@ -301,6 +301,97 @@ describe("BlobVersioningAPIs", () => { assert.strictEqual(await bodyToString(previous, 8), "version1"); }); + /** + * Page through a versioned listing, following continuation tokens. + */ + async function pageThroughVersions(pageSize: number) { + const seen: string[] = []; + let continuationToken: string | undefined; + let pages = 0; + + do { + const result = await containerClient + .listBlobsFlat({ includeVersions: true }) + .byPage({ maxPageSize: pageSize, continuationToken }) + .next(); + + if (result.done) { + break; + } + + for (const item of result.value.segment.blobItems) { + seen.push(`${item.name}@${item.versionId}`); + } + continuationToken = result.value.continuationToken; + pages++; + // Guard against a token that never advances + assert.ok(pages < 50, "Listing did not terminate"); + } while (continuationToken); + + return { seen, pages }; + } + + it("Paginating versions of a single blob should return every version @loki", async () => { + const expected: string[] = []; + for (let i = 0; i < 5; i++) { + const upload = await blockBlobClient.upload(`v${i}`, 2); + expected.push(`${blobName}@${upload.versionId}`); + } + + // A page size smaller than the number of versions forces the continuation token to + // resume part way through one blob's versions. + const { seen, pages } = await pageThroughVersions(2); + + assert.ok(pages > 1, `Expected more than one page, got ${pages}`); + assert.deepStrictEqual(seen, expected); + }); + + it("Paginating versions across several blobs should return every version @loki", async () => { + const expected: string[] = []; + // Names are chosen so that lexical order is deterministic + for (const suffix of ["a", "b", "c"]) { + const name = `${blobName}-${suffix}`; + const client = containerClient.getBlockBlobClient(name); + for (let i = 0; i < 3; i++) { + const upload = await client.upload(`v${i}`, 2); + expected.push(`${name}@${upload.versionId}`); + } + } + + const { seen, pages } = await pageThroughVersions(2); + + assert.ok(pages > 1, `Expected more than one page, got ${pages}`); + assert.deepStrictEqual(seen, expected); + }); + + it("Paginating without versions should be unaffected @loki", async () => { + const names: string[] = []; + for (const suffix of ["a", "b", "c"]) { + const name = `${blobName}-${suffix}`; + const client = containerClient.getBlockBlobClient(name); + await client.upload("v0", 2); + await client.upload("v1", 2); + names.push(name); + } + + const seen: string[] = []; + let continuationToken: string | undefined; + do { + const result = await containerClient + .listBlobsFlat() + .byPage({ maxPageSize: 2, continuationToken }) + .next(); + if (result.done) break; + for (const item of result.value.segment.blobItems) { + seen.push(item.name); + } + continuationToken = result.value.continuationToken; + } while (continuationToken); + + // Only current versions, each blob exactly once + assert.deepStrictEqual(seen, names); + }); + it("Snapshots should still block deleting the base blob @loki", async () => { await blockBlobClient.upload("version1", 8); await blockBlobClient.createSnapshot(); diff --git a/tests/blob/pagewithdelimiter.test.ts b/tests/blob/pagewithdelimiter.test.ts index c488ab8de..143e07b75 100644 --- a/tests/blob/pagewithdelimiter.test.ts +++ b/tests/blob/pagewithdelimiter.test.ts @@ -1,6 +1,10 @@ import * as assert from "assert"; import { BlobPrefixModel } from "../../src/blob/persistence/IBlobMetadataStore"; -import PageWithDelimiter from "../../src/blob/persistence/PageWithDelimiter"; +import PageWithDelimiter, { + decodePageMarker, + encodePageMarker, + isAfterPageMarker +} from "../../src/blob/persistence/PageWithDelimiter"; describe("PageWithDelimiter", () => { function checkResult( @@ -149,3 +153,91 @@ describe("PageWithDelimiter", () => { }); }); }); + +describe("PageWithDelimiter continuation tokens @loki", () => { + it("encodes a key without a secondary key as the plain blob name", () => { + // Preserves the historical Azurite token format for listings that do not involve + // versions, so tokens stay compatible in both directions. + assert.strictEqual(encodePageMarker(["blob1", ""]), "blob1"); + }); + + it("round trips a key with a secondary key", () => { + const encoded = encodePageMarker(["blob1", "2026-08-13T10:00:00.0000000Z"]); + assert.notStrictEqual(encoded, "blob1"); + + const decoded = decodePageMarker(encoded); + assert.strictEqual(decoded.name, "blob1"); + assert.strictEqual(decoded.secondaryKey, "2026-08-13T10:00:00.0000000Z"); + assert.strictEqual(decoded.isComposite, true); + }); + + it("round trips names containing awkward characters", () => { + for (const name of ["a/b c", "a!b", 'quote"name', "2!notatoken", "üñí"]) { + const decoded = decodePageMarker(encodePageMarker([name, "key"])); + assert.strictEqual(decoded.name, name); + assert.strictEqual(decoded.secondaryKey, "key"); + } + }); + + it("treats an unrecognized token as a plain blob name", () => { + for (const token of ["", "blob1", "2!", "2!not-base64!!", "2!" + Buffer.from('"x"').toString("base64")]) { + const decoded = decodePageMarker(token); + assert.strictEqual(decoded.name, token); + assert.strictEqual(decoded.secondaryKey, ""); + assert.strictEqual(decoded.isComposite, false); + } + }); + + it("orders items against a plain token by name only", () => { + const marker = decodePageMarker("blob2"); + assert.strictEqual(isAfterPageMarker(["blob1", ""], marker), false); + // A plain token means every item sharing the name was already returned + assert.strictEqual(isAfterPageMarker(["blob2", ""], marker), false); + assert.strictEqual(isAfterPageMarker(["blob2", "zzz"], marker), false); + assert.strictEqual(isAfterPageMarker(["blob3", ""], marker), true); + }); + + it("orders items against a composite token by name then secondary key", () => { + const marker = decodePageMarker(encodePageMarker(["blob2", "v2"])); + assert.strictEqual(isAfterPageMarker(["blob1", "v9"], marker), false); + assert.strictEqual(isAfterPageMarker(["blob2", "v1"], marker), false); + assert.strictEqual(isAfterPageMarker(["blob2", "v2"], marker), false); + assert.strictEqual(isAfterPageMarker(["blob2", "v3"], marker), true); + assert.strictEqual(isAfterPageMarker(["blob3", ""], marker), true); + }); + + it("emits a composite token when a page stops part way through one name", () => { + // Three versions of one blob, page size 2 + const versions: [string, string][] = [ + ["blob1", "v1"], + ["blob1", "v2"], + ["blob1", "v3"] + ]; + const page = new PageWithDelimiter<[string, string]>(2); + const reader = (o: number) => Promise.resolve(versions.slice(o, o + 2)); + + return page.fill(reader, (item) => item).then(([items, , marker]) => { + assert.strictEqual(items.length, 2); + const decoded = decodePageMarker(marker); + assert.strictEqual(decoded.isComposite, true); + assert.strictEqual(decoded.name, "blob1"); + assert.strictEqual(decoded.secondaryKey, "v2"); + }); + }); + + it("tolerates repeated keys without advancing the marker", () => { + // Snapshots share a blob name and carry no secondary key + const docs: [string, string][] = [ + ["blob1", ""], + ["blob1", ""], + ["blob2", ""] + ]; + const page = new PageWithDelimiter<[string, string]>(2); + const reader = (o: number) => Promise.resolve(docs.slice(o, o + 2)); + + return page.fill(reader, (item) => item).then(([items, , marker]) => { + assert.strictEqual(items.length, 2); + assert.strictEqual(marker, "blob1"); + }); + }); +}); From 2d960a9a545760e2ce2d8b7e1c7a93964f348b5c Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 08:57:22 +0100 Subject: [PATCH 03/10] Fix Delete Blob destroying the current version's content Deleting a versioned blob without a version ID removed the current version's document outright, so the content that was current at delete time was lost permanently. Only the older versions survived. Per the blob versioning documentation: "When you call the Delete Blob operation without specifying a version ID, the current version becomes a previous version, and there's no longer a current version. All existing previous versions of the blob are preserved." deleteBlob now demotes the current version to a previous version instead of removing it, for both the plain delete and the deleteSnapshots=include cases. An explicit delete of a single version by ID is unchanged and still removes that version, and the non-versioned path still removes the blob outright. Because the demoted document keeps isCurrentVersion false, reads and listings that do not name a version already treat the blob as absent: Get Blob returns 404 and List Blobs omits it, while List Blobs with include=versions returns every version with none marked current. Writing to the blob afterwards creates a new current version and leaves the existing versions alone. Also reports HasVersionsOnly on listed versions of a blob that has versions but no current version, which is the documented signal for that state. The previous test for this case asserted the buggy behaviour - that one version survived a delete - so it passed while the content was being lost. It has been replaced with assertions that both versions survive, that neither is current, that the version which was current is still readable by ID, and that HasVersionsOnly is set. Adds cases for writing after a delete, for deleteSnapshots=include retaining versions while removing snapshots, and for a single-version delete still removing the blob. Co-Authored-By: Claude --- ChangeLog.md | 1 + docs/designs/blob-versioning.md | 3 +- src/blob/persistence/LokiBlobMetadataStore.ts | 71 +++++++++--- tests/blob/apis/blob.versioning.test.ts | 106 +++++++++++++++++- 4 files changed, 160 insertions(+), 21 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 1dc8f1bcb..7580f9b78 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -7,6 +7,7 @@ Blob: - Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options rather than through a data plane API. With versioning enabled for an account: overwriting a blob retains the previous content as a version and returns `x-ms-version-id`; `Get Blob` / `Get Blob Properties` accept `?versionid=` and return `x-ms-is-current-version` for the current version; `List Blobs` supports `include=versions`; `Delete Blob` with `?versionid=` removes a single version, and deleting the current version retains previous versions instead of failing with `SnapshotsPresent`; a version can be restored by copying it over the current version. Combining `?snapshot=` with `?versionid=` returns 400 `InvalidQueryParameterValue`. The setting is persisted with the workspace and cannot be changed against existing data. Versioning is not implemented for the SQL metadata store, and blob version SAS (`sr=bv`, the `x` permission), soft delete interactions, and blob expiration interactions are out of scope - see `docs/designs/blob-versioning.md`. +- `Delete Blob` on a versioned blob without a version ID now turns the current version into a previous version and retains it, rather than removing it, and reports `HasVersionsOnly` for a blob that has versions but no current version. - Blob list continuation tokens now carry a secondary key so that a `List Blobs` page can resume part way through one blob's versions. Tokens for listings that do not involve versions keep the previous plain blob name format, so existing tokens remain valid. General: diff --git a/docs/designs/blob-versioning.md b/docs/designs/blob-versioning.md index b6ccd9aca..c5ff0448f 100644 --- a/docs/designs/blob-versioning.md +++ b/docs/designs/blob-versioning.md @@ -112,8 +112,9 @@ a plain blob name. | Get Blob, Get Blob Properties | `?versionid=` addresses one version; without it the current version is addressed. `x-ms-is-current-version: true` is returned only for the current version | | List Blobs | `include=versions` returns previous versions with `VersionId` and `IsCurrentVersion`; they are hidden otherwise. Versions of the same blob are ordered oldest first, current last | | Delete Blob with `?versionid=` | Deletes just that version; `x-ms-delete-snapshots` cannot be combined with it | -| Delete Blob without `?versionid=` | Deletes the current version and leaves previous versions in place - it does **not** fail with `SnapshotsPresent` because versions exist | +| Delete Blob without `?versionid=` | The current version becomes a previous version and is retained, and the blob has no current version. Previous versions persist, and it does **not** fail with `SnapshotsPresent` because versions exist. `HasVersionsOnly` is reported for a blob in that state | | Snapshots | Continue to work as before, and still block deleting the base blob with `SnapshotsPresent`. Using versioning and snapshots together is supported but, as in production, not recommended | +| Write after Delete Blob | Creates a new current version; existing versions are unaffected | | Restore a version | No dedicated API: copy the version over the current version, `Copy Blob` with a `?versionid=` qualified source | | `?snapshot=` and `?versionid=` together | 400 `InvalidQueryParameterValue` | diff --git a/src/blob/persistence/LokiBlobMetadataStore.ts b/src/blob/persistence/LokiBlobMetadataStore.ts index 48a93a82f..0aa261483 100644 --- a/src/blob/persistence/LokiBlobMetadataStore.ts +++ b/src/blob/persistence/LokiBlobMetadataStore.ts @@ -1177,11 +1177,38 @@ export default class LokiBlobMetadataStore nameItem ); + // A blob whose current version has been deleted still has its previous versions. + // HasVersionsOnly flags that state, so a caller listing versions can tell the + // difference between "versions of a live blob" and "all that is left of a blob". + const hasVersionsOnlyByName = new Map(); + if (includeVersions) { + for (const doc of blobItems) { + if (hasVersionsOnlyByName.has(doc.name)) { + continue; + } + const current = coll.findOne( + this.currentVersionQuery({ + accountName: account, + containerName: container, + name: doc.name, + snapshot: "" + }) + ); + hasVersionsOnlyByName.set( + doc.name, + current === null || current === undefined + ); + } + } + return [ blobItems.map((doc) => { doc.properties.contentMD5 = this.restoreUint8Array( doc.properties.contentMD5 ); + if (hasVersionsOnlyByName.get(doc.name) === true) { + doc.hasVersionsOnly = true; + } return LeaseFactory.createLeaseState( new BlobLeaseAdapter(doc), context @@ -1687,18 +1714,40 @@ export default class LokiBlobMetadataStore snapshot: { $gt: "" } }); + const versioningEnabled = this.isVersioningEnabled(account); + + /** + * Remove the current version of the blob, or with versioning enabled retain it. + * + * Deleting a versioned blob without a version ID does not destroy the current + * version's content: "the current version of the blob becomes a previous version, + * and there's no longer a current version. Any previous versions of the blob + * persist." + */ + const deleteCurrentVersion = () => { + const currentQuery = this.currentVersionQuery({ + accountName: account, + containerName: container, + name: blob + }); + + if (!versioningEnabled) { + coll.findAndRemove(currentQuery); + return; + } + + const current = coll.findOne(currentQuery); + if (current !== null && current !== undefined) { + this.demoteToPreviousVersion(coll, current); + } + }; + // Scenario: Delete base blob only if (againstBaseBlob && options.deleteSnapshots === undefined) { if (snapshotCount > 0) { throw StorageErrorFactory.getSnapshotsPresent(context.contextId!); } else { - coll.findAndRemove( - this.currentVersionQuery({ - accountName: account, - containerName: container, - name: blob - }) - ); + deleteCurrentVersion(); } } @@ -1724,13 +1773,7 @@ export default class LokiBlobMetadataStore name: blob, snapshot: { $gt: "" } }); - coll.findAndRemove( - this.currentVersionQuery({ - accountName: account, - containerName: container, - name: blob - }) - ); + deleteCurrentVersion(); } // Scenario: Delete all snapshots only diff --git a/tests/blob/apis/blob.versioning.test.ts b/tests/blob/apis/blob.versioning.test.ts index c14911d81..4167ee3da 100644 --- a/tests/blob/apis/blob.versioning.test.ts +++ b/tests/blob/apis/blob.versioning.test.ts @@ -214,27 +214,121 @@ describe("BlobVersioningAPIs", () => { assert.strictEqual(await bodyToString(current, 8), "version2"); }); - it("Deleting the current version should retain previous versions @loki", async () => { + it("Deleting a blob should turn the current version into a previous version @loki", async () => { const first = await blockBlobClient.upload("version1", 8); - await blockBlobClient.upload("version2", 8); + const second = await blockBlobClient.upload("version2", 8); // Deleting the blob does not fail even though previous versions exist, and it does // not remove them. This differs from snapshots, which return SnapshotsPresent. await blockBlobClient.delete(); + // "the current version of the blob becomes a previous version, and there's no longer + // a current version. Any previous versions of the blob persist." So both versions + // survive the delete, and neither is current. const versions = await listVersions(blobName); - assert.strictEqual(versions.length, 1); + assert.strictEqual(versions.length, 2, "Both versions should survive the delete"); assert.strictEqual(versions[0].versionId, first.versionId); - assert.notStrictEqual(versions[0].isCurrentVersion, true); + assert.strictEqual(versions[1].versionId, second.versionId); + for (const version of versions) { + assert.notStrictEqual( + version.isCurrentVersion, + true, + "No version should be current after the delete" + ); + assert.strictEqual( + version.hasVersionsOnly, + true, + "The blob should report that only versions remain" + ); + } - // The blob itself is gone + // The blob itself is gone for callers that do not ask for a version assert.strictEqual(await blockBlobClient.exists(), false); - // But the previous version is still readable by version ID + let error; + try { + await blockBlobClient.download(); + } catch (err) { + error = err; + } + assert.strictEqual((error as any)?.statusCode, 404); + + // Both versions remain readable by version ID, including the one that was current const previous = await blockBlobClient .withVersion(first.versionId!) .download(); assert.strictEqual(await bodyToString(previous, 8), "version1"); + + const wasCurrent = await blockBlobClient + .withVersion(second.versionId!) + .download(); + assert.strictEqual( + await bodyToString(wasCurrent, 8), + "version2", + "The content that was current at delete time must not be lost" + ); + }); + + it("Writing after a delete should create a new current version @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + const second = await blockBlobClient.upload("version2", 8); + await blockBlobClient.delete(); + + // "Writing new data to the blob creates a new current version of the blob. Any + // existing versions are unaffected." + const third = await blockBlobClient.upload("version3", 8); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 3); + assert.deepStrictEqual( + versions.map((v) => v.versionId), + [first.versionId, second.versionId, third.versionId] + ); + assert.strictEqual(versions[2].isCurrentVersion, true); + for (const version of versions) { + assert.notStrictEqual(version.hasVersionsOnly, true); + } + + const current = await blockBlobClient.download(); + assert.strictEqual(await bodyToString(current, 8), "version3"); + }); + + it("Deleting a blob with deleteSnapshots should retain versions @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + const second = await blockBlobClient.upload("version2", 8); + await blockBlobClient.createSnapshot(); + + await blockBlobClient.delete({ deleteSnapshots: "include" }); + + // Snapshots are removed, versions are not + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 2); + assert.deepStrictEqual( + versions.map((v) => v.versionId), + [first.versionId, second.versionId] + ); + + let snapshotCount = 0; + for await (const item of containerClient.listBlobsFlat({ + includeSnapshots: true + })) { + if (item.name === blobName && item.snapshot) { + snapshotCount++; + } + } + assert.strictEqual(snapshotCount, 0, "Snapshots should have been removed"); + }); + + it("Deleting a blob without versioning should still remove it @loki", async () => { + // Guards the non-versioned path, which must keep removing the blob outright. + // Covered here for the versioned account too via an explicit version delete of the + // only version, which is a hard delete rather than a demotion. + const only = await blockBlobClient.upload("version1", 8); + await blockBlobClient.withVersion(only.versionId!).delete(); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 0); + assert.strictEqual(await blockBlobClient.exists(), false); }); it("Restore should be a copy of a previous version over the current one @loki", async () => { From 05b14c0a6e361c173b08e6012fbf21c05e32526a Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 10:20:23 +0100 Subject: [PATCH 04/10] Cover the remaining version creating operations Only Put Blob, Put Block List and Copy Blob created versions, so several documented write operations silently did not. Per the reference behaviour, for block blobs every write except Put Block creates a version, and for page and append blobs Put Blob, Put Block List, Set Blob Metadata and Copy Blob do. Added: - Set Blob Metadata creates a version for every blob type and returns x-ms-version-id. - Set Blob Properties creates a version for block blobs only. It deliberately does not return x-ms-version-id: the storage swagger declares that header on eleven operations and Set Blob Properties is not one of them, matching the real service. - Page Blob Create and Append Blob Create return x-ms-version-id. The version was already being created by createBlob, only the header was missing. - Snapshot Blob on a versioned blob now creates a new current version alongside the snapshot, and returns both x-ms-snapshot and x-ms-version-id. - Copy Blob and Copy Blob From URL return the destination's new version ID. - Get Blob Tags, Set Blob Tags and Set Blob Tier accept ?versionid=, so tags and access tier are addressable per version rather than silently applying to the current version. - A malformed ?versionid= returns 400 InvalidQueryParameterValue rather than falling through to a 404. Put Page and Append Block continue to create no version, which is the documented exception for page and append blobs and is now covered by tests. Modifying the current version is expressed by a new createNewCurrentVersion helper: it copies the current document, demotes the original in place so it keeps the old state, and gives the copy a fresh version ID. The copy inherits the lease because a lease belongs to the blob rather than to a version. setBlobHTTPHeaders and setBlobMetadata now return the new version ID alongside the properties, and the two copy operations return a properties object widened with an optional versionId, so the handlers can populate the response header. The SQL store implements the same signatures and never produces a version ID, since versioning is not supported there. One existing test expectation was stale rather than wrong: deleting with deleteSnapshots=include after a snapshot now leaves three versions instead of two, because taking the snapshot itself creates one. Co-Authored-By: Claude --- ChangeLog.md | 1 + docs/designs/blob-versioning.md | 15 ++ src/blob/handlers/AppendBlobHandler.ts | 4 +- src/blob/handlers/BlobHandler.ts | 40 +++- src/blob/handlers/PageBlobHandler.ts | 4 +- src/blob/persistence/IBlobMetadataStore.ts | 31 ++- src/blob/persistence/LokiBlobMetadataStore.ts | 135 +++++++++--- src/blob/persistence/SqlBlobMetadataStore.ts | 14 +- src/blob/utils/utils.ts | 15 ++ tests/blob/apis/blob.versioning.test.ts | 192 +++++++++++++++++- 10 files changed, 401 insertions(+), 50 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 7580f9b78..0930ae242 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -7,6 +7,7 @@ Blob: - Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options rather than through a data plane API. With versioning enabled for an account: overwriting a blob retains the previous content as a version and returns `x-ms-version-id`; `Get Blob` / `Get Blob Properties` accept `?versionid=` and return `x-ms-is-current-version` for the current version; `List Blobs` supports `include=versions`; `Delete Blob` with `?versionid=` removes a single version, and deleting the current version retains previous versions instead of failing with `SnapshotsPresent`; a version can be restored by copying it over the current version. Combining `?snapshot=` with `?versionid=` returns 400 `InvalidQueryParameterValue`. The setting is persisted with the workspace and cannot be changed against existing data. Versioning is not implemented for the SQL metadata store, and blob version SAS (`sr=bv`, the `x` permission), soft delete interactions, and blob expiration interactions are out of scope - see `docs/designs/blob-versioning.md`. +- Extended blob versioning to the remaining version creating operations: `Set Blob Metadata` (all blob types), `Set Blob Properties` (block blobs only), `Page Blob Create` and `Append Blob Create` now return `x-ms-version-id`, and `Snapshot Blob` on a versioned blob creates a new current version alongside the snapshot. `Put Page` and `Append Block` correctly do not create versions. `Get Blob Tags`, `Set Blob Tags` and `Set Blob Tier` accept `?versionid=`, and a malformed `?versionid=` returns 400 `InvalidQueryParameterValue`. - `Delete Blob` on a versioned blob without a version ID now turns the current version into a previous version and retains it, rather than removing it, and reports `HasVersionsOnly` for a blob that has versions but no current version. - Blob list continuation tokens now carry a secondary key so that a `List Blobs` page can resume part way through one blob's versions. Tokens for listings that do not involve versions keep the previous plain blob name format, so existing tokens remain valid. diff --git a/docs/designs/blob-versioning.md b/docs/designs/blob-versioning.md index c5ff0448f..8453b1803 100644 --- a/docs/designs/blob-versioning.md +++ b/docs/designs/blob-versioning.md @@ -106,9 +106,21 @@ a plain blob name. ## Behaviour +Which operations create a version follows the reference behaviour: for block blobs every +write except Put Block, and for page and append blobs only Put Blob, Put Block List, Set +Blob Metadata and Copy Blob. + | Operation | Behaviour with versioning enabled | | --- | --- | | Put Blob, Put Block List, Copy Blob | Overwriting retains the previous content as a version; the response carries `x-ms-version-id` | +| Page Blob Create, Append Blob Create | Creates a version and returns `x-ms-version-id` | +| Put Page, Append Block | Do **not** create a version, matching the reference behaviour for page and append blobs | +| Set Blob Metadata | Creates a version for every blob type and returns `x-ms-version-id` | +| Set Blob Properties | Creates a version for block blobs only. No `x-ms-version-id` is returned, because the storage swagger does not declare that header on this operation | +| Snapshot Blob | Creates a snapshot **and** a new current version, returning both `x-ms-snapshot` and `x-ms-version-id` | +| Get Blob Tags, Set Blob Tags | Accept `?versionid=`, so tags are addressable per version | +| Set Blob Tier | Accepts `?versionid=`, so any version can be tiered independently | +| Malformed `?versionid=` | 400 `InvalidQueryParameterValue`, rather than 404 | | Get Blob, Get Blob Properties | `?versionid=` addresses one version; without it the current version is addressed. `x-ms-is-current-version: true` is returned only for the current version | | List Blobs | `include=versions` returns previous versions with `VersionId` and `IsCurrentVersion`; they are hidden otherwise. Versions of the same blob are ordered oldest first, current last | | Delete Blob with `?versionid=` | Deletes just that version; `x-ms-delete-snapshots` cannot be combined with it | @@ -135,4 +147,7 @@ emulated: cannot be used to address a specific version. - **Get Block List** and **Get Page Ranges** with a version ID. The current storage swagger does not define `versionid` on either operation, matching Azure. +- **Verification against a real storage account.** The behaviour here is implemented from + the reference documentation and the storage swagger, not confirmed against a live + account, so a parity test pass against real Azure is still worth doing. - **Blob expiration** and object replication interactions. diff --git a/src/blob/handlers/AppendBlobHandler.ts b/src/blob/handlers/AppendBlobHandler.ts index 08d24f73b..4c69d143c 100644 --- a/src/blob/handlers/AppendBlobHandler.ts +++ b/src/blob/handlers/AppendBlobHandler.ts @@ -95,7 +95,9 @@ export default class AppendBlobHandler extends BaseHandler version: BLOB_API_VERSION, date, isServerEncrypted: true, - clientRequestId: options.requestId + clientRequestId: options.requestId, + // Set by the metadata store when versioning is enabled for the account + versionId: blob.versionId }; return response; diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts index c48eb1fd8..fac8a6e1e 100644 --- a/src/blob/handlers/BlobHandler.ts +++ b/src/blob/handlers/BlobHandler.ts @@ -292,7 +292,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options.modifiedAccessConditions ); } else { - res = await this.metadataStore.setBlobHTTPHeaders( + const headersRes = await this.metadataStore.setBlobHTTPHeaders( context, account, container, @@ -301,6 +301,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options.blobHTTPHeaders, options.modifiedAccessConditions ); + res = headersRes.properties; } const response: Models.BlobSetHTTPHeadersResponse = { @@ -374,13 +375,15 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { // ToDo: return correct headers and test for these. const response: Models.BlobSetMetadataResponse = { statusCode: 200, - eTag: res.etag, - lastModified: res.lastModified, + eTag: res.properties.etag, + lastModified: res.properties.lastModified, isServerEncrypted: true, requestId: context.contextId, date: context.startTime, version: BLOB_API_VERSION, - clientRequestId: options.requestId + clientRequestId: options.requestId, + // Set Blob Metadata creates a version for every blob type + versionId: res.versionId }; return response; @@ -643,7 +646,9 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { date: context.startTime!, version: BLOB_API_VERSION, snapshot: res.snapshot, - clientRequestId: options.requestId + clientRequestId: options.requestId, + // Snapshotting a versioned blob also creates a new current version + versionId: res.versionId }; return response; @@ -724,7 +729,9 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { date: context.startTime, copyId: res.copyId, copyStatus: res.copyStatus, - clientRequestId: options.requestId + clientRequestId: options.requestId, + // Set by the metadata store when versioning is enabled + versionId: res.versionId }; return response; @@ -947,7 +954,9 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { // Per the Copy Blob From URL REST contract, echo the source's Content-MD5 // back to the client when it was supplied in x-ms-source-content-md5. contentMD5: options.sourceContentMD5, - clientRequestId: options.requestId + clientRequestId: options.requestId, + // Set by the metadata store when versioning is enabled + versionId: res.versionId }; return response; @@ -977,7 +986,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { container, blob, tier, - options.leaseAccessConditions + options.leaseAccessConditions, + options.versionId ); const response: Models.BlobSetTierResponse = { @@ -1317,6 +1327,12 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const account = blobCtx.account!; const container = blobCtx.container!; const blob = blobCtx.blob!; + validateSnapshotAndVersionId( + options.snapshot, + options.versionId, + context.contextId + ); + const tags = await this.metadataStore.getBlobTag( context, account, @@ -1324,7 +1340,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { blob, options.snapshot, options.leaseAccessConditions, - options.modifiedAccessConditions + options.modifiedAccessConditions, + options.versionId ); const response: Models.BlobGetTagsResponse = { @@ -1355,6 +1372,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { // Get snapshot (swagger not defined snapshot as parameter, but server support set tag on blob snapshot) let snapshot = context.request!.getQuery("snapshot"); + validateSnapshotAndVersionId(snapshot, options.versionId, context.contextId); + await this.metadataStore.setBlobTag( context, account, @@ -1363,7 +1382,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { snapshot, options.leaseAccessConditions, tags, - options.modifiedAccessConditions + options.modifiedAccessConditions, + options.versionId ); const response: Models.BlobSetTagsResponse = { diff --git a/src/blob/handlers/PageBlobHandler.ts b/src/blob/handlers/PageBlobHandler.ts index 28f79d018..eb7b2225c 100644 --- a/src/blob/handlers/PageBlobHandler.ts +++ b/src/blob/handlers/PageBlobHandler.ts @@ -162,7 +162,9 @@ export default class PageBlobHandler extends BaseHandler version: BLOB_API_VERSION, date, isServerEncrypted: true, - clientRequestId: options.requestId + clientRequestId: options.requestId, + // Set by the metadata store when versioning is enabled for the account + versionId: blob.versionId }; return response; diff --git a/src/blob/persistence/IBlobMetadataStore.ts b/src/blob/persistence/IBlobMetadataStore.ts index 3af01023b..75001484a 100644 --- a/src/blob/persistence/IBlobMetadataStore.ts +++ b/src/blob/persistence/IBlobMetadataStore.ts @@ -156,6 +156,20 @@ interface IGetBlobPropertiesRes { } export type GetBlobPropertiesRes = IGetBlobPropertiesRes; +// The response model for setBlobHTTPHeaders and setBlobMetadata. With versioning enabled +// these operations create a new version, whose ID is returned as x-ms-version-id. +interface ISetBlobPropertiesRes { + properties: Models.BlobPropertiesInternal; + versionId?: string; +} +export type SetBlobPropertiesRes = ISetBlobPropertiesRes; + +// The response model for startCopyFromURL and copyFromURL. Copying over a destination +// that already exists creates a new version of the destination when versioning is on. +export type CopyBlobRes = Models.BlobPropertiesInternal & { + versionId?: string; +}; + export type FilterBlobModel = FilterBlobItem; // The response model for each lease-related request. @@ -174,6 +188,8 @@ export type ChangeBlobLeaseResponse = IBlobLeaseResponse; interface ICreateSnapshotResponse { properties: Models.BlobPropertiesInternal; snapshot: string; + // Taking a snapshot of a versioned blob also creates a new version + versionId?: string; } export type CreateSnapshotResponse = ICreateSnapshotResponse; @@ -648,7 +664,7 @@ export interface IBlobMetadataStore leaseAccessConditions: Models.LeaseAccessConditions | undefined, blobHTTPHeaders: Models.BlobHTTPHeaders | undefined, modifiedAccessConditions?: Models.ModifiedAccessConditions - ): Promise; + ): Promise; /** * Set blob metadata. @@ -671,7 +687,7 @@ export interface IBlobMetadataStore leaseAccessConditions: Models.LeaseAccessConditions | undefined, metadata: Models.BlobMetadata | undefined, modifiedAccessConditions?: Models.ModifiedAccessConditions - ): Promise; + ): Promise; /** * Acquire blob lease. @@ -843,7 +859,7 @@ export interface IBlobMetadataStore metadata: Models.BlobMetadata | undefined, tier: Models.AccessTier | undefined, leaseAccessConditions?: Models.BlobStartCopyFromURLOptionalParams - ): Promise; + ): Promise; /** * Sync copy from Url. @@ -866,7 +882,7 @@ export interface IBlobMetadataStore metadata: Models.BlobMetadata | undefined, tier: Models.AccessTier | undefined, leaseAccessConditions?: Models.BlobCopyFromURLOptionalParams - ): Promise; + ): Promise; /** * Update Tier for a blob. @@ -886,7 +902,8 @@ export interface IBlobMetadataStore container: string, blob: string, tier: Models.AccessTier, - leaseAccessConditions: Models.LeaseAccessConditions | undefined + leaseAccessConditions: Models.LeaseAccessConditions | undefined, + versionId?: string ): Promise<200 | 202>; /** @@ -1122,7 +1139,8 @@ export interface IBlobMetadataStore snapshot: string | undefined, leaseAccessConditions: Models.LeaseAccessConditions | undefined, tags: Models.BlobTags | undefined, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise; /** @@ -1146,6 +1164,7 @@ export interface IBlobMetadataStore snapshot: string | undefined, leaseAccessConditions: Models.LeaseAccessConditions | undefined, modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise; /** diff --git a/src/blob/persistence/LokiBlobMetadataStore.ts b/src/blob/persistence/LokiBlobMetadataStore.ts index 0aa261483..82271d655 100644 --- a/src/blob/persistence/LokiBlobMetadataStore.ts +++ b/src/blob/persistence/LokiBlobMetadataStore.ts @@ -66,7 +66,9 @@ import IBlobMetadataStore, { ReleaseContainerLeaseResponse, RenewBlobLeaseResponse, RenewContainerLeaseResponse, + CopyBlobRes, ServicePropertiesModel, + SetBlobPropertiesRes, SetContainerAccessPolicyOptions } from "./IBlobMetadataStore"; import PageWithDelimiter, { @@ -368,6 +370,60 @@ export default class LokiBlobMetadataStore } } + /** + * Turn a modification of the current version into a new version. + * + * With versioning enabled, a write that modifies an existing blob leaves the old state + * behind as a previous version and captures the new state as a new current version. + * The caller applies its changes to the document returned from here. + * + * The returned document is a copy of the current version carrying a fresh version ID, + * and it inherits the lease, because a lease belongs to the blob rather than to any one + * version. The original document is demoted in place and keeps the old state. + * + * @private + * @param {Collection} coll + * @param {*} doc The current version, which becomes the previous version + * @param {Context} context + * @returns {*} The new current version, already inserted + * @memberof LokiBlobMetadataStore + */ + private createNewCurrentVersion( + coll: Collection, + doc: any, + context: Context + ): any { + // Copy before demoting, so the copy still carries the lease and the old state + const copy: any = { ...doc }; + delete copy.$loki; + delete copy.meta; + copy.properties = { ...doc.properties }; + if (doc.metadata !== undefined) { + copy.metadata = { ...doc.metadata }; + } + if (doc.committedBlocksInOrder !== undefined) { + copy.committedBlocksInOrder = doc.committedBlocksInOrder.slice(); + } + if (doc.pageRangesInOrder !== undefined) { + copy.pageRangesInOrder = doc.pageRangesInOrder.slice(); + } + if (doc.persistency !== undefined) { + copy.persistency = { ...doc.persistency }; + } + + this.demoteToPreviousVersion(coll, doc); + + copy.versionId = this.generateVersionId( + context, + copy.accountName, + copy.containerName, + copy.name + ); + copy.isCurrentVersion = true; + + return coll.insert(copy); + } + /** * Build a Loki query that matches only the current version of a blob. * @@ -1476,9 +1532,19 @@ export default class LokiBlobMetadataStore coll.insert(snapshotBlob); + // "When you take a snapshot of a versioned blob, a new version is created at the same + // time that the snapshot is created. A new current version is also created when a + // snapshot is taken." + let versionId: string | undefined; + if (this.isVersioningEnabled(account)) { + const newCurrent = this.createNewCurrentVersion(coll, doc, context); + versionId = newCurrent.versionId; + } + return { properties: snapshotBlob.properties, - snapshot: snapshotTime + snapshot: snapshotTime, + versionId }; } @@ -1812,9 +1878,9 @@ export default class LokiBlobMetadataStore leaseAccessConditions: Models.LeaseAccessConditions | undefined, blobHTTPHeaders: Models.BlobHTTPHeaders | undefined, modifiedAccessConditions?: Models.ModifiedAccessConditions - ): Promise { + ): Promise { const coll = this.db.getCollection(this.BLOBS_COLLECTION); - const doc = await this.getBlobWithLeaseUpdated( + const current = await this.getBlobWithLeaseUpdated( account, container, blob, @@ -1824,15 +1890,24 @@ export default class LokiBlobMetadataStore true ); - validateWriteConditions(context, modifiedAccessConditions, doc); + validateWriteConditions(context, modifiedAccessConditions, current); - if (!doc) { + if (!current) { throw StorageErrorFactory.getBlobNotFound(context.contextId); } - const lease = new BlobLeaseAdapter(doc); + const lease = new BlobLeaseAdapter(current); new BlobWriteLeaseValidator(leaseAccessConditions).validate(lease, context); + // For block blobs every write except Put Block creates a version. For page and + // append blobs only Put Blob, Put Block List, Set Blob Metadata and Copy Blob do, + // so Set Blob Properties does not create a version for those types. + const doc = + this.isVersioningEnabled(account) && + current.properties.blobType === Models.BlobType.BlockBlob + ? this.createNewCurrentVersion(coll, current, context) + : current; + const blobHeaders = blobHTTPHeaders; const blobProps = doc.properties; // as per https://docs.microsoft.com/en-us/rest/api/storageservices/set-blob-properties#remarks @@ -1856,7 +1931,7 @@ export default class LokiBlobMetadataStore new BlobWriteLeaseSyncer(doc).sync(lease); coll.update(doc); - return doc.properties; + return { properties: doc.properties, versionId: doc.versionId }; } /** @@ -1880,9 +1955,9 @@ export default class LokiBlobMetadataStore leaseAccessConditions: Models.LeaseAccessConditions | undefined, metadata: Models.BlobMetadata | undefined, modifiedAccessConditions?: Models.ModifiedAccessConditions - ): Promise { + ): Promise { const coll = this.db.getCollection(this.BLOBS_COLLECTION); - const doc = await this.getBlobWithLeaseUpdated( + const current = await this.getBlobWithLeaseUpdated( account, container, blob, @@ -1892,20 +1967,26 @@ export default class LokiBlobMetadataStore true ); - validateWriteConditions(context, modifiedAccessConditions, doc); + validateWriteConditions(context, modifiedAccessConditions, current); - if (!doc) { + if (!current) { throw StorageErrorFactory.getBlobNotFound(context.contextId); } - const lease = new BlobLeaseAdapter(doc); + const lease = new BlobLeaseAdapter(current); new BlobWriteLeaseValidator(leaseAccessConditions).validate(lease, context); + + // Set Blob Metadata creates a version for every blob type + const doc = this.isVersioningEnabled(account) + ? this.createNewCurrentVersion(coll, current, context) + : current; + new BlobWriteLeaseSyncer(doc).sync(lease); doc.metadata = metadata; doc.properties.etag = newEtag(); doc.properties.lastModified = context.startTime || new Date(); coll.update(doc); - return doc.properties; + return { properties: doc.properties, versionId: doc.versionId }; } /** @@ -2262,7 +2343,7 @@ export default class LokiBlobMetadataStore metadata: Models.BlobMetadata | undefined, tier: Models.AccessTier | undefined, options: Models.BlobStartCopyFromURLOptionalParams = {} - ): Promise { + ): Promise { const coll = this.db.getCollection(this.BLOBS_COLLECTION); const sourceBlob = await this.getBlobWithLeaseUpdated( source.account, @@ -2454,7 +2535,7 @@ export default class LokiBlobMetadataStore } coll.insert(copiedBlob); - return copiedBlob.properties; + return { ...copiedBlob.properties, versionId: copiedBlob.versionId }; } /** @@ -2478,7 +2559,7 @@ export default class LokiBlobMetadataStore metadata: Models.BlobMetadata | undefined, tier: Models.AccessTier | undefined, options: Models.BlobCopyFromURLOptionalParams = {} - ): Promise { + ): Promise { const coll = this.db.getCollection(this.BLOBS_COLLECTION); const sourceBlob = await this.getBlobWithLeaseUpdated( source.account, @@ -2667,7 +2748,7 @@ export default class LokiBlobMetadataStore } coll.insert(copiedBlob); - return copiedBlob.properties; + return { ...copiedBlob.properties, versionId: copiedBlob.versionId }; } /** @@ -2688,9 +2769,12 @@ export default class LokiBlobMetadataStore container: string, blob: string, tier: Models.AccessTier, - leaseAccessConditions: Models.LeaseAccessConditions | undefined + leaseAccessConditions: Models.LeaseAccessConditions | undefined, + versionId?: string ): Promise<200 | 202> { const coll = this.db.getCollection(this.BLOBS_COLLECTION); + // Any version of a block blob can be tiered, including the current one, so an + // explicit version ID has to address that version rather than the current one. const doc = await this.getBlobWithLeaseUpdated( account, container, @@ -2698,7 +2782,8 @@ export default class LokiBlobMetadataStore undefined, context, true, - true + true, + versionId ); let responseCode: 200 | 202 = 200; @@ -3923,7 +4008,8 @@ export default class LokiBlobMetadataStore snapshot: string | undefined, leaseAccessConditions: Models.LeaseAccessConditions | undefined, tags: Models.BlobTags | undefined, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise { const coll = this.db.getCollection(this.BLOBS_COLLECTION); const doc = await this.getBlobWithLeaseUpdated( @@ -3933,7 +4019,8 @@ export default class LokiBlobMetadataStore snapshot, context, false, - true + true, + versionId ); if (!doc) { @@ -3967,7 +4054,8 @@ export default class LokiBlobMetadataStore blob: string, snapshot: string = "", leaseAccessConditions: Models.LeaseAccessConditions | undefined, - modifiedAccessConditions?: Models.ModifiedAccessConditions + modifiedAccessConditions?: Models.ModifiedAccessConditions, + versionId?: string ): Promise { const doc = await this.getBlobWithLeaseUpdated( account, @@ -3976,7 +4064,8 @@ export default class LokiBlobMetadataStore snapshot, context, false, - true + true, + versionId ); validateReadConditions(context, modifiedAccessConditions, doc); diff --git a/src/blob/persistence/SqlBlobMetadataStore.ts b/src/blob/persistence/SqlBlobMetadataStore.ts index f5c90f110..7d3fe728f 100644 --- a/src/blob/persistence/SqlBlobMetadataStore.ts +++ b/src/blob/persistence/SqlBlobMetadataStore.ts @@ -65,7 +65,9 @@ import IBlobMetadataStore, { ReleaseBlobLeaseResponse, RenewBlobLeaseResponse, RenewContainerLeaseResponse, + CopyBlobRes, ServicePropertiesModel, + SetBlobPropertiesRes, SetContainerAccessPolicyOptions } from "./IBlobMetadataStore"; import PageWithDelimiter from "./PageWithDelimiter"; @@ -2054,7 +2056,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { leaseAccessConditions: Models.LeaseAccessConditions | undefined, blobHTTPHeaders: Models.BlobHTTPHeaders | undefined, modifiedAccessConditions?: Models.ModifiedAccessConditions - ): Promise { + ): Promise { return this.sequelize.transaction(async (t) => { await this.assertContainerExists(context, account, container, t); @@ -2118,7 +2120,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { transaction: t }); - return blobModel.properties; + return { properties: blobModel.properties }; }); } @@ -2130,7 +2132,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { leaseAccessConditions: Models.LeaseAccessConditions | undefined, metadata: Models.BlobMetadata | undefined, modifiedAccessConditions?: Models.ModifiedAccessConditions - ): Promise { + ): Promise { return this.sequelize.transaction(async (t) => { await this.assertContainerExists(context, account, container, t); @@ -2194,7 +2196,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { leaseState: blobModel.properties.leaseState }; - return ret; + return { properties: ret }; }); } @@ -2552,7 +2554,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { metadata: Models.BlobMetadata | undefined, tier: Models.AccessTier | undefined, options: Models.BlobStartCopyFromURLOptionalParams = {} - ): Promise { + ): Promise { return this.sequelize.transaction(async (t) => { const sourceBlob = await this.getBlobWithLeaseUpdated( source.account, @@ -2720,7 +2722,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore { destination: BlobId, copySource: string, metadata: Models.BlobMetadata | undefined - ): Promise { + ): Promise { throw new NotImplementedinSQLError(context.contextId); } diff --git a/src/blob/utils/utils.ts b/src/blob/utils/utils.ts index 58858876c..00bcb42ad 100644 --- a/src/blob/utils/utils.ts +++ b/src/blob/utils/utils.ts @@ -411,6 +411,21 @@ export function validateSnapshotAndVersionId( versionId?: string, contextID?: string ): void { + // A version ID is an RFC 3339 timestamp with 7 digit fractional seconds. Azure rejects + // anything else with 400 InvalidQueryParameterValue rather than returning 404. + if ( + versionId !== undefined && + versionId !== "" && + !/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{7}Z$/.test(versionId) + ) { + throw StorageErrorFactory.getInvalidQueryParameterValue( + contextID, + "versionid", + versionId, + "The version ID is not a valid RFC 3339 timestamp with 7 digit fractional seconds." + ); + } + if ( snapshot !== undefined && snapshot !== "" && diff --git a/tests/blob/apis/blob.versioning.test.ts b/tests/blob/apis/blob.versioning.test.ts index 4167ee3da..9b8098767 100644 --- a/tests/blob/apis/blob.versioning.test.ts +++ b/tests/blob/apis/blob.versioning.test.ts @@ -296,16 +296,17 @@ describe("BlobVersioningAPIs", () => { it("Deleting a blob with deleteSnapshots should retain versions @loki", async () => { const first = await blockBlobClient.upload("version1", 8); const second = await blockBlobClient.upload("version2", 8); - await blockBlobClient.createSnapshot(); + // Snapshotting a versioned blob also creates a version, so this leaves three + const snapshot = await blockBlobClient.createSnapshot(); await blockBlobClient.delete({ deleteSnapshots: "include" }); // Snapshots are removed, versions are not const versions = await listVersions(blobName); - assert.strictEqual(versions.length, 2); + assert.strictEqual(versions.length, 3); assert.deepStrictEqual( versions.map((v) => v.versionId), - [first.versionId, second.versionId] + [first.versionId, second.versionId, snapshot.versionId] ); let snapshotCount = 0; @@ -486,6 +487,191 @@ describe("BlobVersioningAPIs", () => { assert.deepStrictEqual(seen, names); }); + it("Set Blob Metadata should create a version @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + const set = await blockBlobClient.setMetadata({ k: "v2" }); + + // Set Blob Metadata is named explicitly in the docs as version creating, for every + // blob type. + assert.notStrictEqual(set.versionId, undefined); + assert.notStrictEqual(set.versionId, first.versionId); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 2); + assert.deepStrictEqual( + versions.map((v) => v.versionId), + [first.versionId, set.versionId] + ); + assert.strictEqual(versions[1].isCurrentVersion, true); + + // The previous version keeps the old metadata and the old content + const previous = await blockBlobClient + .withVersion(first.versionId!) + .getProperties(); + assert.deepStrictEqual(previous.metadata ?? {}, {}); + const body = await blockBlobClient.withVersion(first.versionId!).download(); + assert.strictEqual(await bodyToString(body, 8), "version1"); + + // The current version has the new metadata + const current = await blockBlobClient.getProperties(); + assert.deepStrictEqual(current.metadata, { k: "v2" }); + }); + + it("Set Blob Properties should create a version for a block blob @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + await blockBlobClient.setHTTPHeaders({ blobContentType: "text/plain" }); + + // For block blobs every write except Put Block creates a version. Set Blob + // Properties does not return x-ms-version-id: the storage swagger does not declare + // that header on this operation. + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 2); + assert.strictEqual(versions[0].versionId, first.versionId); + assert.strictEqual(versions[1].isCurrentVersion, true); + + assert.strictEqual( + (await blockBlobClient.getProperties()).contentType, + "text/plain" + ); + }); + + it("Set Blob Properties should NOT create a version for a page blob @loki", async () => { + // For page and append blobs only Put Blob, Put Block List, Set Blob Metadata and + // Copy Blob create a version. + const name = getUniqueName("page"); + const pageClient = containerClient.getPageBlobClient(name); + await pageClient.create(512); + await pageClient.setHTTPHeaders({ blobContentType: "text/plain" }); + + assert.strictEqual((await listVersions(name)).length, 1); + }); + + it("Page and append blob create should return a version ID @loki", async () => { + const pageName = getUniqueName("page"); + const pageClient = containerClient.getPageBlobClient(pageName); + const pageCreate = await pageClient.create(512); + assert.notStrictEqual(pageCreate.versionId, undefined); + + // Put Page does not create a version + await pageClient.uploadPages("x".repeat(512), 0, 512); + assert.strictEqual((await listVersions(pageName)).length, 1); + + const appendName = getUniqueName("append"); + const appendClient = containerClient.getAppendBlobClient(appendName); + const appendCreate = await appendClient.create(); + assert.notStrictEqual(appendCreate.versionId, undefined); + + // Append Block does not create a version + await appendClient.appendBlock("y", 1); + assert.strictEqual((await listVersions(appendName)).length, 1); + + // Set Blob Metadata does, for both types + const pageMeta = await pageClient.setMetadata({ k: "v" }); + assert.notStrictEqual(pageMeta.versionId, undefined); + assert.strictEqual((await listVersions(pageName)).length, 2); + + const appendMeta = await appendClient.setMetadata({ k: "v" }); + assert.notStrictEqual(appendMeta.versionId, undefined); + assert.strictEqual((await listVersions(appendName)).length, 2); + }); + + it("Snapshot of a versioned blob should create a version @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + const snapshot = await blockBlobClient.createSnapshot(); + + // "a new version is created at the same time that the snapshot is created. A new + // current version is also created when a snapshot is taken." + assert.notStrictEqual(snapshot.snapshot, undefined); + assert.notStrictEqual(snapshot.versionId, undefined); + assert.notStrictEqual(snapshot.versionId, first.versionId); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 2); + assert.strictEqual(versions[1].versionId, snapshot.versionId); + assert.strictEqual(versions[1].isCurrentVersion, true); + }); + + it("Copy should return the destination version ID @loki", async () => { + const source = containerClient.getBlockBlobClient(getUniqueName("src")); + await source.upload("source12", 8); + + const firstDest = await blockBlobClient.upload("version1", 8); + const poller = await blockBlobClient.beginCopyFromURL(source.url); + const copy = await poller.pollUntilDone(); + + assert.notStrictEqual(copy.versionId, undefined); + assert.notStrictEqual(copy.versionId, firstDest.versionId); + + const versions = await listVersions(blobName); + assert.strictEqual(versions.length, 2); + assert.strictEqual(versions[1].versionId, copy.versionId); + }); + + it("Tags should be per version @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + await blockBlobClient.setTags({ tier: "old" }); + const second = await blockBlobClient.upload("version2", 8); + await blockBlobClient.setTags({ tier: "new" }); + + const currentTags = await blockBlobClient.getTags(); + assert.deepStrictEqual(currentTags.tags, { tier: "new" }); + + // The tags set while the first version was current belong to that version + const firstTags = await blockBlobClient + .withVersion(first.versionId!) + .getTags(); + assert.deepStrictEqual(firstTags.tags, { tier: "old" }); + + // Tags can be set on a specific version + await blockBlobClient.withVersion(first.versionId!).setTags({ tier: "archived" }); + assert.deepStrictEqual( + (await blockBlobClient.withVersion(first.versionId!).getTags()).tags, + { tier: "archived" } + ); + // ...without disturbing the current version + assert.deepStrictEqual((await blockBlobClient.getTags()).tags, { + tier: "new" + }); + assert.strictEqual(second.versionId, (await blockBlobClient.getProperties()).versionId); + }); + + it("Access tier should be settable per version @loki", async () => { + const first = await blockBlobClient.upload("version1", 8); + await blockBlobClient.upload("version2", 8); + + await blockBlobClient.withVersion(first.versionId!).setAccessTier("Cool"); + + assert.strictEqual( + (await blockBlobClient.withVersion(first.versionId!).getProperties()) + .accessTier, + "Cool" + ); + // The current version keeps its own tier + assert.notStrictEqual( + (await blockBlobClient.getProperties()).accessTier, + "Cool" + ); + }); + + it("A malformed version ID should fail with 400 @loki", async () => { + await blockBlobClient.upload("version1", 8); + + for (const bad of ["notatimestamp", "2026-08-13", "2026-08-13T10:00:00Z"]) { + let error; + try { + await blockBlobClient.withVersion(bad).download(); + } catch (err) { + error = err; + } + assert.strictEqual( + (error as any)?.statusCode, + 400, + `Expected 400 for version ID "${bad}"` + ); + assert.strictEqual((error as any)?.code, "InvalidQueryParameterValue"); + } + }); + it("Snapshots should still block deleting the base blob @loki", async () => { await blockBlobClient.upload("version1", 8); await blockBlobClient.createSnapshot(); From 1c4468f93d4abf3d21b03d4b1b61b9ccdfb484b2 Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 10:59:03 +0100 Subject: [PATCH 05/10] Align blob versioning with behaviour observed against real Azure Ran tests/blob/apis/blob.versioning.test.ts against a real GPv2 storage account with versioning enabled, via the existing AZURITE_LIVE_TEST_CONNECTION_STRING live mode. Four assertions failed, and in every case the implementation was wrong rather than the test: - Set Blob Properties does not create a version, for any blob type, and returns no x-ms-version-id. The prose documentation says every write on a block blob except Put Block creates a version, which is what the previous commit implemented, but the service disagrees. The swagger agrees with the service: it does not declare x-ms-version-id on this operation. - Deleting the current version by version ID is refused with 403 OperationNotAllowedOnRootBlob. Only a previous version may be targeted by version ID; the current version is removed by deleting the blob without one, which demotes it. Azurite previously hard deleted it. - Combining ?snapshot= with ?versionid= returns 400 MutuallyExclusiveQueryParameters, not InvalidQueryParameterValue. A malformed ?versionid= does return InvalidQueryParameterValue, which was already correct. - HasVersionsOnly is not reported under include=versions, even for a blob whose current version has been deleted. It appears only under include=deletedwithversions, which depends on blob soft delete and is out of scope, so Azurite no longer emits it at all. Adds StorageErrorFactory entries for MutuallyExclusiveQueryParameters (400) and OperationNotAllowedOnRootBlob (403) with the messages the service returns. The suite now passes both ways: 25 tests against real Azure and 28 against Azurite, the difference being the three BlobVersioningDisabledAPIs cases which are skipped in live mode because they assert versioning is off. That equivalence is the point of the exercise, so the live mode setup is documented in docs/designs/blob-versioning.md for the next person. Co-Authored-By: Claude --- ChangeLog.md | 1 + docs/designs/blob-versioning.md | 35 ++++++++-- src/blob/errors/StorageErrorFactory.ts | 32 +++++++++ src/blob/persistence/LokiBlobMetadataStore.ts | 53 ++++++--------- src/blob/utils/utils.ts | 9 +-- tests/blob/apis/blob.versioning.test.ts | 65 ++++++++++--------- 6 files changed, 118 insertions(+), 77 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 0930ae242..ed0df4bad 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -7,6 +7,7 @@ Blob: - Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options rather than through a data plane API. With versioning enabled for an account: overwriting a blob retains the previous content as a version and returns `x-ms-version-id`; `Get Blob` / `Get Blob Properties` accept `?versionid=` and return `x-ms-is-current-version` for the current version; `List Blobs` supports `include=versions`; `Delete Blob` with `?versionid=` removes a single version, and deleting the current version retains previous versions instead of failing with `SnapshotsPresent`; a version can be restored by copying it over the current version. Combining `?snapshot=` with `?versionid=` returns 400 `InvalidQueryParameterValue`. The setting is persisted with the workspace and cannot be changed against existing data. Versioning is not implemented for the SQL metadata store, and blob version SAS (`sr=bv`, the `x` permission), soft delete interactions, and blob expiration interactions are out of scope - see `docs/designs/blob-versioning.md`. +- Aligned blob versioning with behaviour observed against a real storage account: `Set Blob Properties` does not create a version, deleting the current version by version ID returns 403 `OperationNotAllowedOnRootBlob`, combining `?snapshot=` with `?versionid=` returns 400 `MutuallyExclusiveQueryParameters`, and `HasVersionsOnly` is not reported under `include=versions`. - Extended blob versioning to the remaining version creating operations: `Set Blob Metadata` (all blob types), `Set Blob Properties` (block blobs only), `Page Blob Create` and `Append Blob Create` now return `x-ms-version-id`, and `Snapshot Blob` on a versioned blob creates a new current version alongside the snapshot. `Put Page` and `Append Block` correctly do not create versions. `Get Blob Tags`, `Set Blob Tags` and `Set Blob Tier` accept `?versionid=`, and a malformed `?versionid=` returns 400 `InvalidQueryParameterValue`. - `Delete Blob` on a versioned blob without a version ID now turns the current version into a previous version and retains it, rather than removing it, and reports `HasVersionsOnly` for a blob that has versions but no current version. - Blob list continuation tokens now carry a secondary key so that a `List Blobs` page can resume part way through one blob's versions. Tokens for listings that do not involve versions keep the previous plain blob name format, so existing tokens remain valid. diff --git a/docs/designs/blob-versioning.md b/docs/designs/blob-versioning.md index 8453b1803..57205175d 100644 --- a/docs/designs/blob-versioning.md +++ b/docs/designs/blob-versioning.md @@ -116,19 +116,40 @@ Blob Metadata and Copy Blob. | Page Blob Create, Append Blob Create | Creates a version and returns `x-ms-version-id` | | Put Page, Append Block | Do **not** create a version, matching the reference behaviour for page and append blobs | | Set Blob Metadata | Creates a version for every blob type and returns `x-ms-version-id` | -| Set Blob Properties | Creates a version for block blobs only. No `x-ms-version-id` is returned, because the storage swagger does not declare that header on this operation | +| Set Blob Properties | Does **not** create a version, for any blob type, and returns no `x-ms-version-id`. Verified against the real service: the prose docs say every block blob write except Put Block creates a version, but the observed behaviour and the swagger both disagree | | Snapshot Blob | Creates a snapshot **and** a new current version, returning both `x-ms-snapshot` and `x-ms-version-id` | | Get Blob Tags, Set Blob Tags | Accept `?versionid=`, so tags are addressable per version | | Set Blob Tier | Accepts `?versionid=`, so any version can be tiered independently | | Malformed `?versionid=` | 400 `InvalidQueryParameterValue`, rather than 404 | | Get Blob, Get Blob Properties | `?versionid=` addresses one version; without it the current version is addressed. `x-ms-is-current-version: true` is returned only for the current version | | List Blobs | `include=versions` returns previous versions with `VersionId` and `IsCurrentVersion`; they are hidden otherwise. Versions of the same blob are ordered oldest first, current last | -| Delete Blob with `?versionid=` | Deletes just that version; `x-ms-delete-snapshots` cannot be combined with it | -| Delete Blob without `?versionid=` | The current version becomes a previous version and is retained, and the blob has no current version. Previous versions persist, and it does **not** fail with `SnapshotsPresent` because versions exist. `HasVersionsOnly` is reported for a blob in that state | +| Delete Blob with `?versionid=` | Deletes just that version. Only a **previous** version may be targeted: naming the current version returns 403 `OperationNotAllowedOnRootBlob`. `x-ms-delete-snapshots` cannot be combined with it | +| Delete Blob without `?versionid=` | The current version becomes a previous version and is retained, and the blob has no current version. Previous versions persist, and it does **not** fail with `SnapshotsPresent` because versions exist | | Snapshots | Continue to work as before, and still block deleting the base blob with `SnapshotsPresent`. Using versioning and snapshots together is supported but, as in production, not recommended | | Write after Delete Blob | Creates a new current version; existing versions are unaffected | | Restore a version | No dedicated API: copy the version over the current version, `Copy Blob` with a `?versionid=` qualified source | -| `?snapshot=` and `?versionid=` together | 400 `InvalidQueryParameterValue` | +| `?snapshot=` and `?versionid=` together | 400 `MutuallyExclusiveQueryParameters` | + +## Verification against real Azure + +`tests/blob/apis/blob.versioning.test.ts` runs unchanged against a real storage account. +Set `AZURITE_LIVE_TEST_CONNECTION_STRING` to a connection string for a GPv2 account that +has versioning enabled, and the fixture points at that account instead of a local server: + +```bash +export AZURITE_LIVE_TEST_CONNECTION_STRING="DefaultEndpointsProtocol=https;AccountName=...;AccountKey=...;EndpointSuffix=core.windows.net" +npx mocha --require ts-node/register --no-timeouts --grep @loki --exit tests/blob/apis/blob.versioning.test.ts +``` + +The `BlobVersioningDisabledAPIs` block is skipped in live mode, because it asserts that +versioning is off. + +Running this found four places where the implementation had followed the prose +documentation but the service behaves differently: Set Blob Properties does not create a +version, deleting the current version by ID is refused with 403 +`OperationNotAllowedOnRootBlob`, the snapshot/versionid combination returns +`MutuallyExclusiveQueryParameters`, and `HasVersionsOnly` is not reported under +`include=versions`. All four now follow the observed behaviour. ## Not implemented @@ -147,7 +168,7 @@ emulated: cannot be used to address a specific version. - **Get Block List** and **Get Page Ranges** with a version ID. The current storage swagger does not define `versionid` on either operation, matching Azure. -- **Verification against a real storage account.** The behaviour here is implemented from - the reference documentation and the storage swagger, not confirmed against a live - account, so a parity test pass against real Azure is still worth doing. +- **`HasVersionsOnly`.** Verified against the real service: it is not reported under + `include=versions`, only under `include=deletedwithversions`, which depends on blob soft + delete and is therefore out of scope. - **Blob expiration** and object replication interactions. diff --git a/src/blob/errors/StorageErrorFactory.ts b/src/blob/errors/StorageErrorFactory.ts index a0c4e897b..f00a15324 100644 --- a/src/blob/errors/StorageErrorFactory.ts +++ b/src/blob/errors/StorageErrorFactory.ts @@ -637,6 +637,38 @@ export default class StorageErrorFactory { ); } + /** + * Returned when a request names two query parameters that cannot be combined, for + * example snapshot and versionid. Verified against the real service, which uses this + * code rather than InvalidQueryParameterValue. + */ + public static getMutuallyExclusiveQueryParameters( + contextID: string = DefaultID + ): StorageError { + return new StorageError( + 400, + "MutuallyExclusiveQueryParameters", + "The query parameter is invalid. Two or more mutually exclusive query parameters were specified.", + contextID + ); + } + + /** + * Returned when an operation that is only valid against a previous version is attempted + * against the current version, such as deleting the current version by version ID. + * Verified against the real service: 403 OperationNotAllowedOnRootBlob. + */ + public static getOperationNotAllowedOnRootBlob( + contextID: string = DefaultID + ): StorageError { + return new StorageError( + 403, + "OperationNotAllowedOnRootBlob", + "The specified operation is not allowed on root blob.", + contextID + ); + } + public static getSnapshotsPresent(contextID: string): StorageError { return new StorageError( 409, diff --git a/src/blob/persistence/LokiBlobMetadataStore.ts b/src/blob/persistence/LokiBlobMetadataStore.ts index 82271d655..bfe3296b9 100644 --- a/src/blob/persistence/LokiBlobMetadataStore.ts +++ b/src/blob/persistence/LokiBlobMetadataStore.ts @@ -1233,38 +1233,16 @@ export default class LokiBlobMetadataStore nameItem ); - // A blob whose current version has been deleted still has its previous versions. - // HasVersionsOnly flags that state, so a caller listing versions can tell the - // difference between "versions of a live blob" and "all that is left of a blob". - const hasVersionsOnlyByName = new Map(); - if (includeVersions) { - for (const doc of blobItems) { - if (hasVersionsOnlyByName.has(doc.name)) { - continue; - } - const current = coll.findOne( - this.currentVersionQuery({ - accountName: account, - containerName: container, - name: doc.name, - snapshot: "" - }) - ); - hasVersionsOnlyByName.set( - doc.name, - current === null || current === undefined - ); - } - } + // HasVersionsOnly is deliberately not reported here. Verified against the real + // service: listing with include=versions returns no HasVersionsOnly on the version + // items, even for a blob whose current version has been deleted. The flag belongs to + // include=deletedwithversions, which depends on blob soft delete and is out of scope. return [ blobItems.map((doc) => { doc.properties.contentMD5 = this.restoreUint8Array( doc.properties.contentMD5 ); - if (hasVersionsOnlyByName.get(doc.name) === true) { - doc.hasVersionsOnly = true; - } return LeaseFactory.createLeaseState( new BlobLeaseAdapter(doc), context @@ -1761,6 +1739,15 @@ export default class LokiBlobMetadataStore // Scenario: Delete a single blob version. Other versions of the blob, and the // current version, are unaffected. if (options.versionId !== undefined && options.versionId !== "") { + // Verified against the real service: deleting the current version by version ID is + // rejected with 403 OperationNotAllowedOnRootBlob. The current version is removed + // by deleting the blob without a version ID, which demotes it instead. + if (doc.isCurrentVersion === true) { + throw StorageErrorFactory.getOperationNotAllowedOnRootBlob( + context.contextId! + ); + } + coll.findAndRemove({ accountName: account, containerName: container, @@ -1899,14 +1886,12 @@ export default class LokiBlobMetadataStore const lease = new BlobLeaseAdapter(current); new BlobWriteLeaseValidator(leaseAccessConditions).validate(lease, context); - // For block blobs every write except Put Block creates a version. For page and - // append blobs only Put Blob, Put Block List, Set Blob Metadata and Copy Blob do, - // so Set Blob Properties does not create a version for those types. - const doc = - this.isVersioningEnabled(account) && - current.properties.blobType === Models.BlobType.BlockBlob - ? this.createNewCurrentVersion(coll, current, context) - : current; + // Verified against the real service: Set Blob Properties does NOT create a new + // version, for any blob type, and returns no x-ms-version-id. The prose docs say + // every write on a block blob except Put Block creates a version, but the observed + // behaviour and the swagger (which does not declare x-ms-version-id on this + // operation) both say otherwise. + const doc = current; const blobHeaders = blobHTTPHeaders; const blobProps = doc.properties; diff --git a/src/blob/utils/utils.ts b/src/blob/utils/utils.ts index 00bcb42ad..0400d6873 100644 --- a/src/blob/utils/utils.ts +++ b/src/blob/utils/utils.ts @@ -432,11 +432,8 @@ export function validateSnapshotAndVersionId( versionId !== undefined && versionId !== "" ) { - throw StorageErrorFactory.getInvalidQueryParameterValue( - contextID, - "versionid", - versionId, - "The snapshot and versionid query parameters are mutually exclusive." - ); + // Verified against the real service: the snapshot/versionid combination returns + // MutuallyExclusiveQueryParameters, not InvalidQueryParameterValue. + throw StorageErrorFactory.getMutuallyExclusiveQueryParameters(contextID); } } diff --git a/tests/blob/apis/blob.versioning.test.ts b/tests/blob/apis/blob.versioning.test.ts index 9b8098767..3ce695c31 100644 --- a/tests/blob/apis/blob.versioning.test.ts +++ b/tests/blob/apis/blob.versioning.test.ts @@ -13,12 +13,16 @@ import { EMULATOR_ACCOUNT_KEY, EMULATOR_ACCOUNT_NAME, getTestServerBaseURL, - getUniqueName + getUniqueName, + LIVE_TEST_MODE } from "../../testutils"; // Set true to enable debug log configLogger(false); +// In live mode this is ignored: BlobTestServerFactory returns a stub server and the real +// account gets versioning from the ARM management plane instead. The live account must +// therefore have versioning enabled for this suite to pass. const VERSIONING_ENABLED_ACCOUNT_MODEL: IAccountModel = { accounts: [ { @@ -235,11 +239,9 @@ describe("BlobVersioningAPIs", () => { true, "No version should be current after the delete" ); - assert.strictEqual( - version.hasVersionsOnly, - true, - "The blob should report that only versions remain" - ); + // HasVersionsOnly is NOT reported under include=versions. Verified against the + // real service, which returns it only under include=deletedwithversions. + assert.notStrictEqual(version.hasVersionsOnly, true); } // The blob itself is gone for callers that do not ask for a version @@ -285,9 +287,6 @@ describe("BlobVersioningAPIs", () => { [first.versionId, second.versionId, third.versionId] ); assert.strictEqual(versions[2].isCurrentVersion, true); - for (const version of versions) { - assert.notStrictEqual(version.hasVersionsOnly, true); - } const current = await blockBlobClient.download(); assert.strictEqual(await bodyToString(current, 8), "version3"); @@ -320,16 +319,24 @@ describe("BlobVersioningAPIs", () => { assert.strictEqual(snapshotCount, 0, "Snapshots should have been removed"); }); - it("Deleting a blob without versioning should still remove it @loki", async () => { - // Guards the non-versioned path, which must keep removing the blob outright. - // Covered here for the versioned account too via an explicit version delete of the - // only version, which is a hard delete rather than a demotion. + it("Deleting the current version by ID should be rejected @loki", async () => { + // Verified against the real service: a version ID delete may only target a previous + // version. The current version is removed by deleting the blob without a version ID. const only = await blockBlobClient.upload("version1", 8); - await blockBlobClient.withVersion(only.versionId!).delete(); - const versions = await listVersions(blobName); - assert.strictEqual(versions.length, 0); - assert.strictEqual(await blockBlobClient.exists(), false); + let error; + try { + await blockBlobClient.withVersion(only.versionId!).delete(); + } catch (err) { + error = err; + } + + assert.strictEqual((error as any)?.statusCode, 403); + assert.strictEqual((error as any)?.code, "OperationNotAllowedOnRootBlob"); + + // The blob and its version are untouched + assert.strictEqual((await listVersions(blobName)).length, 1); + assert.strictEqual(await blockBlobClient.exists(), true); }); it("Restore should be a copy of a previous version over the current one @loki", async () => { @@ -369,7 +376,7 @@ describe("BlobVersioningAPIs", () => { assert.strictEqual((error as any).statusCode, 400); assert.strictEqual( (error as any).code, - "InvalidQueryParameterValue", + "MutuallyExclusiveQueryParameters", "Error code should match the real service" ); }); @@ -517,32 +524,28 @@ describe("BlobVersioningAPIs", () => { assert.deepStrictEqual(current.metadata, { k: "v2" }); }); - it("Set Blob Properties should create a version for a block blob @loki", async () => { + it("Set Blob Properties should NOT create a version @loki", async () => { + // Verified against the real service: Set Blob Properties creates no version for any + // blob type and returns no x-ms-version-id, despite the prose docs saying every write + // on a block blob except Put Block creates one. The swagger agrees with the observed + // behaviour: it does not declare x-ms-version-id on this operation. const first = await blockBlobClient.upload("version1", 8); await blockBlobClient.setHTTPHeaders({ blobContentType: "text/plain" }); - // For block blobs every write except Put Block creates a version. Set Blob - // Properties does not return x-ms-version-id: the storage swagger does not declare - // that header on this operation. const versions = await listVersions(blobName); - assert.strictEqual(versions.length, 2); + assert.strictEqual(versions.length, 1); assert.strictEqual(versions[0].versionId, first.versionId); - assert.strictEqual(versions[1].isCurrentVersion, true); + assert.strictEqual(versions[0].isCurrentVersion, true); assert.strictEqual( (await blockBlobClient.getProperties()).contentType, "text/plain" ); - }); - it("Set Blob Properties should NOT create a version for a page blob @loki", async () => { - // For page and append blobs only Put Blob, Put Block List, Set Blob Metadata and - // Copy Blob create a version. const name = getUniqueName("page"); const pageClient = containerClient.getPageBlobClient(name); await pageClient.create(512); await pageClient.setHTTPHeaders({ blobContentType: "text/plain" }); - assert.strictEqual((await listVersions(name)).length, 1); }); @@ -692,7 +695,9 @@ describe("BlobVersioningAPIs", () => { }); }); -describe("BlobVersioningDisabledAPIs", () => { +// Asserts versioning is off, so it cannot run against a live account that has versioning +// enabled at the account level. +(LIVE_TEST_MODE ? describe.skip : describe)("BlobVersioningDisabledAPIs", () => { const factory = new BlobTestServerFactory(); const server = factory.createServer(); From 62cd4aeb4711a512cd998e2b405f4f17177b69fe Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 11:41:50 +0100 Subject: [PATCH 06/10] Allow blob versioning to be toggled, and retain versions when it is off Azurite refused to start when the account configuration flipped isVersioningEnabled against an existing workspace, on the assumption that the metadata would be left in a state matching neither setting. Verified against a real storage account, that assumption is wrong and the toggle is well defined: - Turning versioning off keeps existing versions listed under include=versions, readable by version ID, and deletable by version ID. - A write while versioning is off produces a blob that is not a version, but the previously current version is still retained rather than destroyed. A blob can therefore hold versions plus a current blob that is not one. - Turning versioning on over existing data is allowed. A blob written beforehand has no version ID until it is modified, at which point its prior state is captured as a version derived from its last modified time. So the start up conflict is removed for this setting, and the persisted configuration is updated rather than compared. The reconciliation machinery is kept for future settings that genuinely cannot change once data exists and would need a migration rather than a merge; that list is empty today. The retention rule is now expressed as a property of the document rather than of the account setting: an existing current blob is demoted to a previous version whenever it is itself a version, and removed outright only when it is not. That applies to Put Blob, Put Block List, Copy Blob and Delete Blob alike, and it is why turning versioning off does not destroy history. Adds tests/blob/apis/blob.versioning.toggle.test.ts, which restarts the server against the same workspace with different account configuration in both directions. That is how the emulator expresses an account level setting change, so it is skipped in live mode. BlobTestServerFactory gained an optional workspace name so two servers can share one metadata DB. Co-Authored-By: Claude --- ChangeLog.md | 1 + README.md | 10 +- docs/designs/blob-versioning.md | 25 ++- src/blob/persistence/LokiBlobMetadataStore.ts | 105 +++++++--- tests/BlobTestServerFactory.ts | 11 +- .../blob/apis/blob.versioning.toggle.test.ts | 182 ++++++++++++++++++ 6 files changed, 292 insertions(+), 42 deletions(-) create mode 100644 tests/blob/apis/blob.versioning.toggle.test.ts diff --git a/ChangeLog.md b/ChangeLog.md index ed0df4bad..ced0247fa 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -7,6 +7,7 @@ Blob: - Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options rather than through a data plane API. With versioning enabled for an account: overwriting a blob retains the previous content as a version and returns `x-ms-version-id`; `Get Blob` / `Get Blob Properties` accept `?versionid=` and return `x-ms-is-current-version` for the current version; `List Blobs` supports `include=versions`; `Delete Blob` with `?versionid=` removes a single version, and deleting the current version retains previous versions instead of failing with `SnapshotsPresent`; a version can be restored by copying it over the current version. Combining `?snapshot=` with `?versionid=` returns 400 `InvalidQueryParameterValue`. The setting is persisted with the workspace and cannot be changed against existing data. Versioning is not implemented for the SQL metadata store, and blob version SAS (`sr=bv`, the `x` permission), soft delete interactions, and blob expiration interactions are out of scope - see `docs/designs/blob-versioning.md`. +- Blob versioning can now be turned on and off against an existing workspace, matching the real service, rather than failing at start up. Turning it off keeps existing versions listed, readable and deletable by version ID, and a subsequent write retains the previously current version while producing a blob that is not itself a version. Turning it on over existing data captures a blob's prior state as a version when it is next modified. - Aligned blob versioning with behaviour observed against a real storage account: `Set Blob Properties` does not create a version, deleting the current version by version ID returns 403 `OperationNotAllowedOnRootBlob`, combining `?snapshot=` with `?versionid=` returns 400 `MutuallyExclusiveQueryParameters`, and `HasVersionsOnly` is not reported under `include=versions`. - Extended blob versioning to the remaining version creating operations: `Set Blob Metadata` (all blob types), `Set Blob Properties` (block blobs only), `Page Blob Create` and `Append Blob Create` now return `x-ms-version-id`, and `Snapshot Blob` on a versioned blob creates a new current version alongside the snapshot. `Put Page` and `Append Block` correctly do not create versions. `Get Blob Tags`, `Set Blob Tags` and `Set Blob Tier` accept `?versionid=`, and a malformed `?versionid=` returns 400 `InvalidQueryParameterValue`. - `Delete Blob` on a versioned blob without a version ID now turns the current version into a previous version and retains it, rather than removing it, and reports `HasVersionsOnly` for a blob that has versions but no current version. diff --git a/README.md b/README.md index a1a4d18a3..59b222c17 100644 --- a/README.md +++ b/README.md @@ -520,11 +520,11 @@ changes behaviour for accounts you did not name. If you run Azurite with versioning; a single configuration is not applied to all of them implicitly. The resolved configuration is written to the debug log at start up. -Versioning changes how blob writes are persisted, so it cannot be switched on or off -against a workspace that already holds data. The setting is persisted alongside the -metadata, and Azurite fails at start up if the configuration conflicts with the previous -run. To change it, use a clean workspace (a different `--location`, or remove the -existing one). +Versioning can be turned on and off freely, as on a real storage account. The setting is +persisted alongside the metadata, so starting Azurite against an existing workspace +without these options keeps whatever was configured last time. Turning versioning off +keeps existing versions readable by version ID; later writes simply stop creating new +ones. Versioning is implemented for the default LokiJS metadata store only. Configuring it together with the SQL based metadata implementation (via `AZURITE_DB`) is rejected at diff --git a/docs/designs/blob-versioning.md b/docs/designs/blob-versioning.md index 57205175d..c18c931bf 100644 --- a/docs/designs/blob-versioning.md +++ b/docs/designs/blob-versioning.md @@ -62,10 +62,8 @@ report shows which account settings were actually in effect. ### Changing the setting on an existing workspace -Versioning changes how writes are persisted, so switching it on or off against a -workspace that already contains blobs would leave the metadata store in a state that -matches neither setting. The configuration is therefore persisted in the metadata store -and reconciled at start up: +Versioning can be turned on and off freely, matching the real service. The configuration +is persisted in the metadata store and reconciled at start up: 1. Read the configuration persisted by the previous run. 2. Compare it with the configuration supplied on the command line. @@ -73,8 +71,23 @@ and reconciled at start up: input, and persist the result. 4. If there is a conflict, fail at start up with a message naming the account. -To flip the setting, start Azurite against a clean workspace (a different `--location`, -or remove the existing one). +Because the configuration persists, starting Azurite against an existing workspace +*without* `--accountConfig` keeps whatever was configured last time, just as the ARM +setting persists on a real account until it is changed. + +No blob service setting currently conflicts. Verified against the real service: + +- **Turning versioning off** keeps existing versions listed and readable by version ID, + and they can still be deleted by version ID. A subsequent write produces a blob that is + *not* a version, but the previously current version is still retained rather than + destroyed - so a blob can end up with versions plus a current blob that is not one. +- **Turning versioning on** over existing data is allowed. A blob written beforehand has + no version ID until it is modified, at which point its prior state is captured as a + version whose ID is derived from its last modified time. + +The conflict check is kept for future settings that genuinely cannot change once data +exists and would need a migration rather than a merge; the list of such settings is empty +today. ## Data model diff --git a/src/blob/persistence/LokiBlobMetadataStore.ts b/src/blob/persistence/LokiBlobMetadataStore.ts index bfe3296b9..f5a394810 100644 --- a/src/blob/persistence/LokiBlobMetadataStore.ts +++ b/src/blob/persistence/LokiBlobMetadataStore.ts @@ -5,6 +5,7 @@ import { randomUUID as uuid } from "crypto"; import { AccountConfigError, getAccountBlobServiceConfig, + IAccountBlobServiceConfig, IAccountConfig, IAccountModel } from "../../common/AccountModel"; @@ -112,6 +113,14 @@ import { * @export * @class LokiBlobMetadataStore */ +/** + * Account level blob service settings that cannot be changed once a workspace holds data, + * because doing so would leave the metadata in a state matching neither value. Empty + * today: blob versioning is safe to toggle. Any future setting that needs a migration + * rather than a merge belongs here. + */ +const IRRECONCILABLE_BLOB_SERVICE_SETTINGS: (keyof IAccountBlobServiceConfig)[] = []; + export default class LokiBlobMetadataStore implements IBlobMetadataStore, IGCExtentProvider { @@ -270,18 +279,25 @@ export default class LokiBlobMetadataStore for (const incoming of this.inputAccountModel?.accounts ?? []) { const previous = resolved.get(incoming.name); - if ( - previous !== undefined && - previous.blobService.isVersioningEnabled !== - incoming.blobService.isVersioningEnabled - ) { + // Blob versioning can be turned on and off freely, which is verified against the + // real service: disabling it keeps existing versions listable and readable, and a + // later write simply produces a blob that is not a version. So there is nothing to + // reconcile for it. + // + // The conflict check is kept for account settings that genuinely cannot change once + // data exists, which would need a migration rather than a merge. There are none + // today; adding one means listing it here. + const conflicting = IRRECONCILABLE_BLOB_SERVICE_SETTINGS.filter( + (setting) => + previous !== undefined && + previous.blobService[setting] !== incoming.blobService[setting] + ); + + if (conflicting.length > 0) { throw new AccountConfigError( - `Account "${incoming.name}" was previously started with blob versioning ` + - `${previous.blobService.isVersioningEnabled ? "enabled" : "disabled"} ` + - `but is now configured with blob versioning ` + - `${incoming.blobService.isVersioningEnabled ? "enabled" : "disabled"}. ` + - `Changing this setting against an existing workspace is not supported. ` + - `Either keep the previous setting, or start Azurite against a clean ` + + `Account "${incoming.name}" was previously started with different values for ` + + `${conflicting.join(", ")}, which cannot be changed once the workspace holds ` + + `data. Either keep the previous values, or start Azurite against a clean ` + `workspace (a different --location, or remove the existing one).` ); } @@ -290,6 +306,12 @@ export default class LokiBlobMetadataStore if (previous === undefined) { coll.insert({ name: incoming.name, blobService: incoming.blobService }); + } else { + const doc = coll.findOne({ name: incoming.name }); + if (doc !== null && doc !== undefined) { + doc.blobService = incoming.blobService; + coll.update(doc); + } } } @@ -424,6 +446,25 @@ export default class LokiBlobMetadataStore return coll.insert(copy); } + /** + * Whether an existing blob document is itself a version, and so must be retained rather + * than removed when it is overwritten or deleted. + * + * This is deliberately not the same question as "is versioning enabled". Verified + * against the real service: after versioning is turned off, overwriting a blob that has + * version history still retains the previously current version, and only the newly + * written blob is not a version. A blob with no version history is replaced outright, as + * it always was. + * + * @private + * @param {*} doc + * @returns {boolean} + * @memberof LokiBlobMetadataStore + */ + private isVersionDoc(doc: any): boolean { + return doc !== null && doc !== undefined && doc.versionId !== undefined; + } + /** * Build a Loki query that matches only the current version of a blob. * @@ -1369,8 +1410,9 @@ export default class LokiBlobMetadataStore throw StorageErrorFactory.getBlobArchived(context.contextId); } - if (versioningEnabled) { - // Retain the overwritten content as a previous version instead of removing it. + // Retain the overwritten content as a previous version when versioning is on, and + // also when it is off but the existing blob is itself a version. + if (versioningEnabled || this.isVersionDoc(blobDoc)) { this.demoteToPreviousVersion(coll, blobDoc); } else { coll.remove(blobDoc); @@ -1784,15 +1826,18 @@ export default class LokiBlobMetadataStore name: blob }); - if (!versioningEnabled) { - coll.findAndRemove(currentQuery); + const current = coll.findOne(currentQuery); + + // Retain the current version when versioning is on, and also when it is off but the + // current blob is itself a version. + if (versioningEnabled || this.isVersionDoc(current)) { + if (current !== null && current !== undefined) { + this.demoteToPreviousVersion(coll, current); + } return; } - const current = coll.findOne(currentQuery); - if (current !== null && current !== undefined) { - this.demoteToPreviousVersion(coll, current); - } + coll.findAndRemove(currentQuery); }; // Scenario: Delete base blob only @@ -2502,7 +2547,7 @@ export default class LokiBlobMetadataStore const versioningEnabled = this.isVersioningEnabled(destination.account); if (destBlob) { - if (versioningEnabled) { + if (versioningEnabled || this.isVersionDoc(destBlob)) { this.demoteToPreviousVersion(coll, destBlob); } else { coll.remove(destBlob); @@ -2715,7 +2760,7 @@ export default class LokiBlobMetadataStore const versioningEnabled = this.isVersioningEnabled(destination.account); if (destBlob) { - if (versioningEnabled) { + if (versioningEnabled || this.isVersionDoc(destBlob)) { this.demoteToPreviousVersion(coll, destBlob); } else { coll.remove(destBlob); @@ -3111,7 +3156,7 @@ export default class LokiBlobMetadataStore this.isVersioningEnabled(blob.accountName) && (blob.snapshot === "" || blob.snapshot === undefined); - if (versioningEnabled && doc && doc.isCommitted) { + if ((versioningEnabled || this.isVersionDoc(doc)) && doc && doc.isCommitted) { this.demoteToPreviousVersion(coll, doc); blob.committedBlocksInOrder = selectedBlockList; @@ -3120,13 +3165,15 @@ export default class LokiBlobMetadataStore .reduce((total, val) => { return total + val; }, 0); - blob.versionId = this.generateVersionId( - context, - blob.accountName, - blob.containerName, - blob.name - ); - blob.isCurrentVersion = true; + if (versioningEnabled) { + blob.versionId = this.generateVersionId( + context, + blob.accountName, + blob.containerName, + blob.name + ); + blob.isCurrentVersion = true; + } delete (blob as any).$loki; coll.insert(blob); } else if (doc) { diff --git a/tests/BlobTestServerFactory.ts b/tests/BlobTestServerFactory.ts index 4b5bd747b..39dbbff93 100644 --- a/tests/BlobTestServerFactory.ts +++ b/tests/BlobTestServerFactory.ts @@ -26,7 +26,10 @@ export default class BlobTestServerFactory { skipApiVersionCheck: boolean = false, https: boolean = false, oauth?: string, - accountModel?: IAccountModel + accountModel?: IAccountModel, + // Explicit workspace name, so a test can start two servers with different account + // configuration against the same metadata DB (for example to toggle versioning). + workspace?: string ): BlobServer | SqlBlobServer | LiveModeStubServer { if (LIVE_TEST_MODE) { return new LiveModeStubServer(); @@ -85,7 +88,11 @@ export default class BlobTestServerFactory { // Blob versioning cannot be switched on or off against an existing workspace, so // suites that configure it need their own metadata DB. const suffix = - accountModel !== undefined ? `_${accountModel.accounts.map((a) => `${a.name}-${a.blobService.isVersioningEnabled}`).join("_")}` : ""; + workspace !== undefined + ? `_${workspace}` + : accountModel !== undefined + ? `_${accountModel.accounts.map((a) => `${a.name}-${a.blobService.isVersioningEnabled}`).join("_")}` + : ""; const lokiMetadataDBPath = `__test_db_blob${suffix}__.json`; const lokiExtentDBPath = `__test_db_blob_extent${suffix}__.json`; const config = new BlobConfiguration( diff --git a/tests/blob/apis/blob.versioning.toggle.test.ts b/tests/blob/apis/blob.versioning.toggle.test.ts new file mode 100644 index 000000000..823e6677a --- /dev/null +++ b/tests/blob/apis/blob.versioning.toggle.test.ts @@ -0,0 +1,182 @@ +import { + BlobServiceClient, + newPipeline, + StorageSharedKeyCredential +} from "@azure/storage-blob"; +import * as assert from "assert"; + +import { IAccountModel } from "../../../src/common/AccountModel"; +import { configLogger } from "../../../src/common/Logger"; +import BlobTestServerFactory from "../../BlobTestServerFactory"; +import { + EMULATOR_ACCOUNT_KEY, + EMULATOR_ACCOUNT_NAME, + getTestServerBaseURL, + getUniqueName, + LIVE_TEST_MODE +} from "../../testutils"; + +// Set true to enable debug log +configLogger(false); + +const model = (isVersioningEnabled: boolean): IAccountModel => ({ + accounts: [{ name: EMULATOR_ACCOUNT_NAME, blobService: { isVersioningEnabled } }] +}); + +/** + * Turning versioning on and off is a supported operation on a real storage account, and + * these cases cover what happens to existing versions across the change. They restart the + * server against the same workspace with different account configuration, which is how + * the emulator expresses an account level setting change, so they cannot run in live mode. + */ +(LIVE_TEST_MODE ? describe.skip : describe)("BlobVersioningToggle", () => { + const factory = new BlobTestServerFactory(); + const workspace = "toggle"; + + function clientFor(server: { config: { host: string; port: number } }) { + return new BlobServiceClient( + getTestServerBaseURL(server), + newPipeline( + new StorageSharedKeyCredential( + EMULATOR_ACCOUNT_NAME, + EMULATOR_ACCOUNT_KEY + ), + { retryOptions: { maxTries: 1 }, keepAliveOptions: { enable: false } } + ) + ); + } + + it("Existing versions survive versioning being turned off @loki", async () => { + const containerName = getUniqueName("container"); + const blobName = getUniqueName("blob"); + + // --- Phase 1: versioning enabled --- + let server = factory.createServer( + false, + false, + false, + undefined, + model(true), + workspace + ); + await server.start(); + + let containerClient = clientFor(server).getContainerClient(containerName); + await containerClient.create(); + let blobClient = containerClient.getBlockBlobClient(blobName); + const first = await blobClient.upload("version1", 8); + const second = await blobClient.upload("version2", 8); + + const countVersions = async (client: typeof containerClient) => { + let n = 0; + for await (const item of client.listBlobsFlat({ includeVersions: true })) { + if (item.name === blobName) n++; + } + return n; + }; + + assert.strictEqual(await countVersions(containerClient), 2); + await server.close(); + + // --- Phase 2: same workspace, versioning disabled --- + server = factory.createServer( + false, + false, + false, + undefined, + model(false), + workspace + ); + await server.start(); + + containerClient = clientFor(server).getContainerClient(containerName); + blobClient = containerClient.getBlockBlobClient(blobName); + + // Existing versions are still listed and still readable by version ID + assert.strictEqual( + await countVersions(containerClient), + 2, + "Existing versions must survive versioning being disabled" + ); + const previous = await blobClient.withVersion(first.versionId!).download(); + assert.strictEqual(previous.contentLength, 8); + + // A write produces a blob that is not a version, but the previously current version + // is still retained rather than destroyed. + const third = await blobClient.upload("version3", 8); + assert.strictEqual( + third.versionId, + undefined, + "A write with versioning off must not return x-ms-version-id" + ); + assert.strictEqual( + await countVersions(containerClient), + 3, + "The previously current version must be retained alongside the new blob" + ); + assert.strictEqual( + (await blobClient.withVersion(second.versionId!).download()).contentLength, + 8, + "The version that was current when versioning was disabled must still be readable" + ); + + // Old versions can still be deleted by version ID while versioning is off + await blobClient.withVersion(first.versionId!).delete(); + assert.strictEqual(await countVersions(containerClient), 2); + + await containerClient.delete(); + await server.close(); + await server.clean(); + }); + + it("Versioning can be turned back on afterwards @loki", async () => { + const containerName = getUniqueName("container"); + const blobName = getUniqueName("blob"); + + let server = factory.createServer( + false, + false, + false, + undefined, + model(false), + workspace + "2" + ); + await server.start(); + let containerClient = clientFor(server).getContainerClient(containerName); + await containerClient.create(); + let blobClient = containerClient.getBlockBlobClient(blobName); + const unversioned = await blobClient.upload("version1", 8); + assert.strictEqual(unversioned.versionId, undefined); + await server.close(); + + // Enabling versioning on a workspace that already holds data is allowed. The blob + // written beforehand has no version ID until it is modified, at which point its prior + // state is captured as a version. + server = factory.createServer( + false, + false, + false, + undefined, + model(true), + workspace + "2" + ); + await server.start(); + containerClient = clientFor(server).getContainerClient(containerName); + blobClient = containerClient.getBlockBlobClient(blobName); + + const versioned = await blobClient.upload("version2", 8); + assert.notStrictEqual(versioned.versionId, undefined); + + let n = 0; + for await (const item of containerClient.listBlobsFlat({ + includeVersions: true + })) { + if (item.name === blobName) n++; + } + assert.strictEqual(n, 2, "The pre-existing state should be captured as a version"); + + await containerClient.delete(); + await server.close(); + await server.clean(); + }); +}); From 2fdb5c76044f03e19869bc9998ed48f8880920ae Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 13:21:01 +0100 Subject: [PATCH 07/10] Extract account configuration into a standalone store Account configuration was a collection inside the blob metadata database, which made it blob owned data even though the settings are per account. Queue and table would have had to read through the blob service to reach it. It now lives in src/common/account/ with its own database file, __azurite_db_account__.json: - AccountModel.ts, the model plus parsing and validation, moved here from src/common so the module is self contained. Deliberately not left under src/blob: a store in common should not depend on the blob layer. - IAccountModelStore.ts, the contract - lifecycle, resolve(), and getBlobServiceConfig(). - LokiAccountModelStore.ts, the Loki implementation, including the reconciliation of start up configuration against the previous run and the currently empty list of settings that cannot be changed once a workspace holds data. - index.ts as the module entry point. - IAccountModelEnvironment names the accountModel() contract that BlobEnvironment, Environment and VSCEnvironment already satisfy. LokiBlobMetadataStore now takes an IAccountModelStore and asks it whether versioning is enabled, instead of owning a collection and the reconciliation logic. It no longer needs a logger, since the only thing it logged was the resolved account configuration, which the new store reports itself. The blob service owns the store's lifecycle for now, as the only consumer: BlobServer inits it before the metadata store, closes it after, and cleans its database file alongside the others. When queue or table start reading account configuration this ownership has to move to the entry point and the instance be shared, because two Loki instances autosaving one file would corrupt it. That constraint is recorded on the field in BlobServer and in the design doc rather than left for someone to rediscover. Adds tests/common/LokiAccountModelStore.test.ts covering defaults, case insensitive account matching, persistence across runs, changing a persisted setting, merging a new account into existing configuration, clean refusing to run while open, and in memory mode writing no file. Verified: 898 tests pass against Azurite (up from 887, the difference being the new store tests), the versioning suite still passes in --inMemoryPersistence mode, and the 25 test versioning suite still passes against a real storage account. Co-Authored-By: Claude --- ChangeLog.md | 1 + docs/designs/blob-versioning.md | 18 +- src/blob/BlobConfiguration.ts | 4 +- src/blob/BlobEnvironment.ts | 2 +- src/blob/BlobServer.ts | 24 +- src/blob/BlobServerFactory.ts | 18 +- src/blob/IBlobEnvironment.ts | 2 +- src/blob/persistence/LokiBlobMetadataStore.ts | 136 +----------- src/common/Environment.ts | 2 +- src/common/EnvironmentFunctions.ts | 2 +- src/common/IAccountModelEnvironment.ts | 20 ++ src/common/VSCEnvironment.ts | 2 +- src/common/{ => account}/AccountModel.ts | 0 src/common/account/IAccountModelStore.ts | 52 +++++ src/common/account/LokiAccountModelStore.ts | 210 ++++++++++++++++++ src/common/account/index.ts | 11 + src/common/utils/constants.ts | 8 + tests/BlobTestServerFactory.ts | 11 +- tests/blob/apis/blob.versioning.test.ts | 2 +- .../blob/apis/blob.versioning.toggle.test.ts | 2 +- tests/common/AccountModel.test.ts | 2 +- tests/common/LokiAccountModelStore.test.ts | 143 ++++++++++++ 22 files changed, 526 insertions(+), 146 deletions(-) create mode 100644 src/common/IAccountModelEnvironment.ts rename src/common/{ => account}/AccountModel.ts (100%) create mode 100644 src/common/account/IAccountModelStore.ts create mode 100644 src/common/account/LokiAccountModelStore.ts create mode 100644 src/common/account/index.ts create mode 100644 tests/common/LokiAccountModelStore.test.ts diff --git a/ChangeLog.md b/ChangeLog.md index ced0247fa..424e4574f 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -7,6 +7,7 @@ Blob: - Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options rather than through a data plane API. With versioning enabled for an account: overwriting a blob retains the previous content as a version and returns `x-ms-version-id`; `Get Blob` / `Get Blob Properties` accept `?versionid=` and return `x-ms-is-current-version` for the current version; `List Blobs` supports `include=versions`; `Delete Blob` with `?versionid=` removes a single version, and deleting the current version retains previous versions instead of failing with `SnapshotsPresent`; a version can be restored by copying it over the current version. Combining `?snapshot=` with `?versionid=` returns 400 `InvalidQueryParameterValue`. The setting is persisted with the workspace and cannot be changed against existing data. Versioning is not implemented for the SQL metadata store, and blob version SAS (`sr=bv`, the `x` permission), soft delete interactions, and blob expiration interactions are out of scope - see `docs/designs/blob-versioning.md`. +- Moved account (management plane) configuration into a standalone `src/common/account/` module with its own database file (`__azurite_db_account__.json`), so that the queue and table services can share it rather than reading configuration owned by the blob service. - Blob versioning can now be turned on and off against an existing workspace, matching the real service, rather than failing at start up. Turning it off keeps existing versions listed, readable and deletable by version ID, and a subsequent write retains the previously current version while producing a blob that is not itself a version. Turning it on over existing data captures a blob's prior state as a version when it is next modified. - Aligned blob versioning with behaviour observed against a real storage account: `Set Blob Properties` does not create a version, deleting the current version by version ID returns 403 `OperationNotAllowedOnRootBlob`, combining `?snapshot=` with `?versionid=` returns 400 `MutuallyExclusiveQueryParameters`, and `HasVersionsOnly` is not reported under `include=versions`. - Extended blob versioning to the remaining version creating operations: `Set Blob Metadata` (all blob types), `Set Blob Properties` (block blobs only), `Page Blob Create` and `Append Blob Create` now return `x-ms-version-id`, and `Snapshot Blob` on a versioned blob creates a new current version alongside the snapshot. `Put Page` and `Append Block` correctly do not create versions. `Get Blob Tags`, `Set Blob Tags` and `Set Blob Tier` accept `?versionid=`, and a malformed `?versionid=` returns 400 `InvalidQueryParameterValue`. diff --git a/docs/designs/blob-versioning.md b/docs/designs/blob-versioning.md index c18c931bf..37b7084bc 100644 --- a/docs/designs/blob-versioning.md +++ b/docs/designs/blob-versioning.md @@ -101,9 +101,21 @@ Blobs written before versioning was enabled have no `versionId` and no `isCurrentVersion: { $ne: false }` rather than `isCurrentVersion: true`, so those blobs continue to resolve. -Account configuration lives in its own collection (`$ACCOUNTS_COLLECTION$`) rather than -alongside blob documents, so that the queue and table services can reuse it when they -need account level settings. +### Account configuration store + +Account configuration lives in `src/common/account/`, outside the blob service, with its +own database file (`__azurite_db_account__.json`) rather than a collection inside the blob +metadata database. The settings are per account rather than per service, so queue and table +can read the same store when they need account level settings. + +- `AccountModel.ts` - the model and its parsing/validation +- `IAccountModelStore.ts` - the contract: lifecycle, `resolve()`, `getBlobServiceConfig()` +- `LokiAccountModelStore.ts` - the Loki implementation + +The blob service currently owns the store's lifecycle, because it is the only consumer. +**When queue or table start reading account configuration, ownership has to move to the +entry point and the instance be shared** - two Loki instances autosaving the same file +would corrupt it. That is noted on the field in `BlobServer` as well as here. ### List continuation tokens diff --git a/src/blob/BlobConfiguration.ts b/src/blob/BlobConfiguration.ts index a3453e3e1..46831f82c 100644 --- a/src/blob/BlobConfiguration.ts +++ b/src/blob/BlobConfiguration.ts @@ -1,4 +1,4 @@ -import { IAccountModel } from "../common/AccountModel"; +import IAccountModelStore from "../common/account/IAccountModelStore"; import ConfigurationBase from "../common/ConfigurationBase"; import { StoreDestinationArray } from "../common/persistence/IExtentStore"; import { MemoryExtentChunkStore } from "../common/persistence/MemoryExtentStore"; @@ -46,7 +46,7 @@ export default class BlobConfiguration extends ConfigurationBase { disableProductStyleUrl: boolean = false, public readonly isMemoryPersistence: boolean = false, public readonly memoryStore?: MemoryExtentChunkStore, - public readonly accountModel?: IAccountModel, + public readonly accountModelStore?: IAccountModelStore, ) { super( host, diff --git a/src/blob/BlobEnvironment.ts b/src/blob/BlobEnvironment.ts index 14642da24..a8e263065 100644 --- a/src/blob/BlobEnvironment.ts +++ b/src/blob/BlobEnvironment.ts @@ -2,7 +2,7 @@ import args from "args"; import { access, ensureDir } from "fs-extra"; import { dirname } from "path"; -import { IAccountModel } from "../common/AccountModel"; +import { IAccountModel } from "../common/account/AccountModel"; import { resolveAccountModel } from "../common/EnvironmentFunctions"; import IBlobEnvironment from "./IBlobEnvironment"; import { diff --git a/src/blob/BlobServer.ts b/src/blob/BlobServer.ts index c78d8dfc1..5b7a2a1f3 100644 --- a/src/blob/BlobServer.ts +++ b/src/blob/BlobServer.ts @@ -21,6 +21,7 @@ import BlobConfiguration from "./BlobConfiguration"; import BlobRequestListenerFactory from "./BlobRequestListenerFactory"; import BlobGCManager from "./gc/BlobGCManager"; import IBlobMetadataStore from "./persistence/IBlobMetadataStore"; +import IAccountModelStore from "../common/account/IAccountModelStore"; import LokiBlobMetadataStore from "./persistence/LokiBlobMetadataStore"; import StorageError from "./errors/StorageError"; @@ -43,6 +44,13 @@ const AFTER_CLOSE_MESSAGE = `Azurite Blob service successfully closed`; */ export default class BlobServer extends ServerBase implements ICleaner { private readonly metadataStore: IBlobMetadataStore; + /** + * Account (management plane) configuration store. The blob service owns its lifecycle + * because it is currently the only consumer. When queue or table start reading account + * configuration, ownership must move to the entry point and the instance be shared: + * two Loki instances autosaving the same file would corrupt it. + */ + private readonly accountModelStore?: IAccountModelStore; private readonly extentMetadataStore: IExtentMetadataStore; private readonly extentStore: IExtentStore; private readonly accountDataStore: IAccountDataStore; @@ -80,8 +88,7 @@ export default class BlobServer extends ServerBase implements ICleaner { const metadataStore: IBlobMetadataStore = new LokiBlobMetadataStore( configuration.metadataDBPath, configuration.isMemoryPersistence, - configuration.accountModel, - logger + configuration.accountModelStore ); const extentMetadataStore: IExtentMetadataStore = @@ -156,6 +163,7 @@ export default class BlobServer extends ServerBase implements ICleaner { ); this.metadataStore = metadataStore; + this.accountModelStore = configuration.accountModelStore; this.extentMetadataStore = extentMetadataStore; this.extentStore = extentStore; this.accountDataStore = accountDataStore; @@ -183,6 +191,10 @@ export default class BlobServer extends ServerBase implements ICleaner { await this.metadataStore.clean(); } + if (this.accountModelStore !== undefined) { + await this.accountModelStore.clean(); + } + if (this.accountDataStore !== undefined) { await this.accountDataStore.clean(); } @@ -199,6 +211,10 @@ export default class BlobServer extends ServerBase implements ICleaner { await this.accountDataStore.init(); } + if (this.accountModelStore !== undefined) { + await this.accountModelStore.init(); + } + if (this.metadataStore !== undefined) { await this.metadataStore.init(); } @@ -242,6 +258,10 @@ export default class BlobServer extends ServerBase implements ICleaner { await this.metadataStore.close(); } + if (this.accountModelStore !== undefined) { + await this.accountModelStore.close(); + } + if (this.accountDataStore !== undefined) { await this.accountDataStore.close(); } diff --git a/src/blob/BlobServerFactory.ts b/src/blob/BlobServerFactory.ts index 845d7dbda..eb5a30843 100644 --- a/src/blob/BlobServerFactory.ts +++ b/src/blob/BlobServerFactory.ts @@ -1,6 +1,11 @@ import { join } from "path"; -import { DEFAULT_SQL_OPTIONS } from "../common/utils/constants"; +import LokiAccountModelStore from "../common/account/LokiAccountModelStore"; +import logger from "../common/Logger"; +import { + DEFAULT_ACCOUNT_LOKI_DB_PATH, + DEFAULT_SQL_OPTIONS +} from "../common/utils/constants"; import BlobConfiguration from "./BlobConfiguration"; import BlobEnvironment from "./BlobEnvironment"; import BlobServer from "./BlobServer"; @@ -80,6 +85,15 @@ export class BlobServerFactory { return new SqlBlobServer(config); } else { + // The account configuration store keeps its own database file so that queue and + // table can share it later. Blob owns its lifecycle for now, as the only consumer. + const accountModelStore = new LokiAccountModelStore( + join(location, DEFAULT_ACCOUNT_LOKI_DB_PATH), + env.inMemoryPersistence(), + accountModel?.accounts ?? [], + logger + ); + const config = new BlobConfiguration( env.blobHost(), env.blobPort(), @@ -100,7 +114,7 @@ export class BlobServerFactory { env.disableProductStyleUrl(), env.inMemoryPersistence(), undefined, - accountModel, + accountModelStore, ); return new BlobServer(config); diff --git a/src/blob/IBlobEnvironment.ts b/src/blob/IBlobEnvironment.ts index 10b9cba75..d9e7838f8 100644 --- a/src/blob/IBlobEnvironment.ts +++ b/src/blob/IBlobEnvironment.ts @@ -1,4 +1,4 @@ -import { IAccountModel } from "../common/AccountModel"; +import { IAccountModel } from "../common/account/AccountModel"; export default interface IBlobEnvironment { blobHost(): string | undefined; diff --git a/src/blob/persistence/LokiBlobMetadataStore.ts b/src/blob/persistence/LokiBlobMetadataStore.ts index f5a394810..f864af389 100644 --- a/src/blob/persistence/LokiBlobMetadataStore.ts +++ b/src/blob/persistence/LokiBlobMetadataStore.ts @@ -2,15 +2,8 @@ import { stat } from "fs"; import Loki from "lokijs"; import { randomUUID as uuid } from "crypto"; -import { - AccountConfigError, - getAccountBlobServiceConfig, - IAccountBlobServiceConfig, - IAccountConfig, - IAccountModel -} from "../../common/AccountModel"; +import IAccountModelStore from "../../common/account/IAccountModelStore"; import IGCExtentProvider from "../../common/IGCExtentProvider"; -import ILogger from "../../common/ILogger"; import { convertDateTimeStringMsTo7Digital, rimrafAsync @@ -113,14 +106,6 @@ import { * @export * @class LokiBlobMetadataStore */ -/** - * Account level blob service settings that cannot be changed once a workspace holds data, - * because doing so would leave the metadata in a state matching neither value. Empty - * today: blob versioning is safe to toggle. Any future setting that needs a migration - * rather than a merge belongs here. - */ -const IRRECONCILABLE_BLOB_SERVICE_SETTINGS: (keyof IAccountBlobServiceConfig)[] = []; - export default class LokiBlobMetadataStore implements IBlobMetadataStore, IGCExtentProvider { @@ -133,22 +118,17 @@ export default class LokiBlobMetadataStore private readonly CONTAINERS_COLLECTION = "$CONTAINERS_COLLECTION$"; private readonly BLOBS_COLLECTION = "$BLOBS_COLLECTION$"; private readonly BLOCKS_COLLECTION = "$BLOCKS_COLLECTION$"; - private readonly ACCOUNTS_COLLECTION = "$ACCOUNTS_COLLECTION$"; private readonly pageBlobRangesManager = new PageBlobRangesManager(); - /** - * Account level configuration in effect for this run, resolved during init() from the - * configuration supplied on the command line merged with the configuration persisted - * by the previous run. - */ - private accountConfigs: Map = new Map(); - public constructor( public readonly lokiDBPath: string, inMemory: boolean, - private readonly inputAccountModel?: IAccountModel, - private readonly logger?: ILogger + /** + * Account level configuration, owned by its own store so that queue and table can + * share it. Undefined leaves every account on the defaults. + */ + private readonly accountModelStore?: IAccountModelStore ) { this.db = new Loki( lokiDBPath, @@ -223,17 +203,6 @@ export default class LokiBlobMetadataStore }); } - // Create account configuration collection if not exists. Kept in its own - // collection (rather than alongside blob documents) so that queue and table can - // reuse the same account configuration when they need account level settings. - if (this.db.getCollection(this.ACCOUNTS_COLLECTION) === null) { - this.db.addCollection(this.ACCOUNTS_COLLECTION, { - unique: ["name"] - }); - } - - this.resolveAccountConfigs(); - await new Promise((resolve, reject) => { this.db.saveDatabase((err) => { if (err) { @@ -248,92 +217,6 @@ export default class LokiBlobMetadataStore this.closed = false; } - /** - * Resolve the account level configuration for this run. - * - * Blob versioning changes how blob writes are persisted, so switching it on or off - * against an existing workspace would leave the metadata store in a state that does - * not match either setting. The resolution rules are therefore: - * - * 1. Read the configuration persisted by the previous run. - * 2. Compare it with the configuration supplied on the command line. - * 3. If there is no conflict, run with the previous configuration merged with the - * new input, and persist the result. - * 4. If there is a conflict, fail at start up with an actionable message. - * - * @private - * @memberof LokiBlobMetadataStore - */ - private resolveAccountConfigs(): void { - const coll = this.db.getCollection(this.ACCOUNTS_COLLECTION); - const persisted: IAccountConfig[] = coll.find({}).map((doc: any) => ({ - name: doc.name, - blobService: { ...doc.blobService } - })); - - const resolved = new Map(); - for (const account of persisted) { - resolved.set(account.name, account); - } - - for (const incoming of this.inputAccountModel?.accounts ?? []) { - const previous = resolved.get(incoming.name); - - // Blob versioning can be turned on and off freely, which is verified against the - // real service: disabling it keeps existing versions listable and readable, and a - // later write simply produces a blob that is not a version. So there is nothing to - // reconcile for it. - // - // The conflict check is kept for account settings that genuinely cannot change once - // data exists, which would need a migration rather than a merge. There are none - // today; adding one means listing it here. - const conflicting = IRRECONCILABLE_BLOB_SERVICE_SETTINGS.filter( - (setting) => - previous !== undefined && - previous.blobService[setting] !== incoming.blobService[setting] - ); - - if (conflicting.length > 0) { - throw new AccountConfigError( - `Account "${incoming.name}" was previously started with different values for ` + - `${conflicting.join(", ")}, which cannot be changed once the workspace holds ` + - `data. Either keep the previous values, or start Azurite against a clean ` + - `workspace (a different --location, or remove the existing one).` - ); - } - - resolved.set(incoming.name, incoming); - - if (previous === undefined) { - coll.insert({ name: incoming.name, blobService: incoming.blobService }); - } else { - const doc = coll.findOne({ name: incoming.name }); - if (doc !== null && doc !== undefined) { - doc.blobService = incoming.blobService; - coll.update(doc); - } - } - } - - this.accountConfigs = resolved; - - // Print the resolved configuration so that Azurite issue reports include the - // account settings that were actually in effect. - if (this.logger !== undefined) { - if (resolved.size === 0) { - this.logger.debug( - `LokiBlobMetadataStore:resolveAccountConfigs() No account level configuration supplied or persisted, using defaults (blob versioning disabled).` - ); - } else { - this.logger.debug( - `LokiBlobMetadataStore:resolveAccountConfigs() Account level configuration in effect: ${JSON.stringify( - [...resolved.values()] - )}` - ); - } - } - } - /** * Whether blob versioning is enabled for the given account. * @@ -343,10 +226,9 @@ export default class LokiBlobMetadataStore * @memberof LokiBlobMetadataStore */ private isVersioningEnabled(account: string): boolean { - const config = this.accountConfigs.get(account.toLowerCase()); - return config !== undefined - ? config.blobService.isVersioningEnabled - : getAccountBlobServiceConfig(undefined, account).isVersioningEnabled; + return this.accountModelStore === undefined + ? false + : this.accountModelStore.getBlobServiceConfig(account).isVersioningEnabled; } /** diff --git a/src/common/Environment.ts b/src/common/Environment.ts index 6a241de1b..e8af2d6d4 100644 --- a/src/common/Environment.ts +++ b/src/common/Environment.ts @@ -18,7 +18,7 @@ import { DEFAULT_TABLE_SERVER_HOST_NAME } from "../table/utils/constants"; -import { IAccountModel } from "./AccountModel"; +import { IAccountModel } from "./account/AccountModel"; import { resolveAccountModel } from "./EnvironmentFunctions"; import IEnvironment from "./IEnvironment"; import { shouldSkipApiVersionCheck } from "./utils/environment"; diff --git a/src/common/EnvironmentFunctions.ts b/src/common/EnvironmentFunctions.ts index c5bfd395d..e5d14e8b1 100644 --- a/src/common/EnvironmentFunctions.ts +++ b/src/common/EnvironmentFunctions.ts @@ -4,7 +4,7 @@ import { AccountConfigError, IAccountModel, parseAccountModel -} from "./AccountModel"; +} from "./account/AccountModel"; /** * Shared helpers for reading environment/command line options that are used by more diff --git a/src/common/IAccountModelEnvironment.ts b/src/common/IAccountModelEnvironment.ts new file mode 100644 index 000000000..01f9cb6bf --- /dev/null +++ b/src/common/IAccountModelEnvironment.ts @@ -0,0 +1,20 @@ +import { IAccountModel } from "./account/AccountModel"; + +/** + * Implemented by every Azurite entry point's environment, so that account (management + * plane) configuration is read the same way whether Azurite is started as `azurite`, + * `azurite-blob`, or from the VS Code extension. + * + * @export + * @interface IAccountModelEnvironment + */ +export default interface IAccountModelEnvironment { + /** + * Account configuration supplied at start up, or undefined when none was given, in which + * case the configuration persisted by the previous run is used. + * + * @returns {Promise} + * @memberof IAccountModelEnvironment + */ + accountModel(): Promise; +} diff --git a/src/common/VSCEnvironment.ts b/src/common/VSCEnvironment.ts index 02037fce0..fc07cbbf8 100644 --- a/src/common/VSCEnvironment.ts +++ b/src/common/VSCEnvironment.ts @@ -2,7 +2,7 @@ import { access, ensureDir } from "fs-extra"; import { isAbsolute, resolve } from "path"; import { window, workspace, WorkspaceFolder } from "vscode"; -import { IAccountModel } from "./AccountModel"; +import { IAccountModel } from "./account/AccountModel"; import { resolveAccountModel } from "./EnvironmentFunctions"; import IEnvironment from "./IEnvironment"; diff --git a/src/common/AccountModel.ts b/src/common/account/AccountModel.ts similarity index 100% rename from src/common/AccountModel.ts rename to src/common/account/AccountModel.ts diff --git a/src/common/account/IAccountModelStore.ts b/src/common/account/IAccountModelStore.ts new file mode 100644 index 000000000..982fafa8c --- /dev/null +++ b/src/common/account/IAccountModelStore.ts @@ -0,0 +1,52 @@ +import ICleaner from "../ICleaner"; +import IDataStore from "../IDataStore"; +import { IAccountBlobServiceConfig, IAccountConfig } from "./AccountModel"; + +/** + * Persistence for account (management plane) configuration. + * + * Azure Storage keeps account level settings such as blob versioning on the ARM + * management plane, which Azurite does not emulate. Azurite instead takes the settings at + * start up and persists them alongside the workspace, so that a run without the options + * behaves the same as the previous run - just as the ARM setting persists on a real + * account until it is changed. + * + * This lives outside the blob service because the settings are per account rather than per + * service: queue and table can read the same store when they need account level settings, + * without the blob service owning the data. + * + * @export + * @interface IAccountModelStore + */ +export default interface IAccountModelStore extends IDataStore, ICleaner { + /** + * Reconcile the configuration supplied at start up with the configuration persisted by + * the previous run, and persist the result. + * + * Called once during init(). Throws AccountConfigError when a setting that cannot be + * changed once the workspace holds data has a different value. + * + * @param {IAccountConfig[]} incoming Configuration supplied at start up, may be empty + * @returns {Promise} + * @memberof IAccountModelStore + */ + resolve(incoming: IAccountConfig[]): Promise; + + /** + * Blob service configuration in effect for an account, falling back to the defaults when + * the account has no configuration. Account names are matched case insensitively. + * + * @param {string} account + * @returns {IAccountBlobServiceConfig} + * @memberof IAccountModelStore + */ + getBlobServiceConfig(account: string): IAccountBlobServiceConfig; + + /** + * Every account configuration in effect, for diagnostics. + * + * @returns {IAccountConfig[]} + * @memberof IAccountModelStore + */ + listConfigs(): IAccountConfig[]; +} diff --git a/src/common/account/LokiAccountModelStore.ts b/src/common/account/LokiAccountModelStore.ts new file mode 100644 index 000000000..f5e3d6b81 --- /dev/null +++ b/src/common/account/LokiAccountModelStore.ts @@ -0,0 +1,210 @@ +import { stat } from "fs"; +import Loki from "lokijs"; + +import ILogger from "../ILogger"; +import { rimrafAsync } from "../utils/utils"; +import { + AccountConfigError, + DEFAULT_ACCOUNT_BLOB_SERVICE_CONFIG, + IAccountBlobServiceConfig, + IAccountConfig +} from "./AccountModel"; +import IAccountModelStore from "./IAccountModelStore"; + +/** + * Account level blob service settings that cannot be changed once a workspace holds data, + * because doing so would leave the metadata in a state matching neither value. + * + * Empty today. Blob versioning is safe to toggle: verified against the real service, + * turning it off keeps existing versions listed, readable and deletable by version ID, and + * turning it on captures a pre-existing blob's state as a version when it is next modified. + * + * Any future setting that would need a migration rather than a merge belongs here. + */ +const IRRECONCILABLE_BLOB_SERVICE_SETTINGS: (keyof IAccountBlobServiceConfig)[] = + []; + +/** + * Loki backed implementation of IAccountModelStore, with its own database file so that the + * blob, queue and table services can share one account configuration. + * + * @export + * @class LokiAccountModelStore + * @implements {IAccountModelStore} + */ +export default class LokiAccountModelStore implements IAccountModelStore { + private readonly db: Loki; + + private initialized: boolean = false; + private closed: boolean = true; + + private readonly ACCOUNTS_COLLECTION = "$ACCOUNTS_COLLECTION$"; + + /** + * Configuration in effect for this run, resolved during init(). + */ + private configs: Map = new Map(); + + public constructor( + public readonly lokiDBPath: string, + inMemory: boolean, + private readonly incoming: IAccountConfig[] = [], + private readonly logger?: ILogger + ) { + this.db = new Loki( + lokiDBPath, + inMemory + ? { + persistenceMethod: "memory" + } + : { + persistenceMethod: "fs", + autosave: true, + autosaveInterval: 5000 + } + ); + } + + public isInitialized(): boolean { + return this.initialized; + } + + public isClosed(): boolean { + return this.closed; + } + + public async init(): Promise { + await new Promise((resolve, reject) => { + stat(this.lokiDBPath, (statError) => { + if (!statError) { + this.db.loadDatabase({}, (dbError) => { + if (dbError) { + reject(dbError); + } else { + resolve(); + } + }); + } else { + // when the DB file doesn't exist, ignore the error because the following will + // re-create the file + resolve(); + } + }); + }); + + if (this.db.getCollection(this.ACCOUNTS_COLLECTION) === null) { + this.db.addCollection(this.ACCOUNTS_COLLECTION, { + unique: ["name"] + }); + } + + await this.resolve(this.incoming); + + await new Promise((resolve, reject) => { + this.db.saveDatabase((err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); + + this.initialized = true; + this.closed = false; + } + + public async close(): Promise { + await new Promise((resolve, reject) => { + this.db.close((err) => { + if (err) { + reject(err); + } else { + resolve(); + } + }); + }); + + this.closed = true; + } + + public async clean(): Promise { + if (this.isClosed()) { + await rimrafAsync(this.lokiDBPath); + return; + } + throw new Error(`Cannot clean LokiAccountModelStore, it's not closed.`); + } + + public async resolve(incoming: IAccountConfig[]): Promise { + const coll = this.db.getCollection(this.ACCOUNTS_COLLECTION); + + const resolved = new Map(); + for (const doc of coll.find({}) as any[]) { + resolved.set(doc.name, { + name: doc.name, + blobService: { ...doc.blobService } + }); + } + + for (const account of incoming) { + const previous = resolved.get(account.name); + + const conflicting = IRRECONCILABLE_BLOB_SERVICE_SETTINGS.filter( + (setting) => + previous !== undefined && + previous.blobService[setting] !== account.blobService[setting] + ); + + if (conflicting.length > 0) { + throw new AccountConfigError( + `Account "${account.name}" was previously started with different values for ` + + `${conflicting.join(", ")}, which cannot be changed once the workspace holds ` + + `data. Either keep the previous values, or start Azurite against a clean ` + + `workspace (a different --location, or remove the existing one).` + ); + } + + resolved.set(account.name, account); + + if (previous === undefined) { + coll.insert({ name: account.name, blobService: account.blobService }); + } else { + const doc = coll.findOne({ name: account.name }); + if (doc !== null && doc !== undefined) { + doc.blobService = account.blobService; + coll.update(doc); + } + } + } + + this.configs = resolved; + + // Print the resolved configuration so that Azurite issue reports include the account + // settings that were actually in effect. + if (this.logger !== undefined) { + if (resolved.size === 0) { + this.logger.debug( + `LokiAccountModelStore:resolve() No account level configuration supplied or persisted, using defaults (blob versioning disabled).` + ); + } else { + this.logger.debug( + `LokiAccountModelStore:resolve() Account level configuration in effect: ${JSON.stringify( + [...resolved.values()] + )}` + ); + } + } + } + + public getBlobServiceConfig(account: string): IAccountBlobServiceConfig { + const config = this.configs.get(account.toLowerCase()); + return config === undefined + ? DEFAULT_ACCOUNT_BLOB_SERVICE_CONFIG + : config.blobService; + } + + public listConfigs(): IAccountConfig[] { + return [...this.configs.values()]; + } +} diff --git a/src/common/account/index.ts b/src/common/account/index.ts new file mode 100644 index 000000000..180891d7a --- /dev/null +++ b/src/common/account/index.ts @@ -0,0 +1,11 @@ +export { + AccountConfigError, + DEFAULT_ACCOUNT_BLOB_SERVICE_CONFIG, + getAccountBlobServiceConfig, + IAccountBlobServiceConfig, + IAccountConfig, + IAccountModel, + parseAccountModel +} from "./AccountModel"; +export { default as IAccountModelStore } from "./IAccountModelStore"; +export { default as LokiAccountModelStore } from "./LokiAccountModelStore"; diff --git a/src/common/utils/constants.ts b/src/common/utils/constants.ts index c4fa903f5..cad5f2380 100644 --- a/src/common/utils/constants.ts +++ b/src/common/utils/constants.ts @@ -60,3 +60,11 @@ export const EMULATOR_ACCOUNT_KEY = Buffer.from( ); export const VALID_CSHARP_IDENTIFIER_REGEX = /^[a-zA-Z_][a-zA-Z0-9_]*$/; + +/** + * Account (management plane) configuration database. Kept separate from the per service + * databases because the settings are per account: the blob service owns this file today, + * but queue and table can read the same configuration when they need account level + * settings. + */ +export const DEFAULT_ACCOUNT_LOKI_DB_PATH = "__azurite_db_account__.json"; diff --git a/tests/BlobTestServerFactory.ts b/tests/BlobTestServerFactory.ts index 39dbbff93..e0d0a468e 100644 --- a/tests/BlobTestServerFactory.ts +++ b/tests/BlobTestServerFactory.ts @@ -5,7 +5,8 @@ import SqlBlobServer from "../src/blob/SqlBlobServer"; import { StoreDestinationArray } from "../src/common/persistence/IExtentStore"; import { DEFAULT_SQL_OPTIONS } from "../src/common/utils/constants"; import { DEFAULT_BLOB_KEEP_ALIVE_TIMEOUT } from "../src/blob/utils/constants"; -import { IAccountModel } from "../src/common/AccountModel"; +import { IAccountModel } from "../src/common/account/AccountModel"; +import LokiAccountModelStore from "../src/common/account/LokiAccountModelStore"; import { LIVE_TEST_MODE } from "./testutils"; /** @@ -95,6 +96,12 @@ export default class BlobTestServerFactory { : ""; const lokiMetadataDBPath = `__test_db_blob${suffix}__.json`; const lokiExtentDBPath = `__test_db_blob_extent${suffix}__.json`; + const lokiAccountDBPath = `__test_db_account${suffix}__.json`; + const accountModelStore = new LokiAccountModelStore( + lokiAccountDBPath, + inMemoryPersistence, + accountModel?.accounts ?? [] + ); const config = new BlobConfiguration( host, port, @@ -115,7 +122,7 @@ export default class BlobTestServerFactory { undefined, inMemoryPersistence, undefined, - accountModel + accountModelStore ); return new BlobServer(config); } diff --git a/tests/blob/apis/blob.versioning.test.ts b/tests/blob/apis/blob.versioning.test.ts index 3ce695c31..691dc5ebb 100644 --- a/tests/blob/apis/blob.versioning.test.ts +++ b/tests/blob/apis/blob.versioning.test.ts @@ -5,7 +5,7 @@ import { } from "@azure/storage-blob"; import * as assert from "assert"; -import { IAccountModel } from "../../../src/common/AccountModel"; +import { IAccountModel } from "../../../src/common/account/AccountModel"; import { configLogger } from "../../../src/common/Logger"; import BlobTestServerFactory from "../../BlobTestServerFactory"; import { diff --git a/tests/blob/apis/blob.versioning.toggle.test.ts b/tests/blob/apis/blob.versioning.toggle.test.ts index 823e6677a..6a116e857 100644 --- a/tests/blob/apis/blob.versioning.toggle.test.ts +++ b/tests/blob/apis/blob.versioning.toggle.test.ts @@ -5,7 +5,7 @@ import { } from "@azure/storage-blob"; import * as assert from "assert"; -import { IAccountModel } from "../../../src/common/AccountModel"; +import { IAccountModel } from "../../../src/common/account/AccountModel"; import { configLogger } from "../../../src/common/Logger"; import BlobTestServerFactory from "../../BlobTestServerFactory"; import { diff --git a/tests/common/AccountModel.test.ts b/tests/common/AccountModel.test.ts index f32214f2e..d8240ac65 100644 --- a/tests/common/AccountModel.test.ts +++ b/tests/common/AccountModel.test.ts @@ -4,7 +4,7 @@ import { AccountConfigError, getAccountBlobServiceConfig, parseAccountModel -} from "../../src/common/AccountModel"; +} from "../../src/common/account/AccountModel"; import { resolveAccountModel } from "../../src/common/EnvironmentFunctions"; describe("AccountModel @loki", () => { diff --git a/tests/common/LokiAccountModelStore.test.ts b/tests/common/LokiAccountModelStore.test.ts new file mode 100644 index 000000000..10f4ad23b --- /dev/null +++ b/tests/common/LokiAccountModelStore.test.ts @@ -0,0 +1,143 @@ +import * as assert from "assert"; +import { existsSync } from "fs"; + +import { AccountConfigError } from "../../src/common/account/AccountModel"; +import LokiAccountModelStore from "../../src/common/account/LokiAccountModelStore"; + +describe("LokiAccountModelStore @loki", () => { + const dbPath = "__test_db_account_unit__.json"; + + async function openStore( + accounts: { name: string; isVersioningEnabled: boolean }[] = [], + inMemory: boolean = false + ) { + const store = new LokiAccountModelStore( + dbPath, + inMemory, + accounts.map((a) => ({ + name: a.name, + blobService: { isVersioningEnabled: a.isVersioningEnabled } + })) + ); + await store.init(); + return store; + } + + afterEach(async () => { + if (existsSync(dbPath)) { + const store = new LokiAccountModelStore(dbPath, false); + await store.init(); + await store.close(); + await store.clean(); + } + }); + + it("defaults to versioning disabled for an unconfigured account", async () => { + const store = await openStore(); + assert.strictEqual( + store.getBlobServiceConfig("devstoreaccount1").isVersioningEnabled, + false + ); + await store.close(); + }); + + it("applies the configuration supplied at start up", async () => { + const store = await openStore([ + { name: "acct1", isVersioningEnabled: true }, + { name: "acct2", isVersioningEnabled: false } + ]); + assert.strictEqual(store.getBlobServiceConfig("acct1").isVersioningEnabled, true); + assert.strictEqual(store.getBlobServiceConfig("acct2").isVersioningEnabled, false); + // Unlisted accounts keep the defaults, so configuration only ever opts accounts in + assert.strictEqual(store.getBlobServiceConfig("acct3").isVersioningEnabled, false); + await store.close(); + }); + + it("matches account names case insensitively", async () => { + const store = await openStore([{ name: "acct1", isVersioningEnabled: true }]); + assert.strictEqual(store.getBlobServiceConfig("ACCT1").isVersioningEnabled, true); + await store.close(); + }); + + it("persists configuration across runs when none is supplied", async () => { + let store = await openStore([{ name: "acct1", isVersioningEnabled: true }]); + await store.close(); + + // A later run without any account options keeps the previous configuration, the way + // the ARM setting persists on a real account until it is changed. + store = await openStore(); + assert.strictEqual(store.getBlobServiceConfig("acct1").isVersioningEnabled, true); + await store.close(); + }); + + it("allows a persisted setting to be changed", async () => { + let store = await openStore([{ name: "acct1", isVersioningEnabled: true }]); + await store.close(); + + store = await openStore([{ name: "acct1", isVersioningEnabled: false }]); + assert.strictEqual(store.getBlobServiceConfig("acct1").isVersioningEnabled, false); + await store.close(); + + // ...and the change is itself persisted + store = await openStore(); + assert.strictEqual(store.getBlobServiceConfig("acct1").isVersioningEnabled, false); + await store.close(); + }); + + it("merges a new account into persisted configuration", async () => { + let store = await openStore([{ name: "acct1", isVersioningEnabled: true }]); + await store.close(); + + store = await openStore([{ name: "acct2", isVersioningEnabled: true }]); + assert.strictEqual(store.getBlobServiceConfig("acct1").isVersioningEnabled, true); + assert.strictEqual(store.getBlobServiceConfig("acct2").isVersioningEnabled, true); + assert.strictEqual(store.listConfigs().length, 2); + await store.close(); + }); + + it("reports the configuration in effect", async () => { + const store = await openStore([{ name: "acct1", isVersioningEnabled: true }]); + assert.deepStrictEqual(store.listConfigs(), [ + { name: "acct1", blobService: { isVersioningEnabled: true } } + ]); + await store.close(); + }); + + it("tracks initialized and closed state", async () => { + const store = new LokiAccountModelStore(dbPath, false); + assert.strictEqual(store.isInitialized(), false); + assert.strictEqual(store.isClosed(), true); + await store.init(); + assert.strictEqual(store.isInitialized(), true); + assert.strictEqual(store.isClosed(), false); + await store.close(); + assert.strictEqual(store.isClosed(), true); + }); + + it("refuses to clean while open", async () => { + const store = await openStore(); + await assert.rejects(() => store.clean(), /not closed/); + await store.close(); + }); + + it("writes no database file in memory mode", async () => { + const memoryPath = "__test_db_account_memory__.json"; + const store = new LokiAccountModelStore(memoryPath, true, [ + { name: "acct1", blobService: { isVersioningEnabled: true } } + ]); + await store.init(); + assert.strictEqual(store.getBlobServiceConfig("acct1").isVersioningEnabled, true); + assert.strictEqual( + existsSync(memoryPath), + false, + "In memory persistence must not write a database file" + ); + await store.close(); + }); + + it("exposes AccountConfigError for callers to catch", () => { + // The conflict path has no triggering setting today, so assert the contract the + // callers depend on rather than a specific conflict. + assert.ok(new AccountConfigError("x") instanceof Error); + }); +}); From cfcffaf7b201b92a5aaf4b0b0ee239f634441c0e Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 14:06:46 +0100 Subject: [PATCH 08/10] Add delimiter and hierarchy coverage for versioned listings The versioning tests only used listBlobsFlat, so the delimiter path had no versioned coverage at all - despite PageWithDelimiter being the class changed for the pagination fix. It both squashes names into BlobPrefix entries and carries the continuation token, so a page can end part way through the versions of a blob that is itself inside a squashed prefix. That combination was untested. Adds tests/blob/apis/blob.versioning.hierarchy.test.ts with eight cases: - hierarchical listing with include=versions emits each prefix exactly once, however many versions live underneath it - the same listing without versions is unchanged - listing inside a prefix returns every version of every blob under it - paginated hierarchical listing emits no prefix twice and drops no version - prefixes and blobs interleaved lexically, paged at size 1, which is the tightest case: every page holds a single prefix or a single blob version - a blob whose name is also another blob's prefix ("p" as a blob and "p/" as a prefix) is listed separately from the prefix, keeping all its versions - flat listing of the same interleaved tree at page size 1 Each case was run against a real storage account before being run against Azurite, so the expectations are the service's behaviour rather than a restatement of this implementation. Real Azure passes all eight and so does Azurite: no divergence found. That is a weaker result than the earlier live run, which found four divergences, and it contradicts the expectation that the delimiter path would be where the composite continuation token broke. It appears to hold because the marker records the underlying blob's [name, versionId] key even when that blob was squashed into a prefix, so resumption follows the same total ordering either way. The gap was still worth closing: it sat directly on top of modified code and would otherwise have remained an untested claim. Co-Authored-By: Claude --- docs/designs/blob-versioning.md | 6 + .../apis/blob.versioning.hierarchy.test.ts | 314 ++++++++++++++++++ 2 files changed, 320 insertions(+) create mode 100644 tests/blob/apis/blob.versioning.hierarchy.test.ts diff --git a/docs/designs/blob-versioning.md b/docs/designs/blob-versioning.md index 37b7084bc..822346c65 100644 --- a/docs/designs/blob-versioning.md +++ b/docs/designs/blob-versioning.md @@ -169,6 +169,12 @@ npx mocha --require ts-node/register --no-timeouts --grep @loki --exit tests/blo The `BlobVersioningDisabledAPIs` block is skipped in live mode, because it asserts that versioning is off. +The delimiter path is covered too, in `blob.versioning.hierarchy.test.ts`: hierarchical +listing with `include=versions`, listing inside a prefix, a blob whose name is also another +blob's prefix, interleaved prefixes and blobs, and pagination at page size 1. Those cases +exist because `PageWithDelimiter` both squashes prefixes and carries the continuation +token, so a page can end part way through the versions of a blob inside a squashed prefix. + Running this found four places where the implementation had followed the prose documentation but the service behaves differently: Set Blob Properties does not create a version, deleting the current version by ID is refused with 403 diff --git a/tests/blob/apis/blob.versioning.hierarchy.test.ts b/tests/blob/apis/blob.versioning.hierarchy.test.ts new file mode 100644 index 000000000..6dc995baa --- /dev/null +++ b/tests/blob/apis/blob.versioning.hierarchy.test.ts @@ -0,0 +1,314 @@ +import { + BlobServiceClient, + newPipeline, + StorageSharedKeyCredential +} from "@azure/storage-blob"; +import * as assert from "assert"; + +import { IAccountModel } from "../../../src/common/account/AccountModel"; +import { configLogger } from "../../../src/common/Logger"; +import BlobTestServerFactory from "../../BlobTestServerFactory"; +import { + EMULATOR_ACCOUNT_KEY, + EMULATOR_ACCOUNT_NAME, + getTestServerBaseURL, + getUniqueName +} from "../../testutils"; + +// Set true to enable debug log +configLogger(false); + +const VERSIONING_ENABLED_ACCOUNT_MODEL: IAccountModel = { + accounts: [ + { name: EMULATOR_ACCOUNT_NAME, blobService: { isVersioningEnabled: true } } + ] +}; + +/** + * List Blobs with a delimiter squashes names into BlobPrefix entries, which is handled by + * PageWithDelimiter - the same class that carries the continuation token. Versions make + * that harder: every version of a blob shares its name, so a page can end part way through + * the versions of a blob that is itself inside a squashed prefix. + */ +describe("BlobVersioningHierarchy", () => { + const factory = new BlobTestServerFactory(); + const server = factory.createServer( + false, + false, + false, + undefined, + VERSIONING_ENABLED_ACCOUNT_MODEL + ); + + const serviceClient = new BlobServiceClient( + getTestServerBaseURL(server), + newPipeline( + new StorageSharedKeyCredential( + EMULATOR_ACCOUNT_NAME, + EMULATOR_ACCOUNT_KEY + ), + { retryOptions: { maxTries: 1 }, keepAliveOptions: { enable: false } } + ) + ); + + let containerName: string = getUniqueName("container"); + let containerClient = serviceClient.getContainerClient(containerName); + + before(async () => { + await server.start(); + }); + + after(async () => { + await server.close(); + await server.clean(); + }); + + beforeEach(async () => { + containerName = getUniqueName("container"); + containerClient = serviceClient.getContainerClient(containerName); + await containerClient.create(); + }); + + afterEach(async () => { + await containerClient.delete(); + }); + + /** + * Seed a fixed tree: two prefixes with two blobs each, plus a root level blob, with + * several versions apiece. + */ + async function seed() { + const versions: { [name: string]: string[] } = {}; + const write = async (name: string, times: number) => { + versions[name] = []; + for (let i = 0; i < times; i++) { + const res = await containerClient + .getBlockBlobClient(name) + .upload(`v${i}`, 2); + versions[name].push(res.versionId!); + } + }; + await write("p1/a", 2); + await write("p1/b", 2); + await write("p2/c", 2); + await write("root", 3); + return versions; + } + + /** + * Page through a hierarchical listing, following continuation tokens, collecting the + * blob items and prefixes each page returned. + */ + async function pageHierarchy( + pageSize: number | undefined, + options: { includeVersions?: boolean; prefix?: string } = {} + ) { + const blobs: string[] = []; + const prefixes: string[] = []; + let continuationToken: string | undefined; + let pages = 0; + + do { + const result = await containerClient + .listBlobsByHierarchy("/", { + includeVersions: options.includeVersions, + prefix: options.prefix + }) + .byPage({ maxPageSize: pageSize, continuationToken }) + .next(); + + if (result.done) break; + + const segment = result.value.segment; + for (const item of segment.blobItems ?? []) { + blobs.push(`${item.name}@${item.versionId ?? "-"}`); + } + for (const p of segment.blobPrefixes ?? []) { + prefixes.push(p.name); + } + continuationToken = result.value.continuationToken; + pages++; + assert.ok(pages < 50, "Listing did not terminate"); + } while (continuationToken); + + return { blobs, prefixes, pages }; + } + + it("Hierarchical listing with versions squashes prefixes once @loki", async () => { + const versions = await seed(); + + const { blobs, prefixes } = await pageHierarchy(undefined, { + includeVersions: true + }); + + // Each prefix appears exactly once, however many versions live underneath it + assert.deepStrictEqual(prefixes, ["p1/", "p2/"]); + + // Only the root level blob is returned as a blob item, with all its versions + assert.deepStrictEqual( + blobs, + versions["root"].map((v) => `root@${v}`) + ); + }); + + it("Hierarchical listing without versions is unaffected @loki", async () => { + await seed(); + + const { blobs, prefixes } = await pageHierarchy(undefined, { + includeVersions: false + }); + + assert.deepStrictEqual(prefixes, ["p1/", "p2/"]); + assert.strictEqual(blobs.length, 1, "Only the current version of root"); + assert.ok(blobs[0].startsWith("root@")); + }); + + it("Listing inside a prefix returns every version @loki", async () => { + const versions = await seed(); + + const { blobs, prefixes } = await pageHierarchy(undefined, { + includeVersions: true, + prefix: "p1/" + }); + + assert.deepStrictEqual(prefixes, [], "No nested prefixes under p1/"); + assert.deepStrictEqual(blobs, [ + ...versions["p1/a"].map((v) => `p1/a@${v}`), + ...versions["p1/b"].map((v) => `p1/b@${v}`) + ]); + }); + + it("Paginated hierarchical listing returns each prefix once and every version @loki", async () => { + const versions = await seed(); + + // A page size below the number of entries forces continuation across a squashed + // prefix and part way through one blob's versions. + const { blobs, prefixes, pages } = await pageHierarchy(2, { + includeVersions: true + }); + + assert.ok(pages > 1, `Expected more than one page, got ${pages}`); + + // No prefix is emitted twice across pages + assert.deepStrictEqual( + prefixes, + ["p1/", "p2/"], + `Prefixes were duplicated or dropped across pages: ${JSON.stringify(prefixes)}` + ); + + // Every version of the root level blob is returned exactly once + assert.deepStrictEqual( + blobs, + versions["root"].map((v) => `root@${v}`), + `Blob items were duplicated or dropped across pages: ${JSON.stringify(blobs)}` + ); + }); + + /** + * A harder tree than seed(): prefixes and blobs interleave lexically, and one blob has + * the same name as another blob's prefix. Sorted order is + * a/1, b, c/1, p, p/x + * so a hierarchical listing has to alternate between squashing prefixes and emitting + * blobs, and "p" is a blob while "p/" is also a prefix. + */ + async function seedInterleaved() { + const versions: { [name: string]: string[] } = {}; + const write = async (name: string, times: number) => { + versions[name] = []; + for (let i = 0; i < times; i++) { + const res = await containerClient + .getBlockBlobClient(name) + .upload(`v${i}`, 2); + versions[name].push(res.versionId!); + } + }; + await write("a/1", 2); + await write("b", 3); + await write("c/1", 2); + await write("p", 2); + await write("p/x", 2); + return versions; + } + + it("Interleaved prefixes and blobs page correctly at size 1 @loki", async () => { + const versions = await seedInterleaved(); + + // Page size 1 is the tightest case: every page holds a single prefix or a single + // blob version, so the continuation token is exercised on every boundary. + const { blobs, prefixes, pages } = await pageHierarchy(1, { + includeVersions: true + }); + + assert.ok(pages > 4, `Expected many pages at size 1, got ${pages}`); + assert.deepStrictEqual( + prefixes, + ["a/", "c/", "p/"], + `Prefixes duplicated or dropped: ${JSON.stringify(prefixes)}` + ); + assert.deepStrictEqual( + blobs, + [ + ...versions["b"].map((v) => `b@${v}`), + ...versions["p"].map((v) => `p@${v}`) + ], + `Blob items duplicated or dropped: ${JSON.stringify(blobs)}` + ); + }); + + it("A blob sharing a name with a prefix is listed separately @loki", async () => { + const versions = await seedInterleaved(); + + // "p" is a blob and "p/" is a prefix; both must appear, and the blob must keep all + // of its versions. + const { blobs, prefixes } = await pageHierarchy(undefined, { + includeVersions: true + }); + + assert.ok(prefixes.includes("p/")); + assert.deepStrictEqual( + blobs.filter((b) => b.startsWith("p@")), + versions["p"].map((v) => `p@${v}`) + ); + }); + + it("Flat listing of the interleaved tree returns every version at size 1 @loki", async () => { + const versions = await seedInterleaved(); + + const seen: string[] = []; + let continuationToken: string | undefined; + let pages = 0; + do { + const result = await containerClient + .listBlobsFlat({ includeVersions: true }) + .byPage({ maxPageSize: 1, continuationToken }) + .next(); + if (result.done) break; + for (const item of result.value.segment.blobItems) { + seen.push(`${item.name}@${item.versionId}`); + } + continuationToken = result.value.continuationToken; + pages++; + assert.ok(pages < 60, "Listing did not terminate"); + } while (continuationToken); + + const expected = ["a/1", "b", "c/1", "p", "p/x"].flatMap((name) => + versions[name].map((v) => `${name}@${v}`) + ); + assert.deepStrictEqual(seen, expected); + }); + + it("Paginated listing inside a prefix returns every version @loki", async () => { + const versions = await seed(); + + const { blobs, pages } = await pageHierarchy(2, { + includeVersions: true, + prefix: "p1/" + }); + + assert.ok(pages > 1, `Expected more than one page, got ${pages}`); + assert.deepStrictEqual(blobs, [ + ...versions["p1/a"].map((v) => `p1/a@${v}`), + ...versions["p1/b"].map((v) => `p1/b@${v}`) + ]); + }); +}); From 20fd75e85cc90e7b1b861a4c016c11cd6cb77eee Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 15:55:24 +0100 Subject: [PATCH 09/10] Address review concerns: index versionId, honour loose mode, tidy helpers Five things a reviewer would reasonably object to in the versioning work, all found by reading the branch back as an adversarial reviewer. versionId was a query dimension but not an index. Reads, deletes, tag and tier operations all look blobs up by it, and generateVersionId probed for collisions, so every one of those was an unindexed scan. It is now part of the blobs collection index, and ensureIndex is called for a workspace created before versioning existed so lookups stay indexed after an upgrade in place. generateVersionId ran an unbounded loop, querying the database once per candidate millisecond until it found a free value. Because version IDs share a fixed width format, comparing them as strings orders them chronologically, so one query for the newest existing version is enough: take the clock value if it is already past it, otherwise step one millisecond beyond. Single query, bounded work, same monotonic guarantee. Loose mode was not honoured. Loose mode exists so Azurite ignores parameters it would otherwise reject, and before versioning existed versionid was ignored entirely, so rejecting a malformed one was loose mode becoming stricter than it used to be. Simply swallowing the error was not enough - the request then looked up a version that does not exist and returned 404 instead of 400. Validation and sanitisation therefore live together in BaseHandler.resolveVersionId(), which returns the version ID to serve the request with, or undefined in loose mode after logging. Callers consume the return value rather than options.versionId, so a caller cannot skip the error and then act on the bad input. While converting those call sites, Set Blob Tier turned out to accept versionid with no validation at all, so a malformed value reached the metadata store directly. It is now validated like the rest. The empty IRRECONCILABLE_BLOB_SERVICE_SETTINGS array and its filter are gone. A conflict detection framework with nothing to detect is speculative; a comment now records what a future setting needing a migration rather than a merge would have to do, and the store interface no longer documents a throw that cannot happen. demoteToPreviousVersion, createNewCurrentVersion and isVersionDoc took and returned any. They now use BlobModel, with the cast narrowed to the two LokiJS internals ($loki and meta) that are genuinely not part of the model, matching the idiom already used in that file. Adds a BlobVersioningLooseMode suite covering both the malformed version ID and the snapshot/versionid combination. It runs as its own top level suite because the test fixture binds a fixed port, so only one server can run at a time. Verified: 920 tests pass against Azurite, 33 against a real storage account, and the upgrade compatibility suite passes. Co-Authored-By: Claude --- src/blob/handlers/BaseHandler.ts | 42 ++++++++++ src/blob/handlers/BlobHandler.ts | 47 +++++++---- src/blob/persistence/LokiBlobMetadataStore.ts | 79 +++++++++++++------ src/common/account/IAccountModelStore.ts | 5 +- src/common/account/LokiAccountModelStore.ts | 36 ++------- tests/blob/apis/blob.versioning.test.ts | 69 ++++++++++++++++ 6 files changed, 206 insertions(+), 72 deletions(-) diff --git a/src/blob/handlers/BaseHandler.ts b/src/blob/handlers/BaseHandler.ts index 63e6106c6..d4659c8b2 100644 --- a/src/blob/handlers/BaseHandler.ts +++ b/src/blob/handlers/BaseHandler.ts @@ -1,6 +1,7 @@ import IExtentStore from "../../common/persistence/IExtentStore"; import ILogger from "../generated/utils/ILogger"; import IBlobMetadataStore from "../persistence/IBlobMetadataStore"; +import { validateSnapshotAndVersionId } from "../utils/utils"; /** * BaseHandler class should maintain a singleton to persistency layer, such as maintain a database connection pool. @@ -17,4 +18,45 @@ export default class BaseHandler { protected readonly logger: ILogger, protected readonly loose: boolean ) {} + + /** + * Validate the `snapshot` and `versionId` query parameters of a blob request, and return + * the version ID the request should be served with. + * + * A request may address a snapshot or a version, but not both, and a version ID must be + * an RFC 3339 timestamp with 7 digit fractional seconds. Azure Storage rejects either + * mistake with 400, which is what strict mode does. + * + * Loose mode ignores parameters Azurite would otherwise reject, so the offending version + * ID is dropped and the request is served against the current version - the behaviour + * loose mode had before versioning existed, when `versionid` was ignored entirely. + * Returning the value rather than only validating it keeps those two outcomes in one + * place, so a caller cannot skip the error and then act on the bad input. + * + * @protected + * @param {string} [snapshot] + * @param {string} [versionId] + * @param {string} [contextId] + * @returns {(string | undefined)} + * @memberof BaseHandler + */ + protected resolveVersionId( + snapshot?: string, + versionId?: string, + contextId?: string + ): string | undefined { + try { + validateSnapshotAndVersionId(snapshot, versionId, contextId); + return versionId; + } catch (err) { + if (this.loose) { + this.logger.warn( + `BaseHandler:resolveVersionId() Ignoring versionid in loose mode: snapshot=${snapshot}, versionid=${versionId}`, + contextId + ); + return undefined; + } + throw err; + } + } } diff --git a/src/blob/handlers/BlobHandler.ts b/src/blob/handlers/BlobHandler.ts index fac8a6e1e..3ecf86dff 100644 --- a/src/blob/handlers/BlobHandler.ts +++ b/src/blob/handlers/BlobHandler.ts @@ -29,8 +29,7 @@ import { deserializePageBlobRangeHeader, deserializeRangeHeader, getBlobTagsCount, - validateBlobTag, - validateSnapshotAndVersionId + validateBlobTag } from "../utils/utils"; import BaseHandler from "./BaseHandler"; import IPageBlobRangesManager from "./IPageBlobRangesManager"; @@ -71,7 +70,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const containerName = blobCtx.container!; const blobName = blobCtx.blob!; - validateSnapshotAndVersionId( + const versionId = this.resolveVersionId( options.snapshot, options.versionId, context.contextId @@ -85,7 +84,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options.snapshot, options.leaseAccessConditions, options.modifiedAccessConditions, - options.versionId + versionId ); if (blob.properties.accessTier === Models.AccessTier.Archive) { @@ -119,7 +118,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const account = blobCtx.account!; const container = blobCtx.container!; const blob = blobCtx.blob!; - validateSnapshotAndVersionId( + const versionId = this.resolveVersionId( options.snapshot, options.versionId, context.contextId @@ -133,7 +132,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options.snapshot, options.leaseAccessConditions, options.modifiedAccessConditions, - options.versionId + versionId ); // TODO: Create get metadata specific request in swagger @@ -200,7 +199,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const container = blobCtx.container!; const blob = blobCtx.blob!; - validateSnapshotAndVersionId( + options.versionId = this.resolveVersionId( options.snapshot, options.versionId, context.contextId @@ -684,7 +683,11 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { // A previous version can be used as a copy source, which is how a blob is restored // from one of its versions. const sourceVersionId = url.searchParams.get("versionid") || undefined; - validateSnapshotAndVersionId(snapshot, sourceVersionId, context.contextId); + const resolvedSourceVersionId = this.resolveVersionId( + snapshot, + sourceVersionId, + context.contextId + ); if ( sourceAccount === undefined || @@ -711,7 +714,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { container: sourceContainer, blob: sourceBlob, snapshot, - versionId: sourceVersionId + versionId: resolvedSourceVersionId }, { account, container, blob }, copySource, @@ -890,7 +893,11 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { // A previous version can be used as a copy source, which is how a blob is restored // from one of its versions. const sourceVersionId = url.searchParams.get("versionid") || undefined; - validateSnapshotAndVersionId(snapshot, sourceVersionId, context.contextId); + const resolvedSourceVersionId = this.resolveVersionId( + snapshot, + sourceVersionId, + context.contextId + ); if ( sourceAccount === undefined || @@ -921,7 +928,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { container: sourceContainer, blob: sourceBlob, snapshot, - versionId: sourceVersionId + versionId: resolvedSourceVersionId }, { account, container, blob }, copySource, @@ -980,6 +987,12 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const account = blobCtx.account!; const container = blobCtx.container!; const blob = blobCtx.blob!; + const versionId = this.resolveVersionId( + undefined, + options.versionId, + context.contextId + ); + const res = await this.metadataStore.setTier( context, account, @@ -987,7 +1000,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { blob, tier, options.leaseAccessConditions, - options.versionId + versionId ); const response: Models.BlobSetTierResponse = { @@ -1327,7 +1340,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { const account = blobCtx.account!; const container = blobCtx.container!; const blob = blobCtx.blob!; - validateSnapshotAndVersionId( + const versionId = this.resolveVersionId( options.snapshot, options.versionId, context.contextId @@ -1341,7 +1354,7 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { options.snapshot, options.leaseAccessConditions, options.modifiedAccessConditions, - options.versionId + versionId ); const response: Models.BlobGetTagsResponse = { @@ -1372,7 +1385,11 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler { // Get snapshot (swagger not defined snapshot as parameter, but server support set tag on blob snapshot) let snapshot = context.request!.getQuery("snapshot"); - validateSnapshotAndVersionId(snapshot, options.versionId, context.contextId); + options.versionId = this.resolveVersionId( + snapshot, + options.versionId, + context.contextId + ); await this.metadataStore.setBlobTag( context, diff --git a/src/blob/persistence/LokiBlobMetadataStore.ts b/src/blob/persistence/LokiBlobMetadataStore.ts index f864af389..4b9bf96fa 100644 --- a/src/blob/persistence/LokiBlobMetadataStore.ts +++ b/src/blob/persistence/LokiBlobMetadataStore.ts @@ -192,8 +192,20 @@ export default class LokiBlobMetadataStore // Create containers collection if not exists if (this.db.getCollection(this.BLOBS_COLLECTION) === null) { this.db.addCollection(this.BLOBS_COLLECTION, { - indices: ["accountName", "containerName", "name", "snapshot"] // Optimize for find operation + // versionId is indexed alongside snapshot because a version is addressed by it: + // reads, deletes, tag and tier operations all query on it. + indices: [ + "accountName", + "containerName", + "name", + "snapshot", + "versionId" + ] // Optimize for find operation }); + } else { + // A workspace created before versioning existed has no versionId index. Adding it + // here keeps version lookups indexed after an upgrade in place. + this.db.getCollection(this.BLOBS_COLLECTION).ensureIndex("versionId"); } // Create blocks collection if not exists @@ -254,24 +266,35 @@ export default class LokiBlobMetadataStore blob: string ): string { const coll = this.db.getCollection(this.BLOBS_COLLECTION); - let candidateTime = context.startTime!.getTime(); - // eslint-disable-next-line no-constant-condition - while (true) { - const candidate = convertDateTimeStringMsTo7Digital( - new Date(candidateTime).toISOString() - ); - const clash = coll.findOne({ - accountName: account, - containerName: container, - name: blob, - versionId: candidate - }); - if (clash === null || clash === undefined) { - return candidate; + // Version IDs share a fixed width format, so comparing them as strings orders them + // chronologically. One query for the newest existing version is therefore enough to + // guarantee the next ID is unique and increasing, without probing candidate values. + let newest = ""; + for (const doc of coll.find({ + accountName: account, + containerName: container, + name: blob + }) as any[]) { + if (doc.versionId !== undefined && doc.versionId > newest) { + newest = doc.versionId; } - candidateTime += 1; } + + const candidate = convertDateTimeStringMsTo7Digital( + context.startTime!.toISOString() + ); + + if (candidate > newest) { + return candidate; + } + + // The clock has not advanced past the newest version, which happens when several + // writes land in the same millisecond. Step one millisecond past it instead. + const newestMs = new Date(newest.replace(/(\.\d{3})\d{4}Z$/, "$1Z")).getTime(); + return convertDateTimeStringMsTo7Digital( + new Date(newestMs + 1).toISOString() + ); } /** @@ -293,14 +316,15 @@ export default class LokiBlobMetadataStore * @memberof LokiBlobMetadataStore */ private createNewCurrentVersion( - coll: Collection, - doc: any, + coll: Collection, + doc: BlobModel, context: Context - ): any { + ): BlobModel { // Copy before demoting, so the copy still carries the lease and the old state - const copy: any = { ...doc }; - delete copy.$loki; - delete copy.meta; + const copy: BlobModel = { ...doc }; + // $loki and meta are LokiJS internals, not part of the blob model + delete (copy as any).$loki; + delete (copy as any).meta; copy.properties = { ...doc.properties }; if (doc.metadata !== undefined) { copy.metadata = { ...doc.metadata }; @@ -325,7 +349,7 @@ export default class LokiBlobMetadataStore ); copy.isCurrentVersion = true; - return coll.insert(copy); + return coll.insert(copy) as BlobModel; } /** @@ -343,7 +367,7 @@ export default class LokiBlobMetadataStore * @returns {boolean} * @memberof LokiBlobMetadataStore */ - private isVersionDoc(doc: any): boolean { + private isVersionDoc(doc: BlobModel | null | undefined): boolean { return doc !== null && doc !== undefined && doc.versionId !== undefined; } @@ -1328,9 +1352,12 @@ export default class LokiBlobMetadataStore * @param {*} doc * @memberof LokiBlobMetadataStore */ - private demoteToPreviousVersion(coll: Collection, doc: any): void { + private demoteToPreviousVersion( + coll: Collection, + doc: BlobModel + ): void { if (doc.versionId === undefined) { - const lastModified: Date | undefined = doc.properties?.lastModified; + const lastModified: Date | undefined = doc.properties.lastModified; doc.versionId = convertDateTimeStringMsTo7Digital( (lastModified !== undefined ? new Date(lastModified) diff --git a/src/common/account/IAccountModelStore.ts b/src/common/account/IAccountModelStore.ts index 982fafa8c..7c8fe3cf6 100644 --- a/src/common/account/IAccountModelStore.ts +++ b/src/common/account/IAccountModelStore.ts @@ -23,8 +23,9 @@ export default interface IAccountModelStore extends IDataStore, ICleaner { * Reconcile the configuration supplied at start up with the configuration persisted by * the previous run, and persist the result. * - * Called once during init(). Throws AccountConfigError when a setting that cannot be - * changed once the workspace holds data has a different value. + * Called once during init(). Every setting can currently be changed against an existing + * workspace, so this does not reject; a future setting needing a migration rather than a + * merge would be compared here. * * @param {IAccountConfig[]} incoming Configuration supplied at start up, may be empty * @returns {Promise} diff --git a/src/common/account/LokiAccountModelStore.ts b/src/common/account/LokiAccountModelStore.ts index f5e3d6b81..8d762e21b 100644 --- a/src/common/account/LokiAccountModelStore.ts +++ b/src/common/account/LokiAccountModelStore.ts @@ -4,26 +4,12 @@ import Loki from "lokijs"; import ILogger from "../ILogger"; import { rimrafAsync } from "../utils/utils"; import { - AccountConfigError, DEFAULT_ACCOUNT_BLOB_SERVICE_CONFIG, IAccountBlobServiceConfig, IAccountConfig } from "./AccountModel"; import IAccountModelStore from "./IAccountModelStore"; -/** - * Account level blob service settings that cannot be changed once a workspace holds data, - * because doing so would leave the metadata in a state matching neither value. - * - * Empty today. Blob versioning is safe to toggle: verified against the real service, - * turning it off keeps existing versions listed, readable and deletable by version ID, and - * turning it on captures a pre-existing blob's state as a version when it is next modified. - * - * Any future setting that would need a migration rather than a merge belongs here. - */ -const IRRECONCILABLE_BLOB_SERVICE_SETTINGS: (keyof IAccountBlobServiceConfig)[] = - []; - /** * Loki backed implementation of IAccountModelStore, with its own database file so that the * blob, queue and table services can share one account configuration. @@ -147,24 +133,16 @@ export default class LokiAccountModelStore implements IAccountModelStore { }); } + // Every setting can currently be changed against an existing workspace. Blob + // versioning is safe to toggle: verified against the real service, turning it off + // keeps existing versions listed, readable and deletable by version ID, and turning it + // on captures a pre-existing blob's state as a version when it is next modified. + // + // A future setting that needs a migration rather than a merge would have to be + // compared against `previous` here and rejected. for (const account of incoming) { const previous = resolved.get(account.name); - const conflicting = IRRECONCILABLE_BLOB_SERVICE_SETTINGS.filter( - (setting) => - previous !== undefined && - previous.blobService[setting] !== account.blobService[setting] - ); - - if (conflicting.length > 0) { - throw new AccountConfigError( - `Account "${account.name}" was previously started with different values for ` + - `${conflicting.join(", ")}, which cannot be changed once the workspace holds ` + - `data. Either keep the previous values, or start Azurite against a clean ` + - `workspace (a different --location, or remove the existing one).` - ); - } - resolved.set(account.name, account); if (previous === undefined) { diff --git a/tests/blob/apis/blob.versioning.test.ts b/tests/blob/apis/blob.versioning.test.ts index 691dc5ebb..a00aa85e3 100644 --- a/tests/blob/apis/blob.versioning.test.ts +++ b/tests/blob/apis/blob.versioning.test.ts @@ -772,3 +772,72 @@ describe("BlobVersioningAPIs", () => { assert.strictEqual(properties.isCurrentVersion, undefined); }); }); + +// Loose mode exists so Azurite ignores parameters it would otherwise reject. Before +// versioning, versionid was ignored entirely, so loose mode must not become stricter. Runs +// as its own suite because the test fixture binds a fixed port, so only one server at a +// time. Skipped in live mode, where there is no loose mode to configure. +(LIVE_TEST_MODE ? describe.skip : describe)("BlobVersioningLooseMode", () => { + const factory = new BlobTestServerFactory(); + const server = factory.createServer( + true, + false, + false, + undefined, + VERSIONING_ENABLED_ACCOUNT_MODEL, + "loose" + ); + + const serviceClient = new BlobServiceClient( + getTestServerBaseURL(server), + newPipeline( + new StorageSharedKeyCredential( + EMULATOR_ACCOUNT_NAME, + EMULATOR_ACCOUNT_KEY + ), + { retryOptions: { maxTries: 1 }, keepAliveOptions: { enable: false } } + ) + ); + + before(async () => { + await server.start(); + }); + + after(async () => { + await server.close(); + await server.clean(); + }); + + it("A malformed version ID is ignored rather than rejected @loki", async () => { + const containerClient = serviceClient.getContainerClient( + getUniqueName("container") + ); + await containerClient.create(); + const blobClient = containerClient.getBlockBlobClient(getUniqueName("blob")); + await blobClient.upload("version1", 8); + + // Strict mode returns 400 for this; loose mode logs and serves the current version + const downloaded = await blobClient.withVersion("notatimestamp").download(); + assert.strictEqual(await bodyToString(downloaded, 8), "version1"); + + await containerClient.delete(); + }); + + it("Snapshot and version ID together are ignored rather than rejected @loki", async () => { + const containerClient = serviceClient.getContainerClient( + getUniqueName("container") + ); + await containerClient.create(); + const blobClient = containerClient.getBlockBlobClient(getUniqueName("blob")); + const upload = await blobClient.upload("version1", 8); + const snapshot = await blobClient.createSnapshot(); + + const downloaded = await blobClient + .withSnapshot(snapshot.snapshot!) + .withVersion(upload.versionId!) + .download(); + assert.strictEqual(await bodyToString(downloaded, 8), "version1"); + + await containerClient.delete(); + }); +}); From 85b523c37cca8a11052be764075de75892614220 Mon Sep 17 00:00:00 2001 From: Gavin Thompson Date: Thu, 13 Aug 2026 16:30:31 +0100 Subject: [PATCH 10/10] Correct documentation that described superseded behaviour Review of the accumulated branch found five places where prose still described behaviour that later commits reversed. No functional change. The ChangeLog was the worst of it. As live testing corrected the implementation I appended new bullets rather than editing the superseded ones, so the Upcoming Release section contradicted itself: one bullet said the snapshot/versionid combination returns InvalidQueryParameterValue and that the versioning setting cannot be changed against existing data, another said Set Blob Properties creates a version and returns x-ms-version-id, and a third claimed HasVersionsOnly is reported - all three reversed by later bullets in the same list. A reader of the release notes had no way to tell which won. The blob section is now a single coherent description of the behaviour the branch actually ships, organised by area rather than by the order I discovered things, including an explicit list of what is not implemented. Seven accumulated bullets became one entry with sub-bullets. Also corrected: - The JSDoc on validateSnapshotAndVersionId named InvalidQueryParameterValue for the snapshot/versionid combination; the function throws MutuallyExclusiveQueryParameters, which is what the service returns. The JSDoc now names both codes and which case each applies to. - A comment in BlobTestServerFactory said blob versioning cannot be switched on or off against an existing workspace. That was true when the comment was written and is not any more, and the toggle tests deliberately share one workspace to prove it. The comment now explains what the per-suite database is actually for and how to share one on purpose. Verified: 920 tests still pass against Azurite, build and lint clean. Co-Authored-By: Claude --- ChangeLog.md | 11 ++++++----- src/blob/utils/utils.ts | 5 +++-- tests/BlobTestServerFactory.ts | 5 +++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 424e4574f..6e8b720fe 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -6,12 +6,13 @@ Blob: -- Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options rather than through a data plane API. With versioning enabled for an account: overwriting a blob retains the previous content as a version and returns `x-ms-version-id`; `Get Blob` / `Get Blob Properties` accept `?versionid=` and return `x-ms-is-current-version` for the current version; `List Blobs` supports `include=versions`; `Delete Blob` with `?versionid=` removes a single version, and deleting the current version retains previous versions instead of failing with `SnapshotsPresent`; a version can be restored by copying it over the current version. Combining `?snapshot=` with `?versionid=` returns 400 `InvalidQueryParameterValue`. The setting is persisted with the workspace and cannot be changed against existing data. Versioning is not implemented for the SQL metadata store, and blob version SAS (`sr=bv`, the `x` permission), soft delete interactions, and blob expiration interactions are out of scope - see `docs/designs/blob-versioning.md`. +- Added support for blob versioning on the LokiJS metadata store, opt in per storage account (issue #665). Versioning is an ARM management plane setting in Azure Storage, not part of the data plane REST API that Azurite emulates, so it is supplied at start up with the new mutually exclusive `--accountConfigFile ` and `--accountConfig ` options (and the matching `azurite.accountConfigFile` / `azurite.accountConfig` VS Code settings) rather than through a data plane API. Multiple accounts are supported, and an account that is not listed keeps the default of versioning disabled. See `docs/designs/blob-versioning.md`. + - Version creating operations: `Put Blob`, `Put Block List`, `Copy Blob`, `Set Blob Metadata` (all blob types), `Page Blob Create`, `Append Blob Create`, and `Snapshot Blob` create a version and return `x-ms-version-id`. `Put Page`, `Append Block` and `Set Blob Properties` do not create a version, matching the service. + - `Get Blob` and `Get Blob Properties` accept `?versionid=`, and return `x-ms-is-current-version` only for the current version. `List Blobs` supports `include=versions`. `Get Blob Tags`, `Set Blob Tags` and `Set Blob Tier` accept `?versionid=`, so tags and access tier are addressable per version. A version is restored by copying it over the current version. + - `Delete Blob` without a version ID turns the current version into a previous version and retains it, leaving the blob with no current version while previous versions persist. `Delete Blob` with `?versionid=` removes a single previous version; naming the current version returns 403 `OperationNotAllowedOnRootBlob`. Combining `?snapshot=` with `?versionid=` returns 400 `MutuallyExclusiveQueryParameters`, and a malformed `?versionid=` returns 400 `InvalidQueryParameterValue`. In loose mode an unusable `versionid` is ignored rather than rejected. + - Versioning can be turned on and off against an existing workspace, as on a real account. Turning it off keeps existing versions listed, readable and deletable by version ID, and a later write retains the previously current version while producing a blob that is not itself a version. + - Not implemented: the SQL metadata store (configuring versioning with `AZURITE_DB` fails at start up), blob version SAS (`sr=bv` and the `x` permission), soft delete interactions, blob expiration, and `HasVersionsOnly`, which the service reports only under `include=deletedwithversions`. - Moved account (management plane) configuration into a standalone `src/common/account/` module with its own database file (`__azurite_db_account__.json`), so that the queue and table services can share it rather than reading configuration owned by the blob service. -- Blob versioning can now be turned on and off against an existing workspace, matching the real service, rather than failing at start up. Turning it off keeps existing versions listed, readable and deletable by version ID, and a subsequent write retains the previously current version while producing a blob that is not itself a version. Turning it on over existing data captures a blob's prior state as a version when it is next modified. -- Aligned blob versioning with behaviour observed against a real storage account: `Set Blob Properties` does not create a version, deleting the current version by version ID returns 403 `OperationNotAllowedOnRootBlob`, combining `?snapshot=` with `?versionid=` returns 400 `MutuallyExclusiveQueryParameters`, and `HasVersionsOnly` is not reported under `include=versions`. -- Extended blob versioning to the remaining version creating operations: `Set Blob Metadata` (all blob types), `Set Blob Properties` (block blobs only), `Page Blob Create` and `Append Blob Create` now return `x-ms-version-id`, and `Snapshot Blob` on a versioned blob creates a new current version alongside the snapshot. `Put Page` and `Append Block` correctly do not create versions. `Get Blob Tags`, `Set Blob Tags` and `Set Blob Tier` accept `?versionid=`, and a malformed `?versionid=` returns 400 `InvalidQueryParameterValue`. -- `Delete Blob` on a versioned blob without a version ID now turns the current version into a previous version and retains it, rather than removing it, and reports `HasVersionsOnly` for a blob that has versions but no current version. - Blob list continuation tokens now carry a secondary key so that a `List Blobs` page can resume part way through one blob's versions. Tokens for listings that do not involve versions keep the previous plain blob name format, so existing tokens remain valid. General: diff --git a/src/blob/utils/utils.ts b/src/blob/utils/utils.ts index 0400d6873..f06a7369b 100644 --- a/src/blob/utils/utils.ts +++ b/src/blob/utils/utils.ts @@ -398,8 +398,9 @@ export function toBlobTags(input: TagContent[]): BlobTag[] { /** * Validate the `snapshot` and `versionId` query parameters of a blob request. * - * A request may address a snapshot or a version, but not both. Azure Storage rejects - * the combination with 400 InvalidQueryParameterValue. + * A request may address a snapshot or a version, but not both. Verified against the real + * service: the combination is rejected with 400 MutuallyExclusiveQueryParameters, while a + * malformed version ID is rejected with 400 InvalidQueryParameterValue. * * @export * @param {string} [snapshot] diff --git a/tests/BlobTestServerFactory.ts b/tests/BlobTestServerFactory.ts index e0d0a468e..c394c64b7 100644 --- a/tests/BlobTestServerFactory.ts +++ b/tests/BlobTestServerFactory.ts @@ -86,8 +86,9 @@ export default class BlobTestServerFactory { return new SqlBlobServer(config); } else { - // Blob versioning cannot be switched on or off against an existing workspace, so - // suites that configure it need their own metadata DB. + // Account configuration persists with the workspace, so a suite that configures it + // needs its own metadata DB to avoid inheriting another suite's settings. Pass an + // explicit `workspace` to share one DB deliberately, as the toggle tests do. const suffix = workspace !== undefined ? `_${workspace}`