Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable the batch tsp & chat api tsp #2090

Merged
merged 23 commits into from
Nov 1, 2023
Merged

Enable the batch tsp & chat api tsp #2090

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
aa2b4c8
enable the batch tsp
MaryGao Oct 30, 2023
c81f020
Fix batch header issues
MaryGao Oct 30, 2023
be7bba1
Fix the empty credential scopes case in RLC
MaryGao Oct 30, 2023
375d30d
Set the proper credentialScopes values
MaryGao Oct 30, 2023
1091ccc
Refactor code little
MaryGao Oct 30, 2023
447162e
Update the api view for api chat sdk
MaryGao Oct 30, 2023
c92bfbc
Remove useless files
MaryGao Oct 30, 2023
5ce99da
Add UTs to cover empty scopes
MaryGao Oct 30, 2023
4f3817d
Update the comments
MaryGao Oct 30, 2023
54a4c80
Update the comments
MaryGao Oct 30, 2023
d18c217
Debug git dirty
MaryGao Oct 30, 2023
5566793
Merge remote-tracking branch 'origin/main' into enable-batch-test
MaryGao Oct 30, 2023
19beec6
Update the batch testing
MaryGao Oct 30, 2023
4697bc2
Revert changes
MaryGao Oct 30, 2023
f8df8c1
Merge branch 'main' into enable-batch-test
MaryGao Oct 31, 2023
223ebea
Update the default value for empty scopes
MaryGao Oct 31, 2023
96b30e2
Support the nullable header in modular
MaryGao Oct 31, 2023
18ad380
Fix lint issue
MaryGao Oct 31, 2023
b0ec9fe
Remove useless files
MaryGao Oct 31, 2023
4b00247
Refactor the credential option positions
MaryGao Nov 1, 2023
f318b2e
Update the ut after refactoring code
MaryGao Nov 1, 2023
b00c128
Regenerate smoke and integration files
MaryGao Nov 1, 2023
b1dec56
Regenerate in autorest package
MaryGao Nov 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/autorest.typescript/src/utils/schemaHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,10 @@ export function getSecurityInfoFromModel(security: Security) {
const { addCredentials } = getAutorestOptions();
const credentialScopes: Set<string> = new Set<string>();
let credentialKeyHeaderName: string = "";
let hasOAuth2Defined = false;
for (const securitySchema of security.schemes) {
if (securitySchema.type === "OAuth2") {
hasOAuth2Defined = true;
(securitySchema as OAuth2SecurityScheme).scopes.forEach(scope => {
const scopes = scope.split(",");
for (const scope of scopes) {
Expand Down Expand Up @@ -273,7 +275,8 @@ export function getSecurityInfoFromModel(security: Security) {
}
return {
addCredentials: refinedAddCredentials,
credentialScopes: scopes,
credentialScopes:
!hasOAuth2Defined && scopes.length === 0 ? undefined : scopes,
credentialKeyHeaderName: credentialKeyHeaderName
};
}
14 changes: 5 additions & 9 deletions packages/rlc-common/src/buildClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ export function buildClient(model: RLCModel): File | undefined {
credentialKeyHeaderName,
customHttpAuthHeaderName
} = model.options;
const credentialTypes =
credentialScopes && credentialScopes.length > 0 ? ["TokenCredential"] : [];
const credentialTypes = credentialScopes ? ["TokenCredential"] : [];
MaryGao marked this conversation as resolved.
Show resolved Hide resolved

if (credentialKeyHeaderName || customHttpAuthHeaderName) {
credentialTypes.push("KeyCredential");
Expand Down Expand Up @@ -211,9 +210,7 @@ function isSecurityInfoDefined(
customHttpAuthHeaderName?: string
) {
return (
(credentialScopes && credentialScopes.length > 0) ||
credentialKeyHeaderName ||
customHttpAuthHeaderName
credentialScopes || credentialKeyHeaderName || customHttpAuthHeaderName
);
}

Expand Down Expand Up @@ -311,10 +308,9 @@ export function getClientFactoryBody(

const { credentialScopes, credentialKeyHeaderName } = model.options;

const scopesString =
credentialScopes && credentialScopes.length
? credentialScopes.map((cs) => `"${cs}"`).join(", ")
: "";
const scopesString = (credentialScopes ?? [])
.map((cs) => `"${cs}"`)
.join(", ");
const scopes = scopesString
? `scopes: options.credentials?.scopes ?? [${scopesString}],`
: "";
Expand Down
5 changes: 5 additions & 0 deletions packages/rlc-common/src/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ export interface RLCOptions {
batch?: any[];
packageDetails?: PackageDetails;
addCredentials?: boolean;
/** Three possiblie values:
* - undefined, no credentialScopes and relevant settings would be generated
* - [], which means we would generate TokenCredential but no credentialScopes and relevant settings
* - ["..."], which means we would generate credentialScopes and relevant settings with the given values
*/
Copy link
Contributor Author

@MaryGao MaryGao Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we handle the undefined and empty array as different cases.

undefined means we don't have OAuth2 defined;

[] means we have OAuth2 defined but with empty scopes.

Previously we treat the above two cases the same so all of them will not generate TokenCredential in constructor. But now they are different. The first would not but the latter would generate.

Please notice that we will not set the credentialScopes default value as []. If the value is absent the core would populate with {endpoint}/.default automatically.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another place where we should consider if we are generating an Azure SDK or not since the {endpoint}/.default value only makes sense for Azure services.

Copy link
Contributor Author

@MaryGao MaryGao Oct 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, there may be another question - how to handle the empty scopes for non-Azure? How about explicitly set this as an empty array? Do we need to change the default logic for in @typespec/ts-http-runtime?

credentialScopes: []

But I think this topic would not block current pr considering we have a on-going one for integration #2083.

credentialScopes?: string[];
credentialKeyHeaderName?: string;
customHttpAuthHeaderName?: string;
Expand Down
4 changes: 1 addition & 3 deletions packages/rlc-common/src/transformSampleGroups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,7 @@ function convertClientLevelParameters(
model.options;
const hasUrlParameter = !!urlParameters,
hasCredentials =
addCredentials &&
((credentialScopes && credentialScopes.length > 0) ||
credentialKeyHeaderName);
addCredentials && (credentialScopes || credentialKeyHeaderName);

if (hasUrlParameter) {
// convert the host parameters in url
Expand Down
Loading
Loading