Skip to content

feat: override methods in sdk #2668

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

Merged
chronark merged 2 commits into from
Nov 25, 2024
Merged

feat: override methods in sdk #2668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a92f1d7
feat: override methods in sdk
chronark Nov 18, 2024
4df4054
Merge branch 'main' into override-sdk
chronark Nov 25, 2024
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: 5 additions & 0 deletions .changeset/wild-spies-punch.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@unkey/api": minor
---

Add ratelimit override API
22 changes: 11 additions & 11 deletions apps/api/src/routes/v1_keys_getVerifications.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,10 @@ export const registerV1KeysGetVerifications = (app: App) =>
roles: dbRes.roles.map((p) => p.role.name),
identity: dbRes.identity
? {
id: dbRes.identity.id,
externalId: dbRes.identity.externalId,
meta: dbRes.identity.meta,
}
id: dbRes.identity.id,
externalId: dbRes.identity.externalId,
meta: dbRes.identity.meta,
}
: null,
};
});
Expand Down Expand Up @@ -249,20 +249,20 @@ export const registerV1KeysGetVerifications = (app: App) =>
verifications[d.time] = { success: 0, rateLimited: 0, usageExceeded: 0 };
}
switch (d.outcome) {
case "VALID":
// case "VALID":
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 18, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the git history and code context, I can now provide a final response:

Commenting out "VALID" case appears to be unintentional

The change was introduced in commit a92f1d7 "feat: override methods in sdk" but seems to be an accidental formatting change rather than an intentional feature modification because:

  • The code still increments the success counter (verifications[d.time].success += d.count)
  • The case is only commented out, not properly removed
  • Other verification types (RATE_LIMITED, USAGE_EXCEEDED) remain active
  • The change appears alongside formatting-only modifications in the commit

This should be fixed by uncommenting the "VALID" case to maintain proper verification counting.

🔗 Analysis chain

Verify the intent of commenting out the "VALID" case

The "VALID" case has been commented out, which means successful verifications are not being counted. Is this intentional and related to the rate limit override API changes mentioned in the PR objectives?

Let me gather more context about this specific file and the changes around this code section.

Let me try one more verification to understand if this is part of recent changes.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for related changes in verification handling
rg -A 5 "case \"VALID\""

Length of output: 2152

Script:

#!/bin/bash
# Get the full context of the verification handling in this file
rg -B 10 -A 10 "case \"VALID\"" apps/api/src/routes/v1_keys_getVerifications.ts

# Check git history for this specific line
git log -L '/case "VALID":/apps/api/src/routes/v1_keys_getVerifications.ts'

Length of output: 922

Script:

#!/bin/bash
# Check recent commits that modified this file
git log --patch apps/api/src/routes/v1_keys_getVerifications.ts

# Also check if there are any related test files
fd "v1_keys_getVerifications" --type f

Length of output: 14265

verifications[d.time].success += d.count;
break;
break;
case "RATE_LIMITED":
verifications[d.time].rateLimited += d.count;
break;
verifications[d.time].rateLimited += d.count;
break;
case "USAGE_EXCEEDED":
verifications[d.time].usageExceeded += d.count;
break;
verifications[d.time].usageExceeded += d.count;
break;
Comment on lines +252 to +260
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 18, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix broken switch statement syntax and logic

There are several critical issues in the switch statement:

  1. The syntax is incorrect, leading to parsing errors
  2. The "VALID" case is commented out, which means successful verifications aren't being counted
  3. The break statements are unreachable due to incorrect structure

Apply this fix to restore proper functionality:

        switch (d.outcome) {
-          // case "VALID":
-            verifications[d.time].success += d.count;
-        break;
-          case "RATE_LIMITED":
-verifications[d.time].rateLimited += d.count;
-break;
-          case "USAGE_EXCEEDED":
-verifications[d.time].usageExceeded += d.count;
-break;
+          case "VALID":
+            verifications[d.time].success += d.count;
+            break;
+          case "RATE_LIMITED":
+            verifications[d.time].rateLimited += d.count;
+            break;
+          case "USAGE_EXCEEDED":
+            verifications[d.time].usageExceeded += d.count;
+            break;
+          default:
+            logger.warn(`Unknown verification outcome: ${d.outcome}`);
        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// case "VALID":
verifications[d.time].success += d.count;
break;
break;
case "RATE_LIMITED":
verifications[d.time].rateLimited += d.count;
break;
verifications[d.time].rateLimited += d.count;
break;
case "USAGE_EXCEEDED":
verifications[d.time].usageExceeded += d.count;
break;
verifications[d.time].usageExceeded += d.count;
break;
switch (d.outcome) {
case "VALID":
verifications[d.time].success += d.count;
break;
case "RATE_LIMITED":
verifications[d.time].rateLimited += d.count;
break;
case "USAGE_EXCEEDED":
verifications[d.time].usageExceeded += d.count;
break;
default:
logger.warn(`Unknown verification outcome: ${d.outcome}`);
}
🧰 Tools
🪛 Biome

[error] 253-253: Expected a default, or a case but instead found 'verifications'.

Expected a default, or a case here.

(parse)

[error] 255-256: Expected a statement but instead found 'case "RATE_LIMITED":
verifications[d.time].rateLimited += d.count'.

Expected a statement here.

(parse)

[error] 258-259: Expected a statement but instead found 'case "USAGE_EXCEEDED":
verifications[d.time].usageExceeded += d.count'.

Expected a statement here.

(parse)

[error] 256-257: This code is unreachable

... because this statement will break the flow of the code beforehand

(lint/correctness/noUnreachable)

[error] 259-260: This code is unreachable

... because this statement will break the flow of the code beforehand

(lint/correctness/noUnreachable)

}
}
}

// really ugly hack to return an emoty array in case there wasn't a single verification
// really ugly hack to return an empty array in case there wasn't a single verification
// this became necessary when we switched to clickhouse, due to the different responses
if (
Object.values(verifications).every(({ success, rateLimited, usageExceeded }) => {
Expand Down
164 changes: 82 additions & 82 deletions packages/api/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,25 @@ import { type Telemetry, getTelemetry } from "./telemetry";

export type UnkeyOptions = (
| {
token?: never;
token?: never;

/**
* The root key from unkey.dev.
*
* You can create/manage your root keys here:
* https://unkey.dev/app/settings/root-keys
*/
rootKey: string;
}
/**
* The root key from unkey.dev.
*
* You can create/manage your root keys here:
* https://unkey.dev/app/settings/root-keys
*/
rootKey: string;
}
| {
/**
* The workspace key from unkey.dev
*
* @deprecated Use `rootKey`
*/
token: string;
rootKey?: never;
}
/**
* The workspace key from unkey.dev
*
* @deprecated Use `rootKey`
*/
token: string;
rootKey?: never;
}
) & {
/**
* @default https://api.unkey.dev
Expand Down Expand Up @@ -77,32 +77,32 @@ export type UnkeyOptions = (
type ApiRequest = {
path: string[];
} & (
| {
| {
method: "GET";
body?: never;
query?: Record<string, string | number | boolean | null>;
}
| {
| {
method: "POST";
body?: unknown;
query?: never;
}
);
);

type Result<R> =
| {
result: R;
error?: never;
}
result: R;
error?: never;
}
| {
result?: never;
error: {
code: ErrorResponse["error"]["code"];
message: ErrorResponse["error"]["message"];
docs: ErrorResponse["error"]["docs"];
requestId: string;
};
result?: never;
error: {
code: ErrorResponse["error"]["code"];
message: ErrorResponse["error"]["message"];
docs: ErrorResponse["error"]["docs"];
requestId: string;
};
};

export class Unkey {
public readonly baseUrl: string;
Expand Down Expand Up @@ -376,60 +376,60 @@ export class Unkey {
body: req,
});
},
// getOverride: async (
// req: paths["/v1/ratelimits.getOverride"]["get"]["parameters"]["query"],
// ): Promise<
// Result<
// paths["/v1/ratelimits.getOverride"]["get"]["responses"]["200"]["content"]["application/json"]
// >
// > => {
// return await this.fetch({
// path: ["v1", "ratelimits.getOverride"],
// method: "GET",
// query: req,
// });
// },
// listOverrides: async (
// req: paths["/v1/ratelimits.listOverrides"]["get"]["parameters"]["query"],
// ): Promise<
// Result<
// paths["/v1/ratelimits.listOverrides"]["get"]["responses"]["200"]["content"]["application/json"]
// >
// > => {
// return await this.fetch({
// path: ["v1", "ratelimits.listOverrides"],
// method: "GET",
// query: req,
// });
// },
getOverride: async (
req: paths["/v1/ratelimits.getOverride"]["get"]["parameters"]["query"],
): Promise<
Result<
paths["/v1/ratelimits.getOverride"]["get"]["responses"]["200"]["content"]["application/json"]
>
> => {
return await this.fetch({
path: ["v1", "ratelimits.getOverride"],
method: "GET",
query: req,
});
},
listOverrides: async (
req: paths["/v1/ratelimits.listOverrides"]["get"]["parameters"]["query"],
): Promise<
Result<
paths["/v1/ratelimits.listOverrides"]["get"]["responses"]["200"]["content"]["application/json"]
>
> => {
return await this.fetch({
path: ["v1", "ratelimits.listOverrides"],
method: "GET",
query: req,
});
},

// setOverride: async (
// req: paths["/v1/ratelimits.setOverride"]["post"]["requestBody"]["content"]["application/json"],
// ): Promise<
// Result<
// paths["/v1/ratelimits.setOverride"]["post"]["responses"]["200"]["content"]["application/json"]
// >
// > => {
// return await this.fetch({
// path: ["v1", "ratelimits.setOverride"],
// method: "POST",
// body: req,
// });
// },
setOverride: async (
req: paths["/v1/ratelimits.setOverride"]["post"]["requestBody"]["content"]["application/json"],
): Promise<
Result<
paths["/v1/ratelimits.setOverride"]["post"]["responses"]["200"]["content"]["application/json"]
>
> => {
return await this.fetch({
path: ["v1", "ratelimits.setOverride"],
method: "POST",
body: req,
});
},

// deleteOverride: async (
// req: paths["/v1/ratelimits.deleteOverride"]["post"]["requestBody"]["content"]["application/json"],
// ): Promise<
// Result<
// paths["/v1/ratelimits.deleteOverride"]["post"]["responses"]["200"]["content"]["application/json"]
// >
// > => {
// return await this.fetch({
// path: ["v1", "ratelimits.deleteOverride"],
// method: "POST",
// body: req,
// });
// },
deleteOverride: async (
req: paths["/v1/ratelimits.deleteOverride"]["post"]["requestBody"]["content"]["application/json"],
): Promise<
Result<
paths["/v1/ratelimits.deleteOverride"]["post"]["responses"]["200"]["content"]["application/json"]
>
> => {
return await this.fetch({
path: ["v1", "ratelimits.deleteOverride"],
method: "POST",
body: req,
});
},
};
}
public get identities() {
Expand Down
Loading