Skip to content

Commit f2a8a39

Browse files
authored
[storage-blob] Fix paths in client instead of using a pipeline policy (Azure#25381)
### Packages impacted by this PR `@azure/storage-blob` ### Describe the problem that is addressed by this PR When generating the client library, the OpenAPI input specification uses path parameters to model sending the container and blob ids to the service. However, since these ids require special handling to be escaped properly, the SDK manages the complete URL itself. This causes the path parameters to be duplicated when processed by core-client. To avoid this problem, I previously used a pipeline policy that removed the duplicate information. While migrating `storage-file-datalake`, I thought of a different solution, namely that of having the client itself modify the OperationSpec directly before the request is generated. This results in less work at runtime and is a bit cleaner in terms of making each operation request. This PR ports the same approach to storage-blob.
1 parent fcb54f0 commit f2a8a39

File tree

6 files changed

+29
-39
lines changed

6 files changed

+29
-39
lines changed

sdk/storage/storage-blob/src/BlobBatch.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import { StorageSharedKeyCredential } from "./credentials/StorageSharedKeyCreden
3131
import { tracingClient } from "./utils/tracing";
3232
import { authorizeRequestOnTenantChallenge, serializationPolicy } from "@azure/core-client";
3333
import { storageSharedKeyCredentialPolicy } from "./policies/StorageSharedKeyCredentialPolicyV2";
34-
import { pathParameterWorkaroundPolicy } from "./policies/PathParameterWorkaroundPolicy";
3534

3635
/**
3736
* A request associated with a batch operation.
@@ -370,7 +369,6 @@ class InnerBatchRequest {
370369
}),
371370
{ phase: "Serialize" }
372371
);
373-
corePipeline.addPolicy(pathParameterWorkaroundPolicy());
374372
// Use batch header filter policy to exclude unnecessary headers
375373
corePipeline.addPolicy(batchHeaderFilterPolicy());
376374
// Use batch assemble policy to assemble request and intercept request from going to wire

sdk/storage/storage-blob/src/BlobBatchClient.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { Service, Container } from "./generated/src/operationsInterfaces";
1717
import { StorageSharedKeyCredential } from "./credentials/StorageSharedKeyCredential";
1818
import { AnonymousCredential } from "./credentials/AnonymousCredential";
1919
import { BlobDeleteOptions, BlobClient, BlobSetTierOptions } from "./Clients";
20-
import { StorageClient as StorageClientContext } from "./generated/src/storageClient";
20+
import { StorageContextClient } from "./StorageContextClient";
2121
import {
2222
PipelineLike,
2323
StoragePipelineOptions,
@@ -106,7 +106,7 @@ export class BlobBatchClient {
106106
pipeline = newPipeline(credentialOrPipeline, options);
107107
}
108108

109-
const storageClientContext = new StorageClientContext(url, getCoreClientOptions(pipeline));
109+
const storageClientContext = new StorageContextClient(url, getCoreClientOptions(pipeline));
110110

111111
const path = getURLPath(url);
112112
if (path && path !== "/") {

sdk/storage/storage-blob/src/Pipeline.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ import { getCachedDefaultHttpClient } from "./utils/cache";
4343
import { storageBrowserPolicy } from "./policies/StorageBrowserPolicyV2";
4444
import { storageRetryPolicy } from "./policies/StorageRetryPolicyV2";
4545
import { storageSharedKeyCredentialPolicy } from "./policies/StorageSharedKeyCredentialPolicyV2";
46-
import { pathParameterWorkaroundPolicy } from "./policies/PathParameterWorkaroundPolicy";
4746
import { StorageBrowserPolicyFactory } from "./StorageBrowserPolicyFactory";
4847

4948
// Export following interfaces and types for customers who want to implement their
@@ -300,7 +299,6 @@ export function getCoreClientOptions(pipeline: PipelineLike): ExtendedServiceCli
300299
});
301300
corePipeline.removePolicy({ phase: "Retry" });
302301
corePipeline.removePolicy({ name: decompressResponsePolicyName });
303-
corePipeline.addPolicy(pathParameterWorkaroundPolicy());
304302
corePipeline.addPolicy(storageRetryPolicy(restOptions.retryOptions), { phase: "Retry" });
305303
corePipeline.addPolicy(storageBrowserPolicy());
306304
const downlevelResults = processDownlevelPipeline(pipeline);

sdk/storage/storage-blob/src/StorageClient.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license.
33

44
import { StorageClient as StorageClientContext } from "./generated/src/";
5+
import { StorageContextClient } from "./StorageContextClient";
56
import { getCoreClientOptions, getCredentialFromPipeline, PipelineLike } from "./Pipeline";
67
import { escapeURLPath, getURLScheme, iEqual, getAccountNameFromUrl } from "./utils/utils.common";
78
import { AnonymousCredential } from "./credentials/AnonymousCredential";
@@ -58,7 +59,7 @@ export abstract class StorageClient {
5859
this.url = escapeURLPath(url);
5960
this.accountName = getAccountNameFromUrl(url);
6061
this.pipeline = pipeline;
61-
this.storageClientContext = new StorageClientContext(this.url, getCoreClientOptions(pipeline));
62+
this.storageClientContext = new StorageContextClient(this.url, getCoreClientOptions(pipeline));
6263

6364
this.isHttps = iEqual(getURLScheme(this.url) || "", "https");
6465

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
import { OperationArguments, OperationSpec } from "@azure/core-client";
5+
import { StorageClient } from "./generated/src";
6+
7+
/**
8+
* @internal
9+
*/
10+
export class StorageContextClient extends StorageClient {
11+
async sendOperationRequest<T>(
12+
operationArguments: OperationArguments,
13+
operationSpec: OperationSpec
14+
): Promise<T> {
15+
const operationSpecToSend = { ...operationSpec };
16+
17+
if (
18+
operationSpecToSend.path === "/{containerName}" ||
19+
operationSpecToSend.path === "/{containerName}/{blob}"
20+
) {
21+
operationSpecToSend.path = "";
22+
}
23+
return super.sendOperationRequest(operationArguments, operationSpecToSend);
24+
}
25+
}

sdk/storage/storage-blob/src/policies/PathParameterWorkaroundPolicy.ts

Lines changed: 0 additions & 32 deletions
This file was deleted.

0 commit comments

Comments
 (0)