refactor(api): finalize CustomerEntitlement APIs under V2 and fix types #3210
Conversation
GAlexIHU
added
the
release-note/ignore
📝 WalkthroughWalkthroughRemoves the v1 customer entitlements list and most management endpoints, adds a new OpenMeter.Entitlements.V2 API (models, customer endpoints, grants), exposes V2 via OpenMeterCloud.Entitlements.V2, updates router handlers/stubs for V2, and keeps the v1 entitlement value lookup. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
openmeter/server/router/entitlement.go (1)
177-181: V2 value endpoint also drops query paramsSame issue as v1 variant: params aren’t forwarded to the driver.
func (a *Router) GetCustomerEntitlementValueV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string, params api.GetCustomerEntitlementValueV2Params) { a.customerHandler.GetCustomerEntitlementValue().With(customerdriver.GetCustomerEntitlementValueParams{ CustomerIDOrKey: customerIdOrKey, FeatureKey: featureKey, + Params: params, }).ServeHTTP(w, r) }
🧹 Nitpick comments (11)
api/spec/src/entitlements/entitlements.tsp (3)
272-276: Clarify and formally deprecate subjectKey in v1 shapesThe note is good, but consider marking
subjectKeyas deprecated to surface the transition in generated clients and docs, and reference the v2 customer-scoped surface./** * The identifier key unique to the subject. * NOTE: Subjects are being deprecated, please use the new customer APIs. */ @example("customer-1") + #deprecated "Subjects are being deprecated. Use customer-scoped v2 endpoints (OpenMeterCloud.Entitlements.V2.*)." subjectKey: Key;
149-151: Typo in docs: “inpurs” → “inputs”Minor documentation fix.
- * Create inpurs for metered entitlement + * Create inputs for metered entitlement
326-329: Typo in docs: “THe” → “The”Minor documentation fix.
- * THe usage period of the entitlement. + * The usage period of the entitlement.api/spec/src/entitlements/v2/main.tsp (1)
21-21: Grammar nit: “should will” → “should”Minor comment clarity fix.
-// NOTE: Public types, operationIDs and friendlyNames of versioned packages should will use the version suffix. +// NOTE: Public types, operationIDs and friendlyNames of versioned packages should use the version suffix.api/spec/src/cloud/main.tsp (1)
193-197: Consider deprecating the v1 single-customer entitlement endpointTo guide migration, flag the v1 endpoint as deprecated in the Cloud surface and reference v2.
@route("/api/v1/customers/{customerIdOrKey}/entitlements/{featureKey}") @tag("Entitlements") @tag("Customers") + #deprecated "Use /api/v2/customers/{customerIdOrKey}/entitlements/{entitlementIdOrFeatureKey} in OpenMeterCloud.Entitlements.V2." interface CustomerEntitlement extends OpenMeter.Entitlements.CustomerEntitlement {}api/spec/src/entitlements/v2/grants.tsp (1)
16-19: Doc points to v1 subjects path; update to v2 customer pathThe reference should point to the v2 customer-scoped grants endpoint (or keep it neutral) to avoid confusion.
- * To fetch the grants of a specific entitlement please use the /api/v1/subjects/{subjectKeyOrID}/entitlements/{entitlementOrFeatureID}/grants endpoint. + * To fetch the grants of a specific entitlement, use the customer-scoped endpoint: + * /api/v2/customers/{customerIdOrKey}/entitlements/{entitlementIdOrFeatureKey}/grantsapi/spec/src/entitlements/v2/entitlements.tsp (1)
33-40: Use Key type for customerKey for consistencyElsewhere keys use the
Keyalias. AligningcustomerKeyimproves consistency and any validations/formatting associated toKey.@example("customer-1") - customerKey?: string; + customerKey?: Key;api/spec/src/entitlements/v2/customer.tsp (2)
84-84: Fix typos in docsSeveral small typos; quick cleanup improves spec readability.
- * Overriding an entitlement creates a new entitlement from the provided inputs and soft deletes the previous entitlement for the provided customer-feature pair. If the previous entitlement is already deleted or otherwise doesnt exist, the override will fail. + * Overriding an entitlement creates a new entitlement from the provided inputs and soft deletes the previous entitlement for the provided customer-feature pair. If the previous entitlement is already deleted or otherwise doesn't exist, the override will fail.- * Returns historical balance and usage data for the entitlement. The queried history can span accross multiple reset events. + * Returns historical balance and usage data for the entitlement. The queried history can span across multiple reset events.- * BurndownHistory returns a continous history of segments, where the segments are seperated by events that changed either the grant burndown priority or the usage period. + * BurndownHistory returns a continuous history of segments, where the segments are separated by events that changed either the grant burndown priority or the usage period.- * Windowsize + * Window size- * Reset marks the start of a new usage period for the entitlement and initiates grant rollover. At the start of a period usage is zerod out and grants are rolled over based on their rollover settings. It would typically be synced with the customers billing period to enforce usage based on their subscription. + * Reset marks the start of a new usage period for the entitlement and initiates grant rollover. At the start of a period usage is zeroed out and grants are rolled over based on their rollover settings. It would typically be synced with the customers billing period to enforce usage based on their subscription.Also applies to: 170-170, 172-173, 197-197, 208-208
47-52: Confirm intent: mix of cursor and limit/offset paginationBoth QueryPagination and QueryLimitOffset are included. If both styles are intentionally supported concurrently, all good; otherwise, consider exposing only one to avoid ambiguous client usage.
openmeter/server/router/entitlement.go (2)
134-134: Update comments to reference /api/v2 paths for V2 methodsDocstrings still mention /api/v1; adjust to /api/v2 for accuracy.
-// (POST /api/v1/customers/{customerIdOrKey}/entitlements) +// (POST /api/v2/customers/{customerIdOrKey}/entitlements) -// (GET /api/v1/customers/{customerIdOrKey}/entitlements) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements) -// (GET /api/v1/customers/{customerIdOrKey}/entitlements/{featureKey}) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}) -// (DELETE /api/v1/customers/{customerIdOrKey}/entitlements/{featureKey}) +// (DELETE /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}) -// (PUT /api/v1/customers/{customerIdOrKey}/entitlements/{featureKey}/override) +// (PUT /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/override) -// (GET /api/v1/customers/{customerIdOrKey}/entitlements/{featureKey}/grants) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/grants) -// (POST /api/v1/customers/{customerIdOrKey}/entitlements/{featureKey}/grants) +// (POST /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/grants) -// (GET /api/v1/customers/{customerId}/entitlements/{featureKey}/value) +// (GET /api/v2/customers/{customerId}/entitlements/{featureKey}/value) -// (GET /api/v1/customers/{customerId}/entitlements/{featureKey}/history) +// (GET /api/v2/customers/{customerId}/entitlements/{featureKey}/history) -// (POST /api/v1/customers/{customerId}/entitlements/{featureKey}/reset) +// (POST /api/v2/customers/{customerId}/entitlements/{featureKey}/reset)Also applies to: 140-140, 146-146, 152-152, 158-158, 164-164, 170-170, 176-176, 185-185, 191-191
147-149: Param name clarity: consider entitlementIdOrFeatureKeySpecs accept either entitlement ID or feature key; naming the param featureKey may mislead. Consider renaming the parameter to entitlementIdOrFeatureKey for V2 methods to reflect semantics (no functional change).
Also applies to: 153-155, 159-161, 165-167, 171-173, 177-181, 186-188, 192-194
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/entitlements/customer.tsp(0 hunks)api/spec/src/entitlements/entitlements.tsp(1 hunks)api/spec/src/entitlements/v2/customer.tsp(1 hunks)api/spec/src/entitlements/v2/entitlements.tsp(1 hunks)api/spec/src/entitlements/v2/grants.tsp(1 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)api/spec/src/main.tsp(1 hunks)openmeter/server/router/credit.go(1 hunks)openmeter/server/router/entitlement.go(3 hunks)
💤 Files with no reviewable changes (1)
- api/spec/src/entitlements/customer.tsp
🔇 Additional comments (3)
api/spec/src/main.tsp (1)
22-22: Wire-up for Entitlements V2 looks correctImporting
./entitlements/v2here is appropriate and aligns with the new v2 surface.api/spec/src/cloud/main.tsp (2)
199-215: Good: Cloud surface exposes v2 customer entitlements and grantsV2 Cloud endpoints are correctly namespaced and routed.
212-215: Double-check visibility of v2 grants (internal vs public)Spec marks v2 grants as
x-internal. Confirm whether exposing/api/v2/grantson Cloud is intended for public use; if not, mirror the internal visibility or gate with auth accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
openmeter/server/router/entitlement.go (1)
114-119: Query param 'time' is ignored in v1; forward it to the handlerThe endpoint accepts api.GetCustomerEntitlementValueParams but does not forward the time filter to the driver, so the query parameter is silently dropped.
Apply this diff here to pass the time parameter:
func (a *Router) GetCustomerEntitlementValue(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string, params api.GetCustomerEntitlementValueParams) { a.customerHandler.GetCustomerEntitlementValue().With(customerdriver.GetCustomerEntitlementValueParams{ CustomerIDOrKey: customerIdOrKey, FeatureKey: featureKey, + Time: params.Time, }).ServeHTTP(w, r) }Note: customerdriver.GetCustomerEntitlementValueParams currently does not expose a field for Time. Extend it accordingly:
// openmeter/customer/httpdriver/customer.go // add: import "time" type ( GetCustomerEntitlementValueParams = struct { CustomerIDOrKey string FeatureKey string Time *time.Time } )Run this script to locate and update the type and usages:
#!/bin/bash # Locate the alias/struct for GetCustomerEntitlementValueParams and all usages. rg -n -A2 -B2 $'GetCustomerEntitlementValueParams\\s*=\\s*struct' openmeter | sed -n '1,120p' rg -n -A2 -B2 $'GetCustomerEntitlementValue\\(\\)' openmeter | sed -n '1,200p'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
openmeter/server/router/entitlement.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
openmeter/server/router/entitlement.go (5)
openmeter/server/router/router.go (1)
Router(217-242)api/api.gen.go (5)
GetCustomerEntitlementValueParams(7821-7823)ListCustomerEntitlementsV2Params(8464-8492)ListCustomerEntitlementGrantsV2Params(8495-8523)GetCustomerEntitlementValueV2Params(8542-8544)GetCustomerEntitlementHistoryV2Params(8526-8539)api/client/go/client.gen.go (5)
GetCustomerEntitlementValueParams(7085-7087)ListCustomerEntitlementsV2Params(7728-7756)ListCustomerEntitlementGrantsV2Params(7759-7787)GetCustomerEntitlementValueV2Params(7806-7808)GetCustomerEntitlementHistoryV2Params(7790-7803)openmeter/customer/httpdriver/customer.go (1)
GetCustomerEntitlementValueParams(333-336)openmeter/customer/customer.go (1)
CustomerIDOrKey(111-114)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
openmeter/server/router/entitlement.go (1)
133-194: Verify theunimplementedstubs and add the importI didn’t find any
package unimplementedor definitions for the various*CustomerEntitlementV2stubs in the repo. Please confirm that your generated OpenAPI/Swagger server stubs are present, locate their package path, and then add an import, for example:import ( "net/http" "github.com/openmeterio/openmeter/api" customerdriver "github.com/openmeterio/openmeter/openmeter/customer/httpdriver" entitlementdriver "github.com/openmeterio/openmeter/openmeter/entitlement/driver" + unimplemented "<path/to/your/generated/unimplemented/package>" )If you’re not sure where the stubs live, you can search for their definitions:
# Look for any file or directory named “unimplemented” fd -HI unimplemented # Or scan for the stub functions once you know where they should be rg -n '^func\s+CreateCustomerEntitlementV2\(' -S
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/spec/src/entitlements/entitlements.tsp (1)
271-277: Mark subjectKey as deprecated to guide migration to V2 customer APIsThe note is helpful, but marking it as deprecated will propagate clearly to OpenAPI clients and docs, aligning with the PR goal of deprecating v1 subject endpoints.
Apply this diff to deprecate the field:
/** * The identifier key unique to the subject. - * NOTE: Subjects are being deprecated, please use the new customer APIs. + * NOTE: Subjects are being deprecated, please use the new customer APIs. */ + #deprecated "Subjects are deprecated in favor of the customer-scoped APIs under /api/v2. Use the V2 customer endpoints." @example("customer-1") subjectKey: Key;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
api/spec/src/entitlements/entitlements.tsp(2 hunks)api/spec/src/entitlements/v2/entitlements.tsp(1 hunks)openmeter/entitlement/driver/parser.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/spec/src/entitlements/v2/entitlements.tsp
🧰 Additional context used
🧬 Code Graph Analysis (1)
openmeter/entitlement/driver/parser.go (5)
api/api.gen.go (2)
EntitlementBooleanType(2654-2654)EntitlementType(3200-3200)api/client/go/client.gen.go (2)
EntitlementBooleanType(2394-2394)EntitlementType(2723-2723)api/client/javascript/src/client/schemas.ts (1)
EntitlementType(10864-10864)openmeter/entitlement/entitlement.go (1)
EntitlementType(294-294)openmeter/ent/db/entitlement/entitlement.go (1)
EntitlementType(171-171)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
openmeter/entitlement/driver/parser.go (1)
108-108: Boolean Type mapping now correctly uses the generated enum aliasSwitching to
api.EntitlementBooleanType(boolean.EntitlementType)aligns boolean with metered/static mappings and the generated API types. This improves type-safety without affecting wire format (still string).api/spec/src/entitlements/entitlements.tsp (1)
361-368: Boolean entitlement model made consistent with metered/staticRefactoring
EntitlementBooleanto a standalone model that setstype: EntitlementType.booleanand pulls in fields viaOmitPropertiesmatches the pattern used by metered/static. This also enables generators to emit a dedicatedEntitlementBooleanType, which unblocks the parser change.
GAlexIHU
force-pushed
the
feat/entitlement-api-v2
branch
from
39d1e66 to
a8d83da
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
openmeter/server/router/credit.go (1)
32-34: Compile error: unresolved reference to unimplemented.ListGrantsV2; return 501 stubThis will not compile without an imported/defined unimplemented helper. Use an explicit 501 for now (or import the correct helper if it exists).
func (a *Router) ListGrantsV2(w http.ResponseWriter, r *http.Request, params api.ListGrantsV2Params) { - unimplemented.ListGrantsV2(w, r, params) + // TODO: implement V2 grants listing (align with credit handler semantics) + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) }openmeter/server/router/entitlement.go (6)
126-127: Fix V2 access route placeholder to {customerIdOrKey}.Spec and function param use customerIdOrKey; comment still shows {customerId}.
-// (GET /api/v2/customers/{customerId}/access) +// (GET /api/v2/customers/{customerIdOrKey}/access)
176-177: Fix V2 value route placeholder to {customerIdOrKey}.Comment shows {customerId}; align with spec/param.
-// (GET /api/v2/customers/{customerId}/entitlements/{featureKey}/value) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/value)
185-186: Fix V2 history route placeholder to {customerIdOrKey}.-// (GET /api/v2/customers/{customerId}/entitlements/{featureKey}/history) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/history)
191-192: Fix V2 reset route placeholder to {customerIdOrKey}.-// (POST /api/v2/customers/{customerId}/entitlements/{featureKey}/reset) +// (POST /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/reset)
112-119: Forward query params to driver (v1 value endpoint).The router drops incoming query params. Forward them to the driver so filters are respected.
func (a *Router) GetCustomerEntitlementValue(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string, params api.GetCustomerEntitlementValueParams) { a.customerHandler.GetCustomerEntitlementValue().With(customerdriver.GetCustomerEntitlementValueParams{ CustomerIDOrKey: customerIdOrKey, FeatureKey: featureKey, + Params: params, }).ServeHTTP(w, r) }
176-182: Forward the time query param to the driver (V2 value endpoint).The optional time filter is ignored. Pass it through so downstream handler can honor it.
func (a *Router) GetCustomerEntitlementValueV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string, params api.GetCustomerEntitlementValueV2Params) { a.customerHandler.GetCustomerEntitlementValue().With(customerdriver.GetCustomerEntitlementValueParams{ CustomerIDOrKey: customerIdOrKey, FeatureKey: featureKey, + Time: params.Time, }).ServeHTTP(w, r) }
🧹 Nitpick comments (3)
openmeter/server/router/credit.go (1)
26-28: Nit: fix pluralization in comment (“APIs” not “APIS”)Micro-typo in the section header.
-// V2 APIS +// V2 APIsapi/spec/src/entitlements/v2/grants.tsp (2)
16-18: Doc references v1 subject endpoint; confirm intended until v2 subject-scoped grants existIf v2 subject-scoped grants will be added soon, consider updating this note when available to avoid stale references.
41-49: Clarify precedence when both page and limit/offset are suppliedYou include both pagination styles; the doc mentions page takes precedence. Consider adding a brief note in the endpoint description to explicitly state the precedence and any constraints (e.g., disallow mixing in validation).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/entitlements/customer.tsp(0 hunks)api/spec/src/entitlements/entitlements.tsp(2 hunks)api/spec/src/entitlements/v2/customer.tsp(1 hunks)api/spec/src/entitlements/v2/entitlements.tsp(1 hunks)api/spec/src/entitlements/v2/grants.tsp(1 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)api/spec/src/main.tsp(1 hunks)openmeter/entitlement/driver/parser.go(1 hunks)openmeter/server/router/credit.go(1 hunks)openmeter/server/router/entitlement.go(1 hunks)
💤 Files with no reviewable changes (1)
- api/spec/src/entitlements/customer.tsp
🚧 Files skipped from review as they are similar to previous changes (7)
- api/spec/src/entitlements/v2/main.tsp
- api/spec/src/main.tsp
- openmeter/entitlement/driver/parser.go
- api/spec/src/entitlements/entitlements.tsp
- api/spec/src/entitlements/v2/entitlements.tsp
- api/spec/src/cloud/main.tsp
- api/spec/src/entitlements/v2/customer.tsp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Artifacts / Container image
- GitHub Check: Artifacts / Benthos Collector Container image
- GitHub Check: Test
- GitHub Check: Lint
- GitHub Check: Migration Checks
- GitHub Check: Build
- GitHub Check: Code Generators
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
api/spec/src/entitlements/v2/grants.tsp (2)
12-50: Spec structure LGTMRoute, tagging, operationId, filters, and return type align with existing V2 patterns. No blocking issues from the spec shape.
1-50: Spec wiring and types are correctly set up
All verifications passed:
grants.tspis imported inapi/spec/src/entitlements/v2/main.tspoperationId("listGrantsV2")is unique- Referenced types (
Grant,GrantOrderBy,ULIDOrExternalKey,QueryPagination,QueryLimitOffset,QueryOrdering) exist in the specNo further changes required.
openmeter/server/router/entitlement.go (1)
134-137: Confirm ‘unimplemented’ is defined in this package; no import neededThe symbol
unimplementedis declared as a package-level var inopenmeter/server/router/noop.go(line 9):package router import "github.com/openmeterio/openmeter/api" // noop.go var unimplemented api.ServerInterface = api.Unimplemented{}Since
entitlement.gois in the samerouterpackage, it can referenceunimplemented.*without importing a separate package. No changes required.
GAlexIHU
force-pushed
the
feat/entitlement-api-v2
branch
from
a8d83da to
a73e1b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
openmeter/server/router/credit.go (1)
32-34: Fix compile error: remove unresolvedunimplemented.ListGrantsV2and return 501 stub
unimplementedisn’t imported/defined here, so this won’t compile. Stub the endpoint explicitly until implemented.Apply:
func (a *Router) ListGrantsV2(w http.ResponseWriter, r *http.Request, params api.ListGrantsV2Params) { - unimplemented.ListGrantsV2(w, r, params) + // TODO: implement V2 grants listing + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) }openmeter/server/router/entitlement.go (1)
126-126: Align V2 route comments with placeholders{customerIdOrKey}Comments still reference
{customerId}; the V2 contract uses{customerIdOrKey}. Update for consistency with the spec/SDKs.Apply:
-// (GET /api/v2/customers/{customerId}/access) +// (GET /api/v2/customers/{customerIdOrKey}/access)-// (GET /api/v2/customers/{customerId}/entitlements/{featureKey}/value) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/value)-// (GET /api/v2/customers/{customerId}/entitlements/{featureKey}/history) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/history)-// (POST /api/v2/customers/{customerId}/entitlements/{featureKey}/reset) +// (POST /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/reset)Also applies to: 176-177, 185-186, 191-192
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/entitlements/customer.tsp(0 hunks)api/spec/src/entitlements/entitlements.tsp(2 hunks)api/spec/src/entitlements/v2/customer.tsp(1 hunks)api/spec/src/entitlements/v2/entitlements.tsp(1 hunks)api/spec/src/entitlements/v2/grants.tsp(1 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)api/spec/src/main.tsp(1 hunks)openmeter/entitlement/driver/parser.go(1 hunks)openmeter/server/router/credit.go(1 hunks)openmeter/server/router/entitlement.go(1 hunks)
💤 Files with no reviewable changes (1)
- api/spec/src/entitlements/customer.tsp
🚧 Files skipped from review as they are similar to previous changes (7)
- openmeter/entitlement/driver/parser.go
- api/spec/src/entitlements/v2/main.tsp
- api/spec/src/main.tsp
- api/spec/src/entitlements/v2/grants.tsp
- api/spec/src/cloud/main.tsp
- api/spec/src/entitlements/v2/customer.tsp
- api/spec/src/entitlements/v2/entitlements.tsp
🧰 Additional context used
🧬 Code Graph Analysis (2)
openmeter/server/router/credit.go (3)
openmeter/server/router/router.go (1)
Router(217-242)api/api.gen.go (1)
ListGrantsV2Params(8701-8740)api/client/go/client.gen.go (1)
ListGrantsV2Params(8059-8098)
openmeter/server/router/entitlement.go (5)
openmeter/server/router/router.go (1)
Router(217-242)api/api.gen.go (5)
GetCustomerEntitlementValueParams(7941-7943)ListCustomerEntitlementsV2Params(8584-8612)ListCustomerEntitlementGrantsV2Params(8615-8643)GetCustomerEntitlementValueV2Params(8662-8664)GetCustomerEntitlementHistoryV2Params(8646-8659)api/client/go/client.gen.go (5)
GetCustomerEntitlementValueParams(7299-7301)ListCustomerEntitlementsV2Params(7942-7970)ListCustomerEntitlementGrantsV2Params(7973-8001)GetCustomerEntitlementValueV2Params(8020-8022)GetCustomerEntitlementHistoryV2Params(8004-8017)openmeter/customer/httpdriver/customer.go (2)
GetCustomerEntitlementValueParams(333-336)GetCustomerAccessParams(384-386)openmeter/customer/customer.go (1)
CustomerIDOrKey(131-134)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Artifacts / Container image
- GitHub Check: Lint
- GitHub Check: Migration Checks
- GitHub Check: Code Generators
- GitHub Check: Test
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
api/spec/src/entitlements/entitlements.tsp (1)
272-274: Spec changes look consistent with V2 direction
- subjectKey doc tweak adds clear deprecation guidance toward customer APIs.
- EntitlementBoolean remodeled to mirror the Static/Metered pattern (own
typeplus OmitProperties), which aligns with the discriminated union.Also applies to: 361-368
GAlexIHU
force-pushed
the
feat/entitlement-api-v2
branch
from
a73e1b9 to
87f9c4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
openmeter/server/router/credit.go (1)
32-34: Fix compile error: unresolved reference to unimplemented.ListGrantsV2This will not compile unless there’s an imported/defined
unimplementedhelper. If this is a placeholder, return 501 explicitly to keep the server building and make the route behavior clear.func (a *Router) ListGrantsV2(w http.ResponseWriter, r *http.Request, params api.ListGrantsV2Params) { - unimplemented.ListGrantsV2(w, r, params) + // TODO: implement V2 grants listing (align with credit handler semantics) + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) }api/spec/src/entitlements/v2/customer.tsp (1)
94-96: Align override() param and body types with the rest of the surfaceOther endpoints use
ULIDOrKeyforentitlementIdOrFeatureKeyand fully-qualify the create input type. Keep override consistent to prevent generation mismatches.override( @path customerIdOrKey: ULIDOrExternalKey, - @path entitlementIdOrFeatureKey: ULIDOrExternalKey, - @body entitlement: EntitlementCreateInputs, + @path entitlementIdOrFeatureKey: ULIDOrKey, + @body entitlement: OpenMeter.Entitlements.EntitlementCreateInputs, ):
🧹 Nitpick comments (4)
openmeter/server/router/credit.go (2)
26-28: Nit: fix “APIS” header spellingUse “APIs” (plural, mixed case) for readability.
-// V2 APIS +// V2 APIs
30-34: Option: temporarily reuse the v1 handler to preserve behavior parityIf the v2 params are compatible (or can be mapped) you can delegate to the existing credit handler instead of returning 501. This avoids a behavioral regression while the dedicated v2 handler lands. If you want, I can propose a small adapter that translates
api.ListGrantsV2Paramsto the v1ListGrantsParams.api/spec/src/entitlements/v2/customer.tsp (2)
156-167: Optional: clarify time query parameter semanticsConsider clarifying whether
timeis evaluated as “effective at” versus “as-of end-of-period” and whether it defaults to now. This reduces ambiguity for consumers and generators.Example:
- @query(#{ explode: true }) time?: DateTime, + // Point-in-time for value evaluation; defaults to now if omitted. + @query(#{ explode: true }) time?: DateTime,
199-205: Optional: consider an enum for time zones to improve validationIf you already have a common type (e.g., IANA TZ string), reference it; otherwise, consider documenting IANA format explicitly.
- @query windowTimeZone?: string = "UTC", + // IANA time zone name (e.g., "UTC", "America/New_York") + @query windowTimeZone?: string = "UTC",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/entitlements/customer.tsp(0 hunks)api/spec/src/entitlements/entitlements.tsp(2 hunks)api/spec/src/entitlements/v2/customer.tsp(1 hunks)api/spec/src/entitlements/v2/entitlements.tsp(1 hunks)api/spec/src/entitlements/v2/grants.tsp(1 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)api/spec/src/main.tsp(1 hunks)openmeter/entitlement/driver/parser.go(1 hunks)openmeter/server/router/credit.go(1 hunks)openmeter/server/router/entitlement.go(1 hunks)
💤 Files with no reviewable changes (1)
- api/spec/src/entitlements/customer.tsp
🚧 Files skipped from review as they are similar to previous changes (8)
- api/spec/src/main.tsp
- openmeter/entitlement/driver/parser.go
- api/spec/src/entitlements/v2/main.tsp
- api/spec/src/entitlements/v2/grants.tsp
- api/spec/src/entitlements/entitlements.tsp
- api/spec/src/cloud/main.tsp
- openmeter/server/router/entitlement.go
- api/spec/src/entitlements/v2/entitlements.tsp
🧰 Additional context used
🧬 Code graph analysis (1)
openmeter/server/router/credit.go (2)
openmeter/server/router/router.go (1)
Router(217-242)api/api.gen.go (1)
ListGrantsV2Params(8716-8755)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Lint
- GitHub Check: Migration Checks
- GitHub Check: Build
- GitHub Check: Test
- GitHub Check: Code Generators
- GitHub Check: Analyze (go)
GAlexIHU
changed the title
GAlexIHU
force-pushed
the
feat/entitlement-api-v2
branch
from
87f9c4d to
726af66
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
openmeter/server/router/entitlement.go (7)
126-127: Align V2 path comment with spec: {customerIdOrKey}-// (GET /api/v2/customers/{customerId}/access) +// (GET /api/v2/customers/{customerIdOrKey}/access)
176-177: Fix V2 path placeholder: {customerIdOrKey}-// (GET /api/v2/customers/{customerId}/entitlements/{featureKey}/value) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/value)
185-186: Fix V2 path placeholder: {customerIdOrKey}-// (GET /api/v2/customers/{customerId}/entitlements/{featureKey}/history) +// (GET /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/history)
191-192: Fix V2 path placeholder: {customerIdOrKey}-// (POST /api/v2/customers/{customerId}/entitlements/{featureKey}/reset) +// (POST /api/v2/customers/{customerIdOrKey}/entitlements/{featureKey}/reset)
115-119: Forward the optional time query parameter to the driver
This dropsparams.Time, so client time-based queries are ignored. Also update the driver param struct accordingly.func (a *Router) GetCustomerEntitlementValue(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string, params api.GetCustomerEntitlementValueParams) { a.customerHandler.GetCustomerEntitlementValue().With(customerdriver.GetCustomerEntitlementValueParams{ CustomerIDOrKey: customerIdOrKey, FeatureKey: featureKey, + Time: params.Time, }).ServeHTTP(w, r) }Outside this file (driver), add support for the field:
--- a/openmeter/customer/httpdriver/customer.go +++ b/openmeter/customer/httpdriver/customer.go @@ - GetCustomerEntitlementValueParams = struct { - CustomerIDOrKey string - FeatureKey string - } + GetCustomerEntitlementValueParams = struct { + CustomerIDOrKey string + FeatureKey string + Time *time.Time + }
135-137: *Replace unresolved unimplemented.V2 calls with HTTP 501 stubs to keep build green
Theunimplementedsymbol isn’t imported/defined here, which will fail compilation. Return explicit 501s (consistent with other stubs) until implementations land.func (a *Router) CreateCustomerEntitlementV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string) { - unimplemented.CreateCustomerEntitlementV2(w, r, customerIdOrKey) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) } func (a *Router) ListCustomerEntitlementsV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, params api.ListCustomerEntitlementsV2Params) { - unimplemented.ListCustomerEntitlementsV2(w, r, customerIdOrKey, params) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) } func (a *Router) GetCustomerEntitlementV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string) { - unimplemented.GetCustomerEntitlementV2(w, r, customerIdOrKey, featureKey) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) } func (a *Router) DeleteCustomerEntitlementV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string) { - unimplemented.DeleteCustomerEntitlementV2(w, r, customerIdOrKey, featureKey) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) } func (a *Router) OverrideCustomerEntitlementV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string) { - unimplemented.OverrideCustomerEntitlementV2(w, r, customerIdOrKey, featureKey) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) } func (a *Router) ListCustomerEntitlementGrantsV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string, params api.ListCustomerEntitlementGrantsV2Params) { - unimplemented.ListCustomerEntitlementGrantsV2(w, r, customerIdOrKey, featureKey, params) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) } func (a *Router) CreateCustomerEntitlementGrantV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string) { - unimplemented.CreateCustomerEntitlementGrantV2(w, r, customerIdOrKey, featureKey) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) } func (a *Router) GetCustomerEntitlementHistoryV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string, params api.GetCustomerEntitlementHistoryV2Params) { - unimplemented.GetCustomerEntitlementHistoryV2(w, r, customerIdOrKey, featureKey, params) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) } func (a *Router) ResetCustomerEntitlementUsageV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string) { - unimplemented.ResetCustomerEntitlementUsageV2(w, r, customerIdOrKey, featureKey) + // TODO: implement V2 + http.Error(w, http.StatusText(http.StatusNotImplemented), http.StatusNotImplemented) }Also applies to: 141-143, 147-149, 153-155, 159-161, 165-167, 171-173, 187-189, 193-194
177-182: Also forward V2 time param to the driver
Same issue as v1—params.Timeis dropped.func (a *Router) GetCustomerEntitlementValueV2(w http.ResponseWriter, r *http.Request, customerIdOrKey string, featureKey string, params api.GetCustomerEntitlementValueV2Params) { a.customerHandler.GetCustomerEntitlementValue().With(customerdriver.GetCustomerEntitlementValueParams{ CustomerIDOrKey: customerIdOrKey, FeatureKey: featureKey, + Time: params.Time, }).ServeHTTP(w, r) }
🧹 Nitpick comments (3)
api/spec/src/entitlements/entitlements.tsp (2)
271-276: Mark subjectKey as deprecated to align with the migration away from subjectsYou already communicate deprecation in the description. Make it machine-readable so SDKs/docs surface it consistently.
/** * The identifier key unique to the subject. * NOTE: Subjects are being deprecated, please use the new customer APIs. */ @example("customer-1") - subjectKey: Key; + #deprecated "Subjects are being deprecated. Use customer-scoped V2 APIs instead." + subjectKey: Key;
360-368: Boolean model still inherits usagePeriod/currentUsagePeriod via SharedFieldsUnlike Metered (Lines 351–353), Boolean does not exclude
currentUsagePeriodandusagePeriodfromEntitlementSharedFields. If Boolean entitlements aren’t periodized, this leaks fields with unclear semantics. If they are periodized, consider adding read-only annotations similar to the metered calculated fields.model EntitlementBoolean { type: EntitlementType.boolean; ...OmitProperties< EntitlementBooleanCreateInputs, "type" | "metadata" | "usagePeriod" | "featureKey" | "featureId" >; - ...OmitProperties<EntitlementSharedFields, "type">; + ...OmitProperties<EntitlementSharedFields, "type" | "currentUsagePeriod" | "usagePeriod">; }Follow-up: If Boolean entitlements do need visibility into period boundaries, introduce dedicated calculated fields with
@visibility(Lifecycle.Read)as done for Metered, and document their semantics explicitly.openmeter/server/router/entitlement.go (1)
112-114: Fix path placeholder: use {customerIdOrKey} to match the handler and spec
The function parameter iscustomerIdOrKey, but the comment shows{customerId}.-// (GET /api/v1/customers/{customerId}/entitlements/{featureKey}/value) +// (GET /api/v1/customers/{customerIdOrKey}/entitlements/{featureKey}/value)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/entitlements/customer.tsp(0 hunks)api/spec/src/entitlements/entitlements.tsp(2 hunks)api/spec/src/entitlements/v2/customer.tsp(1 hunks)api/spec/src/entitlements/v2/entitlements.tsp(1 hunks)api/spec/src/entitlements/v2/grants.tsp(1 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)api/spec/src/main.tsp(1 hunks)openmeter/entitlement/driver/parser.go(1 hunks)openmeter/server/router/credit.go(1 hunks)openmeter/server/router/entitlement.go(1 hunks)
💤 Files with no reviewable changes (1)
- api/spec/src/entitlements/customer.tsp
🚧 Files skipped from review as they are similar to previous changes (8)
- api/spec/src/main.tsp
- openmeter/server/router/credit.go
- api/spec/src/entitlements/v2/main.tsp
- api/spec/src/cloud/main.tsp
- api/spec/src/entitlements/v2/grants.tsp
- openmeter/entitlement/driver/parser.go
- api/spec/src/entitlements/v2/customer.tsp
- api/spec/src/entitlements/v2/entitlements.tsp
🧰 Additional context used
🧬 Code graph analysis (1)
openmeter/server/router/entitlement.go (4)
openmeter/server/router/router.go (1)
Router(217-242)api/api.gen.go (5)
GetCustomerEntitlementValueParams(7958-7960)ListCustomerEntitlementsV2Params(8604-8632)ListCustomerEntitlementGrantsV2Params(8635-8663)GetCustomerEntitlementValueV2Params(8682-8684)GetCustomerEntitlementHistoryV2Params(8666-8679)openmeter/customer/httpdriver/customer.go (2)
GetCustomerEntitlementValueParams(333-336)GetCustomerAccessParams(384-386)openmeter/customer/customer.go (1)
CustomerIDOrKey(131-134)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Artifacts / Benthos Collector Container image
- GitHub Check: Artifacts / Container image
- GitHub Check: Migration Checks
- GitHub Check: Lint
- GitHub Check: Code Generators
- GitHub Check: Test
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
openmeter/server/router/entitlement.go (1)
112-119: Quick verification script for param forwarding and placeholder consistency
Run to confirm: (1) driver struct has Time, (2) router forwards Time for v1 and v2, (3) no leftover{customerId}placeholders in router comments.Also applies to: 177-182
3 participants
Overview
Moves CustomerEntitlement APIs under V2 and fixes new EntitlementV2 types.
spec/src/entitlements/v2:namespace OpenMeter.Entitlements.V2Summary by CodeRabbit
New Features
Breaking Changes
Documentation