Skip to content

Commit

Permalink
[Storage] Blob tags - resolve comments
Browse files Browse the repository at this point in the history
  • Loading branch information
XiaoningLiu committed Jun 12, 2020
1 parent d5b5d2a commit fc86b0f
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 51 deletions.
5 changes: 1 addition & 4 deletions sdk/storage/storage-blob/review/storage-blob.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2706,10 +2706,7 @@ export class StorageSharedKeyCredentialPolicy extends CredentialPolicy {
export type SyncCopyStatusType = 'success';

// @public
export interface Tags {
// (undocumented)
[propertyName: string]: string;
}
export type Tags = Record<string, string>;

// @public
export interface UserDelegationKey {
Expand Down
16 changes: 10 additions & 6 deletions sdk/storage/storage-blob/src/BlobSASSignatureValues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,18 +475,22 @@ function generateBlobSASQueryParameters20181109(
);
}

if (blobSASSignatureValues.versionId) {
const version = blobSASSignatureValues.version ? blobSASSignatureValues.version : SERVICE_VERSION;
let resource: string = "c";
let verifiedPermissions: string | undefined;

if (blobSASSignatureValues.versionId && version < "2019-10-10") {
throw RangeError("'version' must be >= '2019-10-10' when provided 'versionId'.");
}

if (blobSASSignatureValues.permissions && blobSASSignatureValues.permissions.tag) {
if (
blobSASSignatureValues.permissions &&
blobSASSignatureValues.permissions.tag &&
version < "2019-12-12"
) {
throw RangeError("'version' must be >= '2019-12-12' when provided 't' permission.");
}

const version = blobSASSignatureValues.version ? blobSASSignatureValues.version : SERVICE_VERSION;
let resource: string = "c";
let verifiedPermissions: string | undefined;

if (blobSASSignatureValues.blobName === undefined && blobSASSignatureValues.snapshotTime) {
throw RangeError("Must provide 'blobName' when provided 'snapshotTime'.");
}
Expand Down
2 changes: 2 additions & 0 deletions sdk/storage/storage-blob/src/BlobServiceClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,8 @@ export class BlobServiceClient extends StorageClient {
*
* .byPage() returns an async iterable iterator to list the blobs in pages.
*
* @see https://docs.microsoft.com/en-us/rest/api/storageservices/get-blob-service-properties
*
* Example using `for await` syntax:
*
* ```js
Expand Down
7 changes: 1 addition & 6 deletions sdk/storage/storage-blob/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,8 @@ import { EncryptionAlgorithmAES25 } from "./utils/constants";

/**
* Blob tags.
*
* @export
* @interface Tags
*/
export interface Tags {
[propertyName: string]: string;
}
export type Tags = Record<string, string>;

/**
* A map of name-value pairs to associate with the resource.
Expand Down
21 changes: 12 additions & 9 deletions sdk/storage/storage-blob/test/blobclient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
getBSU,
getSASConnectionStringFromEnvironment,
recorderEnvSetup,
isBlobVersioningDisabled,
isBlobVersioningDisabled
} from "./utils";
import { record, delay } from "@azure/test-utils-recorder";
import {
Expand Down Expand Up @@ -52,7 +52,7 @@ describe("BlobClient", () => {
}
});

it.only("Set blob tags should work", async () => {
it("Set blob tags should work", async () => {
const tags = {
tag1: "val1",
tag2: "val2"
Expand All @@ -77,7 +77,7 @@ describe("BlobClient", () => {
assert.deepStrictEqual(segment.value.segment.blobItems[0].tags, tags);
});

it.only("Get blob tags should work with a snapshot", async () => {
it("Get blob tags should work with a snapshot", async () => {
const tags = {
tag1: "val1",
tag2: "val2"
Expand All @@ -91,7 +91,7 @@ describe("BlobClient", () => {
assert.deepStrictEqual(response.tags, tags);
});

it.only("Create block blob blob should work with tags", async () => {
it("Create block blob blob should work with tags", async () => {
await blockBlobClient.delete();

const tags = {
Expand All @@ -104,7 +104,7 @@ describe("BlobClient", () => {
assert.deepStrictEqual(response.tags, tags);
});

it.only("Create append blob should work with tags", async () => {
it("Create append blob should work with tags", async () => {
await blockBlobClient.delete();

const tags = {
Expand All @@ -119,7 +119,7 @@ describe("BlobClient", () => {
assert.deepStrictEqual(response.tags, tags);
});

it.only("Create page blob should work with tags", async () => {
it("Create page blob should work with tags", async () => {
await blockBlobClient.delete();

const tags = {
Expand Down Expand Up @@ -336,7 +336,7 @@ describe("BlobClient", () => {
const iter = containerClient
.listBlobsFlat({
includeDeleted: true,
includeVersions: true, // Need this when blob versioning is turned on.
includeVersions: true // Need this when blob versioning is turned on.
})
.byPage({ maxPageSize: 1 });

Expand Down Expand Up @@ -375,15 +375,18 @@ describe("BlobClient", () => {
);

if (isBlobVersioningDisabled()) {
assert.ok(result.segment.blobItems![0].deleted, "Expect that the blob is marked for deletion");
assert.ok(
result.segment.blobItems![0].deleted,
"Expect that the blob is marked for deletion"
);
}

await blobClient.undelete();

const iter2 = containerClient
.listBlobsFlat({
includeDeleted: true,
includeVersions: true, // Need this when blob versioning is turned on.
includeVersions: true // Need this when blob versioning is turned on.
})
.byPage();

Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/storage-blob/test/blobserviceclient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ describe("BlobServiceClient", () => {
assert.notDeepStrictEqual(response.signedExpiresOn, undefined);
});

it.only("Find blob by tags should work", async () => {
it("Find blob by tags should work", async () => {
const blobServiceClient = getBSU();

const containerName = recorder.getUniqueName("container1");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ describe("Highlevel", () => {
assert.equal(uploadedString, downloadedString);
});

it.only("uploadBrowserDataToBlockBlob should work with tags", async () => {
it("uploadBrowserDataToBlockBlob should work with tags", async () => {
recorder.skip("browser", "Temp file - recorder doesn't support saving the file");

const tags = {
Expand Down
37 changes: 23 additions & 14 deletions sdk/storage/storage-blob/test/node/highlevel.node.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import * as path from "path";
import { PassThrough } from "stream";

import { AbortController } from "@azure/abort-controller";
import { createRandomLocalFile, getBSU, recorderEnvSetup, isBlobVersioningDisabled } from "../utils";
import {
createRandomLocalFile,
getBSU,
recorderEnvSetup,
isBlobVersioningDisabled
} from "../utils";
import { RetriableReadableStreamOptions } from "../../src/utils/RetriableReadableStream";
import { record, Recorder } from "@azure/test-utils-recorder";
import { ContainerClient, BlobClient, BlockBlobClient, BlobServiceClient } from "../../src";
Expand All @@ -28,7 +33,7 @@ describe("Highlevel", () => {
let recorder: Recorder;

let blobServiceClient: BlobServiceClient;
beforeEach(async function () {
beforeEach(async function() {
recorder = record(this, recorderEnvSetup);
blobServiceClient = getBSU();
containerName = recorder.getUniqueName("container");
Expand All @@ -39,14 +44,14 @@ describe("Highlevel", () => {
blockBlobClient = blobClient.getBlockBlobClient();
});

afterEach(async function () {
afterEach(async function() {
if (!this.currentTest?.isPending()) {
await containerClient.delete();
recorder.stop();
}
});

before(async function () {
before(async function() {
recorder = record(this, recorderEnvSetup);
if (!fs.existsSync(tempFolderPath)) {
fs.mkdirSync(tempFolderPath);
Expand All @@ -58,7 +63,7 @@ describe("Highlevel", () => {
recorder.stop();
});

after(async function () {
after(async function() {
recorder = record(this, recorderEnvSetup);
fs.unlinkSync(tempFileLarge);
fs.unlinkSync(tempFileSmall);
Expand All @@ -67,7 +72,7 @@ describe("Highlevel", () => {

it("put blob with maximum size", async () => {
recorder.skip("node", "Temp file - recorder doesn't support saving the file");
const MB = 1024 * 1024
const MB = 1024 * 1024;
const maxPutBlobSizeLimitInMB = 5000;
const tempFile = await createRandomLocalFile(tempFolderPath, maxPutBlobSizeLimitInMB, MB);
const inputStream = fs.createReadStream(tempFile);
Expand All @@ -77,7 +82,7 @@ describe("Highlevel", () => {
abortSignal: AbortController.timeout(20 * 1000) // takes too long to upload the file
});
} catch (err) {
assert.equal(err.name, 'AbortError');
assert.equal(err.name, "AbortError");
}
}).timeout(timeoutForLargeFileUploadingTest);

Expand All @@ -99,7 +104,7 @@ describe("Highlevel", () => {
assert.ok(downloadedData.equals(uploadedData));
}).timeout(timeoutForLargeFileUploadingTest);

it.only("uploadFile should work with tags", async () => {
it("uploadFile should work with tags", async () => {
recorder.skip("node", "Temp file - recorder doesn't support saving the file");

const tags = {
Expand Down Expand Up @@ -201,7 +206,7 @@ describe("Highlevel", () => {
aborter.abort();
}
});
} catch (err) { }
} catch (err) {}
assert.ok(eventTriggered);
});

Expand All @@ -224,20 +229,24 @@ describe("Highlevel", () => {
aborter.abort();
}
});
} catch (err) { }
} catch (err) {}
assert.ok(eventTriggered);
});

it("uploadFile should succeed with blockSize = BLOCK_BLOB_MAX_STAGE_BLOCK_BYTES", async () => {
recorder.skip("node", "Temp file - recorder doesn't support saving the file");
const tempFile = await createRandomLocalFile(tempFolderPath, BLOCK_BLOB_MAX_STAGE_BLOCK_BYTES / (1024 * 1024) + 1, 1024 * 1024);
const tempFile = await createRandomLocalFile(
tempFolderPath,
BLOCK_BLOB_MAX_STAGE_BLOCK_BYTES / (1024 * 1024) + 1,
1024 * 1024
);
try {
await blockBlobClient.uploadFile(tempFile, {
blockSize: BLOCK_BLOB_MAX_STAGE_BLOCK_BYTES,
abortSignal: AbortController.timeout(20 * 1000) // takes too long to upload the file
});
} catch (err) {
assert.equal(err.name, 'AbortError');
assert.equal(err.name, "AbortError");
}

fs.unlinkSync(tempFile);
Expand Down Expand Up @@ -280,7 +289,7 @@ describe("Highlevel", () => {
fs.unlinkSync(downloadFilePath);
});

it.only("uploadStream should work with tags", async () => {
it("uploadStream should work with tags", async () => {
recorder.skip("node", "Temp file - recorder doesn't support saving the file");

const buf = Buffer.from([0x62, 0x75, 0x66, 0x66, 0x65, 0x72]);
Expand Down Expand Up @@ -442,7 +451,7 @@ describe("Highlevel", () => {
aborter.abort();
}
});
} catch (err) { }
} catch (err) {}
assert.ok(eventTriggered);
});

Expand Down
26 changes: 16 additions & 10 deletions sdk/storage/storage-blob/test/node/sas.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ describe("Shared Access Signature (SAS) generation Node.js only", () => {
await containerClient.delete();
});

it.only("generateBlobSASQueryParameters should work for blob tags", async () => {
it("generateBlobSASQueryParameters should work for blob tags", async () => {
const now = recorder.newDate("now");
now.setMinutes(now.getMinutes() - 5); // Skip clock skew with server

Expand Down Expand Up @@ -862,8 +862,10 @@ describe("Shared Access Signature (SAS) generation Node.js only", () => {
await containerClient.delete();
});

it("generateAccountSASQueryParameters should work for blob version delete", async function () {
if (isBlobVersioningDisabled()) { this.skip(); }
it("generateAccountSASQueryParameters should work for blob version delete", async function() {
if (isBlobVersioningDisabled()) {
this.skip();
}

// create versions
const containerName = recorder.getUniqueName("container");
Expand Down Expand Up @@ -906,8 +908,10 @@ describe("Shared Access Signature (SAS) generation Node.js only", () => {
await containerClientwithSAS.delete();
});

it("generateBlobSASQueryParameters should work for blob version delete", async function () {
if (isBlobVersioningDisabled()) { this.skip(); }
it("generateBlobSASQueryParameters should work for blob version delete", async function() {
if (isBlobVersioningDisabled()) {
this.skip();
}

// create versions
const containerName = recorder.getUniqueName("container");
Expand Down Expand Up @@ -939,7 +943,7 @@ describe("Shared Access Signature (SAS) generation Node.js only", () => {
ipRange: { start: "0.0.0.0", end: "255.255.255.255" },
permissions: BlobSASPermissions.parse("racwdx"),
protocol: SASProtocol.HttpsAndHttp,
versionId: uploadRes.versionId,
versionId: uploadRes.versionId
},
sharedKeyCredential as StorageSharedKeyCredential
);
Expand All @@ -953,15 +957,17 @@ describe("Shared Access Signature (SAS) generation Node.js only", () => {
});

// TODO: prepare ACCOUNT_TOKEN for the test account
it.skip("GenerateUserDelegationSAS should work for blob version delete", async function () {
if (isBlobVersioningDisabled()) { this.skip(); }
it.skip("GenerateUserDelegationSAS should work for blob version delete", async function() {
if (isBlobVersioningDisabled()) {
this.skip();
}

// Try to get blobServiceClient object with TokenCredential
// when ACCOUNT_TOKEN environment variable is set
let blobServiceClientWithToken: BlobServiceClient | undefined;
try {
blobServiceClientWithToken = getTokenBSU();
} catch { }
} catch {}

// Requires bearer token for this case which cannot be generated in the runtime
// Make sure this case passed in sanity test
Expand Down Expand Up @@ -1001,7 +1007,7 @@ describe("Shared Access Signature (SAS) generation Node.js only", () => {
ipRange: { start: "0.0.0.0", end: "255.255.255.255" },
permissions: BlobSASPermissions.parse("racwdx"),
protocol: SASProtocol.HttpsAndHttp,
versionId: uploadRes.versionId,
versionId: uploadRes.versionId
},
userDelegationKey,
accountName
Expand Down

0 comments on commit fc86b0f

Please sign in to comment.