Skip to content

Commit

Permalink
Fixes & updates for mismatch behaviors found during .net sdk testing (#…
Browse files Browse the repository at this point in the history 
…369)

* Copy blob will override existing blob

* Update account SAS permission requirements for get account properties

* Update error code from 400 to 409 when page operations agasint block blob

* REturn 206 for downloading partial page blob, fixed #367

* Update blob createtime when commit block blob first create blob without override
  • Loading branch information
@XiaoningLiu @blueww
XiaoningLiu authored and blueww committed Dec 26, 2019
1 parent ac31c87 commit 53d61fa
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 30 deletions.
85 changes: 72 additions & 13 deletions src/blob/authentication/OperationAccountSASPermission.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ export class OperationAccountSASPermission {
public validateResourceTypes(
resourceTypes: AccountSASResourceTypes | string
): boolean {
return resourceTypes.toString().includes(this.resourceType);
for (const p of this.resourceType) {
if (resourceTypes.toString().includes(p)) {
return true;
}
}
return false;
}

public validatePermissions(
Expand All @@ -61,53 +66,107 @@ OPERATION_ACCOUNT_SAS_PERMISSIONS.set(
Operation.Service_GetAccountInfo,
new OperationAccountSASPermission(
AccountSASService.Blob,
AccountSASResourceType.Service,
AccountSASPermission.Read
AccountSASResourceType.Service +
AccountSASResourceType.Container +
AccountSASResourceType.Object,
AccountSASPermission.Read +
AccountSASPermission.Create +
AccountSASPermission.Delete +
AccountSASPermission.List +
AccountSASPermission.Process +
AccountSASPermission.Read +
AccountSASPermission.Update +
AccountSASPermission.Write
)
);

OPERATION_ACCOUNT_SAS_PERMISSIONS.set(
Operation.Service_GetAccountInfoWithHead,
new OperationAccountSASPermission(
AccountSASService.Blob,
AccountSASResourceType.Service,
AccountSASPermission.Read
AccountSASResourceType.Service +
AccountSASResourceType.Container +
AccountSASResourceType.Object,
AccountSASPermission.Read +
AccountSASPermission.Create +
AccountSASPermission.Delete +
AccountSASPermission.List +
AccountSASPermission.Process +
AccountSASPermission.Read +
AccountSASPermission.Update +
AccountSASPermission.Write
)
);

OPERATION_ACCOUNT_SAS_PERMISSIONS.set(
Operation.Container_GetAccountInfo,
new OperationAccountSASPermission(
AccountSASService.Blob,
AccountSASResourceType.Service,
AccountSASPermission.Read
AccountSASResourceType.Service +
AccountSASResourceType.Container +
AccountSASResourceType.Object,
AccountSASPermission.Read +
AccountSASPermission.Create +
AccountSASPermission.Delete +
AccountSASPermission.List +
AccountSASPermission.Process +
AccountSASPermission.Read +
AccountSASPermission.Update +
AccountSASPermission.Write
)
);

OPERATION_ACCOUNT_SAS_PERMISSIONS.set(
Operation.Container_GetAccountInfoWithHead,
new OperationAccountSASPermission(
AccountSASService.Blob,
AccountSASResourceType.Service,
AccountSASPermission.Read
AccountSASResourceType.Service +
AccountSASResourceType.Container +
AccountSASResourceType.Object,
AccountSASPermission.Read +
AccountSASPermission.Create +
AccountSASPermission.Delete +
AccountSASPermission.List +
AccountSASPermission.Process +
AccountSASPermission.Read +
AccountSASPermission.Update +
AccountSASPermission.Write
)
);

OPERATION_ACCOUNT_SAS_PERMISSIONS.set(
Operation.Blob_GetAccountInfo,
new OperationAccountSASPermission(
AccountSASService.Blob,
AccountSASResourceType.Service,
AccountSASPermission.Read
AccountSASResourceType.Service +
AccountSASResourceType.Container +
AccountSASResourceType.Object,
AccountSASPermission.Read +
AccountSASPermission.Create +
AccountSASPermission.Delete +
AccountSASPermission.List +
AccountSASPermission.Process +
AccountSASPermission.Read +
AccountSASPermission.Update +
AccountSASPermission.Write
)
);

OPERATION_ACCOUNT_SAS_PERMISSIONS.set(
Operation.Blob_GetAccountInfoWithHead,
new OperationAccountSASPermission(
AccountSASService.Blob,
AccountSASResourceType.Service,
AccountSASPermission.Read
AccountSASResourceType.Service +
AccountSASResourceType.Container +
AccountSASResourceType.Object,
AccountSASPermission.Read +
AccountSASPermission.Create +
AccountSASPermission.Delete +
AccountSASPermission.List +
AccountSASPermission.Process +
AccountSASPermission.Read +
AccountSASPermission.Update +
AccountSASPermission.Write
)
);

Expand Down
3 changes: 2 additions & 1 deletion src/blob/handlers/BlobHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,8 @@ export default class BlobHandler extends BaseHandler implements IBlobHandler {
}

const response: Models.BlobDownloadResponse = {
statusCode: rangesParts[1] === Infinity ? 200 : 206,
statusCode:
rangesParts[1] === Infinity && rangesParts[0] === 0 ? 200 : 206,
body,
metadata: blob.metadata,
eTag: blob.properties.etag,
Expand Down
3 changes: 2 additions & 1 deletion src/blob/handlers/BlockBlobHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ export default class BlockBlobHandler extends BaseHandler
name: blobName,
snapshot: "",
properties: {
lastModified: new Date(),
lastModified: context.startTime!,
creationTime: context.startTime!,
etag: newEtag()
},
isCommitted: true
Expand Down
15 changes: 3 additions & 12 deletions src/blob/handlers/PageBlobHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,7 @@ export default class PageBlobHandler extends BaseHandler
);

if (blob.properties.blobType !== Models.BlobType.PageBlob) {
throw StorageErrorFactory.getInvalidOperation(
blobCtx.contextId!,
"Get Page Ranges could only be against a page blob."
);
throw StorageErrorFactory.getBlobInvalidBlobType(blobCtx.contextId!);
}

// Check Lease status
Expand Down Expand Up @@ -276,10 +273,7 @@ export default class PageBlobHandler extends BaseHandler
);

if (blob.properties.blobType !== Models.BlobType.PageBlob) {
throw StorageErrorFactory.getInvalidOperation(
blobCtx.contextId!,
"Get Page Ranges could only be against a page blob."
);
throw StorageErrorFactory.getBlobInvalidBlobType(blobCtx.contextId!);
}

let ranges;
Expand Down Expand Up @@ -339,10 +333,7 @@ export default class PageBlobHandler extends BaseHandler
);

if (blob.properties.blobType !== Models.BlobType.PageBlob) {
throw StorageErrorFactory.getInvalidOperation(
blobCtx.contextId!,
"Get Page Ranges could only be against a page blob."
);
throw StorageErrorFactory.getBlobInvalidBlobType(blobCtx.contextId!);
}

let ranges = deserializePageBlobRangeHeader(
Expand Down
4 changes: 4 additions & 0 deletions src/blob/persistence/LokiBlobMetadataStore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1722,6 +1722,9 @@ export default class LokiBlobMetadataStore
});
}

if (destBlob) {
coll.remove(destBlob);
}
coll.insert(copiedBlob);
return copiedBlob.properties;
}
Expand Down Expand Up @@ -1970,6 +1973,7 @@ export default class LokiBlobMetadataStore
if (doc) {
// Commit block list
doc.properties.blobType = blob.properties.blobType;
doc.properties.lastModified = blob.properties.lastModified;
doc.committedBlocksInOrder = selectedBlockList;
doc.isCommitted = true;
doc.metadata = blob.metadata;
Expand Down
6 changes: 5 additions & 1 deletion src/blob/persistence/SqlBlobMetadataStore.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1444,11 +1444,15 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
transaction: t
});

let creationTime = blob.properties.creationTime || context.startTime;

if (blobFindResult !== null && blobFindResult !== undefined) {
const blobModel: BlobModel = this.convertDbModelToBlobModel(
blobFindResult
);

creationTime = blobModel.properties.creationTime || creationTime;

LeaseFactory.createLeaseState(
new BlobLeaseAdapter(blobModel),
context
Expand Down Expand Up @@ -1519,7 +1523,7 @@ export default class SqlBlobMetadataStore implements IBlobMetadataStore {
committedBlocksInOrder: selectedBlockList,
properties: {
...blob.properties,
creationTime: blob.properties.creationTime || context.startTime,
creationTime,
lastModified: blob.properties.lastModified || context.startTime,
contentLength: selectedBlockList
.map(block => block.size)
Expand Down
7 changes: 6 additions & 1 deletion src/queue/authentication/OperationAccountSASPermission.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ export class OperationAccountSASPermission {
public validateResourceTypes(
resourceTypes: AccountSASResourceTypes | string
): boolean {
return resourceTypes.toString().includes(this.resourceType);
for (const p of this.resourceType) {
if (resourceTypes.toString().includes(p)) {
return true;
}
}
return false;
}

public validatePermissions(
Expand Down
8 changes: 8 additions & 0 deletions tests/blob/apis/blockblob.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,10 @@ describe("BlockBlobAPIs", () => {
result_commit._response.request.headers.get("x-ms-client-request-id"),
result_commit.clientRequestId
);

const properties1 = await blockBlobURL.getProperties(Aborter.none);
assert.notDeepStrictEqual(properties1.creationTime, undefined);

const listResponse = await blockBlobURL.getBlockList(
Aborter.none,
"committed"
Expand All @@ -259,6 +263,10 @@ describe("BlockBlobAPIs", () => {
assert.equal(listResponse2.committedBlocks!.length, 1);
assert.equal(listResponse2.committedBlocks![0].name, base64encode("2"));
assert.equal(listResponse2.committedBlocks![0].size, body.length);

const properties2 = await blockBlobURL.getProperties(Aborter.none);
assert.notDeepStrictEqual(properties2.creationTime, undefined);
assert.deepStrictEqual(properties1.creationTime, properties2.creationTime);
});

it("commitBlockList with empty list should create an empty block blob @loki @sql", async () => {
Expand Down
6 changes: 5 additions & 1 deletion tests/blob/apis/pageblob.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ describe("PageBlobAPIs", () => {
ranges._response.request.headers.get("x-ms-client-request-id"),
ranges.clientRequestId
);
const result = await blobURL.download(Aborter.none, 0, 10);
let result = await blobURL.download(Aborter.none, 0, 10);
assert.deepStrictEqual(result.contentRange, `bytes 0-9/5120`);
assert.deepStrictEqual(
await bodyToString(result, length),
Expand All @@ -162,6 +162,10 @@ describe("PageBlobAPIs", () => {
result._response.request.headers.get("x-ms-client-request-id"),
result.clientRequestId
);

result = await blobURL.download(Aborter.none, 1);
assert.deepStrictEqual(result.contentRange, `bytes 1-5119/5120`);
assert.deepStrictEqual(result._response.status, 206);
});

it("download page blob with no ranges uploaded @loki", async () => {
Expand Down

0 comments on commit 53d61fa

Please sign in to comment.