refactor(entitlement): update V2 API to better conform to new standards #3401
Conversation
📝 WalkthroughWalkthroughMigrates entitlements and grants to V2 across API specs, clients, handlers, and services. Introduces annotations on grants, makes expiration optional, adds grant arrays to entitlement create/override, updates database schema and migrations, adjusts engine/time utilities for nil/open intervals, and updates tests to new types and signatures. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
GAlexIHU
force-pushed
the
refactor/entitlement-v2-touchup
branch
2 times, most recently
from
23154db to
d69bcd6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
openmeter/entitlement/entitlement_types.go (1)
22-33: Plan deprecation path and audit SubTypeConnector usagesSubTypeConnector is still referenced—add a deprecation note (or link an ADR/issue) and migrate/remove all implementers before deleting. References found: openmeter/entitlement/entitlement_types.go (definition), openmeter/entitlement/service/service.go, openmeter/entitlement/static/connector.go, openmeter/entitlement/metered/connector.go, openmeter/entitlement/boolean/connector.go, openmeter/entitlement/service/utils_test.go
openmeter/entitlement/driver/v2/customer_metered.go (1)
48-53: Bug: list handler ignores query params (order/page).Params from the router are not copied into the request. As a result, order/pagination inputs are silently dropped.
Apply this fix:
return ListCustomerEntitlementGrantsHandlerRequest{ CustomerIDOrKey: params.CustomerIDOrKey, EntitlementIdOrFeatureKey: params.EntitlementIdOrFeatureKey, Namespace: ns, + Params: params.Params, }, nilapi/spec/src/entitlements/v2/entitlements.tsp (1)
189-206: Exclude create-only fields from EntitlementMeteredV2 read modelEntitlementMeteredV2 derives from EntitlementMeteredV2CreateInputs but does not omit create-only fields ("issue", "issueAfterReset", "issueAfterResetPriority") — they will appear in API responses. Apply this diff:
model EntitlementMeteredV2 { type: EntitlementType.metered; ...OmitProperties< EntitlementMeteredV2CreateInputs, - | "type" + | "type" | "measureUsageFrom" | "metadata" | "usagePeriod" | "featureKey" | "featureId" | "currentUsagePeriod" | "grants" + | "issue" + | "issueAfterReset" + | "issueAfterResetPriority" >;Verified EntitlementBooleanV2 and EntitlementStaticV2 do not expose create-only properties through their derivations.
🧹 Nitpick comments (50)
tools/migrate/migrations/20250912135241_grant-annotations.down.sql (1)
1-4: Prefer IF EXISTS for idempotent rollbacks.Safer when re-running or partially applied migrations.
--- reverse: create index "grant_annotations" to table: "grants" -DROP INDEX "grant_annotations"; +DROP INDEX IF EXISTS "grant_annotations"; -- reverse: modify "grants" table -ALTER TABLE "grants" DROP COLUMN "annotations"; +ALTER TABLE "grants" DROP COLUMN IF EXISTS "annotations";tools/migrate/migrations/20250912135241_grant-annotations.up.sql (2)
4-7: Comment/intent mismatch: UPDATE does not “convert all values to strings.”The statement copies metadata as-is. If the intent is 1:1 copy (keep strings as strings), fix the comment. If you truly want stringification, implement it explicitly.
Option A — fix comments:
--- NOTE: metadata will be dropped later --- NOTE: technically, this might parse the values from strings to other primitive types when we later parse this. --- convert metadata to annotations: all values will be converted to strings +-- NOTE: metadata will be dropped later. +-- Copy metadata to annotations 1:1 (types preserved). Backward compatibility: existing string values remain strings.Option B — implement stringification (top-level only; nested requires a more complex transform):
-- Example approach (top-level keys only): -- UPDATE "grants" -- SET "annotations" = ( -- SELECT jsonb_object_agg(k, to_jsonb(v)) -- FROM jsonb_each_text("metadata") AS t(k, v) -- );
10-11: Operational note: consider CREATE INDEX CONCURRENTLY on large tables.GIN index creation can block writes; CONCURRENTLY avoids that but requires running outside a transaction. Adjust if your migration runner supports non-transactional steps.
openmeter/ent/db/grant/where.go (1)
345-354: Metadata is deprecated; mark these predicates as deprecated to discourage new usage.Avoid expanding surface for legacy field. If you control templates, add deprecation doc comments.
Apply this diff to annotate deprecation (or adjust codegen/templates accordingly):
-// MetadataIsNil applies the IsNil predicate on the "metadata" field. +// Deprecated: Use AnnotationsIsNil instead. MetadataIsNil applies the IsNil predicate on the "metadata" field. func MetadataIsNil() predicate.Grant { return predicate.Grant(sql.FieldIsNull(FieldMetadata)) } -// MetadataNotNil applies the NotNil predicate on the "metadata" field. +// Deprecated: Use AnnotationsNotNil instead. MetadataNotNil applies the NotNil predicate on the "metadata" field. func MetadataNotNil() predicate.Grant { return predicate.Grant(sql.FieldNotNull(FieldMetadata)) }openmeter/entitlement/service/utils_test.go (1)
77-78: Consider renaming field to avoid ambiguity with the imported package.The field name
registrycan be confused with theregistrypackage identifier. Prefer a more explicit name.Apply this diff:
type dependencies struct { @@ - registry *registry.Entitlement + entitlementRegistry *registry.Entitlement } @@ - registry: entitlementRegistry, + entitlementRegistry: entitlementRegistry,Also applies to: 168-169
openmeter/entitlement/metered/reset_test.go (5)
318-318: Fix misleading comment on rollover cap (100).Comment says “full amount” but the cap is 100.
Apply:
- ResetMaxRollover: 100, // full amount can be rolled over + ResetMaxRollover: 100, // cap rollover to 100
673-673: Fix misleading comment on rollover disabled (0).Comment says “full amount” but zero disables rollover.
Apply:
- ResetMaxRollover: 0, // full amount can be rolled over + ResetMaxRollover: 0, // no rollover
764-764: Spelling: “reseting” → “resetting”.Tighten test names.
- name: "Should reseting without anchor update keeps the next reset time intact", + name: "Should resetting without anchor update keeps the next reset time intact",- name: "Should reseting with anchor update updates the next reset time too", + name: "Should resetting with anchor update updates the next reset time too",Also applies to: 806-806
906-906: Avoid time leakage across subtests: add defer clock.ResetTime().This test sets the global clock but never resets it; add a defer for isolation.
ctx := context.Background() + defer clock.ResetTime()
54-54: Use clock.Now() instead of time.Now() for determinism.Aligns with clock.SetTime used in later tests.
- currentUsagePeriod, err := input.UsagePeriod.GetValue().GetPeriodAt(time.Now()) // This should be calculated properly when testing batch resets + currentUsagePeriod, err := input.UsagePeriod.GetValue().GetPeriodAt(clock.Now()) // This should be calculated properly when testing batch resetsopenmeter/credit/grant/events_2.go (1)
33-40: Make optional fields omit empty in JSONWithout omitempty on pointer fields, events will emit nulls. Align with other optional fields (e.g., recurrence, voidedAt).
- Expiration *ExpirationPeriod `json:"expiration"` - ExpiresAt *time.Time `json:"expiresAt"` + Expiration *ExpirationPeriod `json:"expiration,omitempty"` + ExpiresAt *time.Time `json:"expiresAt,omitempty"`pkg/timeutil/boundedperiod.go (1)
5-56: Add brief doc comment on StartBoundedPeriod semanticsAdd a package comment describing inclusive/exclusive semantics and that To may be nil (open-ended).
pkg/timeutil/openperiod_test.go (1)
769-872: Add zero‑length overlap cases to catch containment bugCover [t,t] inside a larger period (and vice versa) to validate OverlapsInclusive behavior.
@@ t.Run("Should be in parity with ClosedPeriod.OverlapsInclusive for convertible periods", func(t *testing.T) { tests := []struct { name string period1 ClosedPeriod period2 ClosedPeriod }{ + { + name: "Zero-length inside other", + period1: ClosedPeriod{From: now, To: now}, + period2: ClosedPeriod{From: before, To: after}, + }, + { + name: "Zero-length equals other endpoint", + period1: ClosedPeriod{From: now, To: now}, + period2: ClosedPeriod{From: before, To: now}, + }, } @@ t.Run("Should work for open periods", func(t *testing.T) { tests := []struct { name string period1 OpenPeriod period2 OpenPeriod expected bool }{ + { + name: "Point inside", + period1: OpenPeriod{From: &now, To: &now}, + period2: OpenPeriod{From: &before, To: &after}, + expected: true, + }, + { + name: "Point equals endpoint", + period1: OpenPeriod{From: &now, To: &now}, + period2: OpenPeriod{From: &before, To: &now}, + expected: true, + }, }openmeter/ent/db/grant_update.go (4)
70-80: Deprecate Metadata setters to steer callers to Annotations.Add deprecation doc comments so new code prefers Annotations.
-// SetMetadata sets the "metadata" field. +// Deprecated: Use SetAnnotations instead. SetMetadata sets the "metadata" field. func (_u *GrantUpdate) SetMetadata(v map[string]string) *GrantUpdate { _u.mutation.SetMetadata(v) return _u } -// ClearMetadata clears the value of the "metadata" field. +// Deprecated: Use ClearAnnotations instead. ClearMetadata clears the value of the "metadata" field. func (_u *GrantUpdate) ClearMetadata() *GrantUpdate { _u.mutation.ClearMetadata() return _u }
178-189: Avoid ambiguous dual writes to Annotations and Metadata in one mutation.If both are set/cleared together, downstream readers might diverge. Consider enforcing mutual exclusivity in builder check() or define precedence.
As a follow‑up, add a guard to check():
func (_u *GrantUpdate) check() error { if _u.mutation.EntitlementCleared() && len(_u.mutation.EntitlementIDs()) > 0 { return errors.New(`db: clearing a required unique edge "Grant.entitlement"`) } if _, aSet := _u.mutation.Annotations(); aSet { if _, mSet := _u.mutation.Metadata(); mSet { return errors.New(`db: set both "annotations" and "metadata" in the same update; prefer annotations`) } } if _u.mutation.AnnotationsCleared() && _u.mutation.MetadataCleared() { return errors.New(`db: cleared both "annotations" and "metadata" in the same update; prefer annotations`) } return nil }
260-271: GrantUpdateOne: deprecate Metadata setters similarly.-// SetMetadata sets the "metadata" field. +// Deprecated: Use SetAnnotations instead. SetMetadata sets the "metadata" field. func (_u *GrantUpdateOne) SetMetadata(v map[string]string) *GrantUpdateOne { _u.mutation.SetMetadata(v) return _u } -// ClearMetadata clears the value of the "metadata" field. +// Deprecated: Use ClearAnnotations instead. ClearMetadata clears the value of the "metadata" field. func (_u *GrantUpdateOne) ClearMetadata() *GrantUpdateOne { _u.mutation.ClearMetadata() return _u }
398-409: GrantUpdateOne: guard against simultaneous Metadata and Annotations updates.Same concern as the multi‑update path; add a check to avoid dual writes in a single mutation.
api/spec/src/entitlements/v2/grants.tsp (1)
45-49: Align pagination docs, query params, and return type.This endpoint returns PaginatedResponse but still exposes both page-based and limit/offset query shapes and mentions “page takes precedence.” Please confirm intended behavior:
- If only page-based pagination is supported, drop QueryLimitOffset and update the comment.
- If both are supported, consider documenting the non-page response or clarify that both shapes influence the same paginated result.
openmeter/entitlement/entitlement_grant.go (1)
5-7: Add a brief doc comment and confirm single canonical type.
- Add a GoDoc comment for CreateEntitlementGrantInputs.
- Ensure metered package re-exports this (type alias) to avoid divergence between packages.
openmeter/entitlement/driver/v2/customer_metered.go (1)
85-87: Avoid shadowing the imported grant package in the mapper.Rename the lambda param for clarity.
- mapped := pagination.MapResult(grants, func(grant meteredentitlement.EntitlementGrant) api.EntitlementGrantV2 { - return MapEntitlementGrantToAPIV2(&grant) + mapped := pagination.MapResult(grants, func(g meteredentitlement.EntitlementGrant) api.EntitlementGrantV2 { + return MapEntitlementGrantToAPIV2(&g) })openmeter/credit/adapter/grant.go (1)
239-242: Normalize ExpiresAt to UTC (minor consistency)VoidedAt is normalized to UTC/truncated; ExpiresAt is passed through as-is. Consider consistent normalization to avoid TZ drift.
- ExpiresAt: entity.ExpiresAt, + ExpiresAt: convert.SafeDeRef(entity.ExpiresAt, func(t time.Time) *time.Time { + return convert.ToPointer(t.In(time.UTC).Truncate(time.Minute)) + }),openmeter/credit/driver/grant.go (1)
301-306: Ensure non-zero page and pageSize in response when using limit/offsetWhen page is unset,
grants.Page.PageNumber/PageSizecan be zero. Fill sane defaults for API ergonomics.- return api.GrantV2PaginatedResponse{ - Items: apiGrants, - TotalCount: grants.TotalCount, - Page: grants.Page.PageNumber, - PageSize: grants.Page.PageSize, - }, nil + // Guarantee non-zero values if pagination was provided as limit/offset + return api.GrantV2PaginatedResponse{ + Items: apiGrants, + TotalCount: grants.TotalCount, + Page: func() int { + if grants.Page.PageNumber == 0 { + return 1 + } + return grants.Page.PageNumber + }(), + PageSize: func() int { + if grants.Page.PageSize == 0 { + if request.params.Limit > 0 { + return request.params.Limit + } + return 100 + } + return grants.Page.PageSize + }(), + }, nilapi/spec/src/entitlements/v2/grant.tsp (1)
31-36: Nit: fix typo “recurr” → “recur” in docstringKeeps docs polished.
- /** - * The next time the grant will recurr. - */ + /** + * The next time the grant will recur. + */openmeter/entitlement/driver/metered.go (3)
106-112: Guard optional Expiration (V1 input is required, but be defensive)If the input type ever becomes optional, this will panic. Small guard improves resilience.
- Expiration: &grant.ExpirationPeriod{ - Count: apiGrant.Expiration.Count, - Duration: grant.ExpirationPeriodDuration(apiGrant.Expiration.Duration), - }, + Expiration: func() *grant.ExpirationPeriod { + if apiGrant.Expiration == nil { + return nil + } + return &grant.ExpirationPeriod{ + Count: apiGrant.Expiration.Count, + Duration: grant.ExpirationPeriodDuration(apiGrant.Expiration.Duration), + } + }(),
110-112: Default max rollover to Amount when omittedAligns with PR objective: default maxRolloverAmount should be Amount.
- ResetMaxRollover: defaultx.WithDefault(apiGrant.MaxRolloverAmount, 0), + ResetMaxRollover: defaultx.WithDefault(apiGrant.MaxRolloverAmount, apiGrant.Amount),
411-433: Use a deterministic far-future ExpiresAt for V1 hackUsing now()+100y yields non-deterministic responses. Prefer a fixed far-future timestamp.
- ExpiresAt: lo.ToPtr(lo.FromPtrOr(grant.ExpiresAt, clock.Now().AddDate(100, 0, 0))), // V1 API expects all grants to have an expiresAt so we'll artificially set a very far future date. This is a hack that our users were already doing to get this behavior and we will sunset it later... + ExpiresAt: func() *time.Time { + if grant.ExpiresAt != nil { + return grant.ExpiresAt + } + t := time.Date(9999, time.December, 31, 23, 59, 59, 0, time.UTC) + return &t + }(), // V1 API expects expiresAt; use a deterministic far-future value.e2e/entitlement_parity_test.go (1)
197-199: Prefer typed enum/cast for Expiration.Duration to avoid type drift.If ExpirationDuration is a named string type (not an alias), assigning a plain string can break compilation depending on codegen. Cast explicitly (or use the enum constant if available).
Apply this diff:
- Expiration: &api.ExpirationPeriod{Duration: "MONTH", Count: 1}, + Expiration: &api.ExpirationPeriod{Duration: api.ExpirationDuration("MONTH"), Count: 1},And:
- Expiration: &api.ExpirationPeriod{Duration: "MONTH", Count: 1}, + Expiration: &api.ExpirationPeriod{Duration: api.ExpirationDuration("MONTH"), Count: 1},Also applies to: 205-209
openmeter/entitlement/driver/v2/mapping.go (1)
199-215: Avoid emitting annotations: null when empty.Use EmptyableToPtr to omit the field entirely when there are no annotations, matching other mappers and cleaner JSON.
Apply this diff:
- Annotations: lo.ToPtr(api.Annotations(grant.Annotations)), + Annotations: lo.EmptyableToPtr(api.Annotations(grant.Annotations)),api/spec/src/entitlements/v2/customer.tsp (1)
90-92: Parameter type inconsistency: ULIDOrExternalKey vs ULIDOrKey.Other operations use ULIDOrKey for entitlementIdOrFeatureKey. Align override for consistency and generator stability.
Apply this diff:
- @path entitlementIdOrFeatureKey: ULIDOrExternalKey, + @path entitlementIdOrFeatureKey: ULIDOrKey,openmeter/ent/db/migrate/schema.go (1)
1573-1589: Grants: annotations added + optional expirations — verify coexistence with metadataBoth annotations (jsonb) and metadata (jsonb) are present on grants. The PR goal states “rename metadata to annotations.” Keeping both can lead to drift if writers/readers aren’t aligned. Please confirm:
- Which field is the source of truth going forward.
- Backfill/cleanup plan (e.g., migrate metadata→annotations, gate writes to metadata, or drop metadata later).
Also verify callers tolerate nullable expiration/expires_at.openmeter/entitlement/driver/v2/customer.go (6)
25-28: Unify Grants type to entitlement.CreateEntitlementGrantInputs; drop metered importUse the canonical type to avoid importing the metered alias just for typing.
-import ( +import ( "context" "errors" "fmt" "net/http" "github.com/samber/lo" "github.com/openmeterio/openmeter/api" "github.com/openmeterio/openmeter/openmeter/customer" "github.com/openmeterio/openmeter/openmeter/entitlement" - meteredentitlement "github.com/openmeterio/openmeter/openmeter/entitlement/metered" "github.com/openmeterio/openmeter/pkg/clock" "github.com/openmeterio/openmeter/pkg/defaultx" "github.com/openmeterio/openmeter/pkg/framework/commonhttp" "github.com/openmeterio/openmeter/pkg/framework/transport/httptransport" "github.com/openmeterio/openmeter/pkg/models" "github.com/openmeterio/openmeter/pkg/pagination" "github.com/openmeterio/openmeter/pkg/strcase" ) @@ type ( - CreateCustomerEntitlementHandlerRequest = struct { + CreateCustomerEntitlementHandlerRequest = struct { Entitlement entitlement.CreateEntitlementInputs - Grants []meteredentitlement.CreateEntitlementGrantInputs + Grants []entitlement.CreateEntitlementGrantInputs } )
41-44: Avoid unnecessary allocation; decode into a nil pointerMinor tidy-up: let the decoder allocate the struct.
- inp := &api.EntitlementV2CreateInputs{} + var inp *api.EntitlementV2CreateInputs request := CreateCustomerEntitlementHandlerRequest{} if err := commonhttp.JSONRequestBodyDecoder(r, &inp); err != nil { return request, err }
70-72: Parse v2 inputs and (optionally) enforce one-of (issueAfterReset xor grants)The comment says “Reuse v1 parser” but the code uses v2. Also, PR requires exactly one of issueAfterReset or grants. Mapping currently errors when both are set but allows neither. Consider enforcing the one-of here to reflect the API contract.
- // Reuse v1 parser to build entitlement create inputs using the subject key - return ParseAPICreateInputV2(inp, ns, cus.GetUsageAttribution()) + // Parse v2 inputs to build entitlement create inputs using the subject key + req, err := ParseAPICreateInputV2(inp, ns, cus.GetUsageAttribution()) + if err != nil { + return request, err + } + // Optional: enforce exactly-one-of per API contract + if (req.Entitlement.IssueAfterReset == nil) == (len(req.Grants) == 0) { + return request, commonhttp.NewHTTPError(http.StatusBadRequest, errors.New("exactly one of issueAfterReset or grants must be provided")) + } + return req, nil
317-329: Align Grants type with create pathUse entitlement.CreateEntitlementGrantInputs here as well (same canonical type). This also keeps the file import set minimal.
OverrideCustomerEntitlementHandlerRequest struct { CustomerID string EntitlementIDOrFeatureKey string Namespace string Inputs entitlement.CreateEntitlementInputs - Grants []entitlement.CreateEntitlementGrantInputs + Grants []entitlement.CreateEntitlementGrantInputs }
344-346: Same decode tidy-up hereConsistent with create handler.
- apiInp := &api.EntitlementV2CreateInputs{} + var apiInp *api.EntitlementV2CreateInputs if err := commonhttp.JSONRequestBodyDecoder(r, &apiInp); err != nil { return OverrideCustomerEntitlementHandlerRequest{}, err }
366-378: Update stale comment and consider one-of enforcementMirror the create-path validation to ensure exactly one-of (issueAfterReset xor grants) before override.
- // Reuse v1 parser to build entitlement create inputs using the subject key + // Parse v2 inputs to build entitlement create inputs using the subject key createInp, err := ParseAPICreateInputV2(apiInp, ns, cus.GetUsageAttribution()) if err != nil { return OverrideCustomerEntitlementHandlerRequest{}, err } + // Optional: enforce exactly-one-of per API contract + if (createInp.Entitlement.IssueAfterReset == nil) == (len(createInp.Grants) == 0) { + return OverrideCustomerEntitlementHandlerRequest{}, commonhttp.NewHTTPError(http.StatusBadRequest, errors.New("exactly one of issueAfterReset or grants must be provided")) + }api/spec/src/entitlements/v2/entitlements.tsp (2)
119-139: Doc nit: deprecation text casing.Deprecation text points to
issue.Amount/issue.Priority, but the fields areissue.amount/issue.priority. Align the case to avoid confusion.Apply:
-#deprecated "Use issue.Amount instead, will be removed in next major version." +#deprecated "Use issue.amount instead; will be removed in the next major version." ... -#deprecated "Use issue.Priority instead, will be removed in next major version." +#deprecated "Use issue.priority instead; will be removed in the next major version."
140-182: Missing exclusivity: require exactly one ofissueorgrants.Spec doesn’t encode the “exactly one of” constraint between
issueandgrants. If you don’t enforce this here, clients will send ambiguous payloads.
- Option A (preferred if acceptable in TypeSpec): model the mutually-exclusive shape via an intersection with a union, e.g. a base model intersected with
({ issue: IssueAfterReset; grants?: never } | { issue?: never; grants: GrantCreateInputV2[] }).- Option B: add explicit doc summaries on both fields and enforce in validators (Zod) and server. Please confirm validation exists in the JS schema and server drivers.
openmeter/entitlement/service/service.go (4)
72-76: Use consistent validation error typing.This returns a plain fmt.Errorf; elsewhere (Override) you wrap in
models.NewGenericValidationError. Use the same error type for consistent API responses.Apply:
- if input.ActiveTo != nil || input.ActiveFrom != nil { - return nil, fmt.Errorf("activeTo and activeFrom are not supported in CreateEntitlement") - } + if input.ActiveTo != nil || input.ActiveFrom != nil { + return nil, models.NewGenericValidationError(fmt.Errorf("activeTo and activeFrom are not supported in CreateEntitlement")) + }
78-81: Guard grants to metered type early—ok; add defensive nil-check on connector.Type check is correct. Add a defensive check to avoid nil deref if the metered connector wasn’t wired.
Apply:
- if len(grants) > 0 && input.EntitlementType != entitlement.EntitlementTypeMetered { + if len(grants) > 0 && input.EntitlementType != entitlement.EntitlementTypeMetered { return nil, entitlement.ErrEntitlementGrantsOnlySupportedForMeteredEntitlements.WithAttr("entitlement_type", input.EntitlementType).WithAttr("grants", grants) } + if len(grants) > 0 && c.meteredEntitlementConnector == nil { + return nil, models.NewGenericValidationError(fmt.Errorf("service misconfigured: metered entitlement connector is not set")) + }Repeat the same defensive check in OverrideEntitlement before creating grants.
Also applies to: 106-109
87-95: Grant creation loop—ok; consider batching or parallelism if grant counts can be high.If clients may pass many grants, consider a simple bounded semaphore to parallelize or a batch create on the connector. Not blocking.
Also applies to: 133-141
124-126: Consistent messaging.Use the same wording as in CreateEntitlement and the same error wrapper type for “ActiveFrom/ActiveTo not supported” to keep API errors uniform.
- return nil, models.NewGenericValidationError(fmt.Errorf("the ActiveFrom and ActiveTo are not supported in OverrideEntitlement")) + return nil, models.NewGenericValidationError(fmt.Errorf("activeFrom and activeTo are not supported in OverrideEntitlement"))api/client/javascript/src/client/schemas.ts (3)
4781-4783: Fix incorrect description for recurrence (CreateInput).This should say “recurrence period of the grant,” not “subject.”
Apply this diff (source of truth should be OpenAPI/TypeSpec, then regenerate):
- /** @description The subject of the grant. */ + /** @description The recurrence period of the grant. */ recurrence?: components['schemas']['RecurringPeriodCreateInput']
4897-4901: Typo: “recurr” → “recur”.Minor copy fix in the JSDoc.
Apply this diff (update in OpenAPI/TypeSpec, then regenerate):
- * @description The next time the grant will recurr. + * @description The next time the grant will recur.
5226-5291: Model XOR constraints in the schema (featureKey vs featureId; issue vs grants).V2 requires exactly one of issue or grants, and either featureKey or featureId. Express these with oneOf in the OpenAPI/TypeSpec so generators and validators enforce it.
If you’re using OpenAPI, consider:
EntitlementMeteredV2CreateInputs: type: object required: [type, usagePeriod] properties: type: type: string enum: [metered] featureKey: type: string featureId: type: string issue: $ref: '#/components/schemas/IssueAfterReset' grants: type: array items: { $ref: '#/components/schemas/EntitlementGrantCreateInputV2' } oneOf: - required: [featureKey] - required: [featureId] allOf: - oneOf: - required: [issue] not: { required: [grants] } - required: [grants] not: { required: [issue] }Then regenerate this client.
openmeter/ent/db/mutation.go (3)
36342-36348: Fields() capacity likely stalePrealloc of 17 may now be low with added fields. Either bump or drop the capacity literal to avoid drift.
Apply one of:
- fields := make([]string, 0, 17) + fields := make([]string, 0)—or update to the accurate count if known.
36495-36501: Accept plain map[string]interface{} for SetField(annotations)Type assertion to models.Annotations will fail for plain map[string]interface{}. Accept both for better ergonomics and parity with metadata.
Apply:
- case dbgrant.FieldAnnotations: - v, ok := value.(models.Annotations) - if !ok { - return fmt.Errorf("unexpected type %T for field %s", value, name) - } - m.SetAnnotations(v) + case dbgrant.FieldAnnotations: + switch vv := value.(type) { + case models.Annotations: + m.SetAnnotations(vv) + case map[string]interface{}: + m.SetAnnotations(models.Annotations(vv)) + default: + return fmt.Errorf("unexpected type %T for field %s", value, name) + } + return nil
36523-36529: Optionally accept models.Metadata in SetField(metadata)Not required, but symmetrical handling improves DX.
- case dbgrant.FieldMetadata: - v, ok := value.(map[string]string) - if !ok { - return fmt.Errorf("unexpected type %T for field %s", value, name) - } - m.SetMetadata(v) - return nil + case dbgrant.FieldMetadata: + switch vv := value.(type) { + case map[string]string: + m.SetMetadata(vv) + case models.Metadata: + m.SetMetadata(map[string]string(vv)) + default: + return fmt.Errorf("unexpected type %T for field %s", value, name) + } + return nilapi/openapi.yaml (2)
16566-16575: Doc nit: reference the actual property name isSoftLimitKeep terminology consistent with the property.
- description: If softLimit=true the subject can use the feature even if the entitlement is exhausted, hasAccess will always be true. + description: If isSoftLimit=true the subject can use the feature even if the entitlement is exhausted; hasAccess will always be true.
17804-17833: Pagination: add minimum constraints for page and pageSizePrevents invalid zero/negative values.
page: type: integer description: The page index. example: 1 + minimum: 1 pageSize: type: integer description: The maximum number of items per page. example: 100 + minimum: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (61)
api/client/javascript/src/client/schemas.ts(17 hunks)api/client/javascript/src/zod/index.ts(8 hunks)api/openapi.cloud.yaml(13 hunks)api/openapi.yaml(13 hunks)api/spec/src/entitlements/v2/customer.tsp(4 hunks)api/spec/src/entitlements/v2/entitlements.tsp(4 hunks)api/spec/src/entitlements/v2/grant.tsp(1 hunks)api/spec/src/entitlements/v2/grants.tsp(1 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)e2e/entitlement_parity_test.go(7 hunks)e2e/entitlement_test.go(6 hunks)openmeter/credit/adapter/grant.go(3 hunks)openmeter/credit/driver/grant.go(3 hunks)openmeter/credit/engine/engine_test.go(4 hunks)openmeter/credit/engine/grant.go(5 hunks)openmeter/credit/engine/reset_test.go(1 hunks)openmeter/credit/engine/run_test.go(9 hunks)openmeter/credit/grant.go(3 hunks)openmeter/credit/grant/events_2.go(4 hunks)openmeter/credit/grant/grant.go(3 hunks)openmeter/credit/grant/grant_test.go(11 hunks)openmeter/credit/grant/repo.go(1 hunks)openmeter/ent/db/grant.go(10 hunks)openmeter/ent/db/grant/grant.go(3 hunks)openmeter/ent/db/grant/where.go(4 hunks)openmeter/ent/db/grant_create.go(14 hunks)openmeter/ent/db/grant_update.go(9 hunks)openmeter/ent/db/migrate/schema.go(3 hunks)openmeter/ent/db/mutation.go(19 hunks)openmeter/ent/db/runtime.go(1 hunks)openmeter/ent/db/setorclear.go(2 hunks)openmeter/ent/schema/grant.go(2 hunks)openmeter/entitlement/connector.go(1 hunks)openmeter/entitlement/driver/entitlement.go(2 hunks)openmeter/entitlement/driver/metered.go(5 hunks)openmeter/entitlement/driver/v2/customer.go(9 hunks)openmeter/entitlement/driver/v2/customer_metered.go(6 hunks)openmeter/entitlement/driver/v2/mapping.go(3 hunks)openmeter/entitlement/entitlement_grant.go(1 hunks)openmeter/entitlement/entitlement_types.go(1 hunks)openmeter/entitlement/errors.go(1 hunks)openmeter/entitlement/metered/balance_test.go(17 hunks)openmeter/entitlement/metered/connector.go(1 hunks)openmeter/entitlement/metered/entitlement_grant.go(2 hunks)openmeter/entitlement/metered/grant_owner_adapter_test.go(1 hunks)openmeter/entitlement/metered/lateevents_test.go(1 hunks)openmeter/entitlement/metered/reset_test.go(13 hunks)openmeter/entitlement/service/access_test.go(6 hunks)openmeter/entitlement/service/scheduling_test.go(3 hunks)openmeter/entitlement/service/service.go(4 hunks)openmeter/entitlement/service/utils_test.go(3 hunks)openmeter/server/server_test.go(1 hunks)pkg/timeutil/boundedperiod.go(1 hunks)pkg/timeutil/openperiod.go(1 hunks)pkg/timeutil/openperiod_test.go(1 hunks)test/customer/customer.go(1 hunks)test/entitlement/regression/scenario_test.go(13 hunks)tools/migrate/migrations/20250912135241_grant-annotations.down.sql(1 hunks)tools/migrate/migrations/20250912135241_grant-annotations.up.sql(1 hunks)tools/migrate/migrations/20250922104242_grant-expiration-optional.down.sql(1 hunks)tools/migrate/migrations/20250922104242_grant-expiration-optional.up.sql(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-07T12:17:43.129Z
Learnt from: GAlexIHU
PR: openmeterio/openmeter#2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may use variables like `meterSlug` and `namespace` without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
Applied to files:
openmeter/entitlement/service/service.goopenmeter/entitlement/service/scheduling_test.go
🧬 Code graph analysis (43)
openmeter/entitlement/metered/grant_owner_adapter_test.go (1)
openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
openmeter/entitlement/metered/entitlement_grant.go (4)
api/client/javascript/src/client/schemas.ts (1)
Annotations(11341-11341)api/api.gen.go (1)
Annotations(1036-1036)api/client/go/client.gen.go (1)
Annotations(951-951)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)
openmeter/entitlement/errors.go (2)
pkg/models/errors.go (1)
ErrorCode(294-294)pkg/models/validationissue.go (1)
NewValidationIssue(219-230)
openmeter/credit/grant/repo.go (4)
api/client/javascript/src/client/schemas.ts (2)
ExpirationPeriod(11563-11563)Annotations(11341-11341)api/api.gen.go (2)
ExpirationPeriod(3421-3427)Annotations(1036-1036)api/client/go/client.gen.go (2)
ExpirationPeriod(3135-3141)Annotations(951-951)openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
openmeter/credit/engine/engine_test.go (3)
api/api.gen.go (1)
ExpirationPeriod(3421-3427)api/client/go/client.gen.go (1)
ExpirationPeriod(3135-3141)openmeter/credit/grant/expiration.go (1)
ExpirationPeriod(6-12)
openmeter/credit/engine/reset_test.go (2)
api/client/javascript/src/client/schemas.ts (1)
ExpirationPeriod(11563-11563)api/api.gen.go (1)
ExpirationPeriod(3421-3427)
openmeter/entitlement/metered/reset_test.go (1)
openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
openmeter/entitlement/service/utils_test.go (2)
openmeter/registry/entitlement.go (1)
Entitlement(11-21)app/common/entitlement.go (1)
Entitlement(19-21)
openmeter/credit/grant.go (3)
api/api.gen.go (2)
ExpirationPeriod(3421-3427)Annotations(1036-1036)openmeter/credit/grant/repo.go (1)
RepoCreateInput(57-69)pkg/timeutil/recurrence.go (1)
Recurrence(13-18)
openmeter/entitlement/metered/lateevents_test.go (1)
openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
openmeter/ent/db/grant.go (3)
api/api.gen.go (3)
Annotations(1036-1036)Metadata(4945-4945)ExpirationPeriod(3421-3427)api/client/go/client.gen.go (3)
Annotations(951-951)Metadata(4478-4478)ExpirationPeriod(3135-3141)openmeter/ent/db/grant/grant.go (3)
FieldAnnotations(20-20)FieldMetadata(28-28)FieldExpiration(38-38)
openmeter/server/server_test.go (2)
openmeter/entitlement/entitlement.go (1)
CreateEntitlementInputs(78-103)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)
openmeter/entitlement/driver/v2/mapping.go (9)
pkg/timeutil/recurrence.go (1)
Recurrence(13-18)openmeter/entitlement/driver/parser.go (2)
MapRecurrenceToAPI(363-387)MapAPIPeriodIntervalToRecurrence(341-361)openmeter/entitlement/driver/v2/customer.go (1)
CreateCustomerEntitlementHandlerRequest(25-28)openmeter/entitlement/entitlement.go (2)
CreateEntitlementInputs(78-103)MeasureUsageFromInput(42-44)pkg/timeutil/timeline.go (1)
AsTimed(21-28)pkg/clock/clock.go (1)
Now(14-21)pkg/slicesx/map.go (1)
MapWithErr(25-49)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)openmeter/credit/grant.go (1)
CreateGrantInput(25-34)
openmeter/ent/schema/grant.go (4)
pkg/framework/entutils/mixins.go (5)
AnnotationsMixin(157-159)AnnotationsMixin(162-170)AnnotationsMixin(172-181)TimeMixin(184-186)TimeMixin(189-201)openmeter/credit/grant/grant.go (1)
Grant(14-58)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)api/api.gen.go (1)
ExpirationPeriod(3421-3427)
openmeter/ent/db/grant/grant.go (1)
openmeter/ent/db/entitlement/entitlement.go (2)
FieldAnnotations(67-67)FieldMetadata(23-23)
openmeter/entitlement/entitlement_grant.go (1)
openmeter/credit/grant.go (1)
CreateGrantInput(25-34)
pkg/timeutil/openperiod_test.go (2)
pkg/timeutil/closedperiod.go (1)
ClosedPeriod(8-11)pkg/timeutil/openperiod.go (1)
OpenPeriod(8-11)
openmeter/ent/db/grant/where.go (4)
openmeter/ent/db/entitlement/where.go (4)
AnnotationsIsNil(1265-1267)AnnotationsNotNil(1270-1272)MetadataIsNil(236-238)MetadataNotNil(241-243)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)openmeter/ent/schema/grant.go (5)
Grant(15-17)Grant(19-26)Grant(28-59)Grant(61-66)Grant(68-77)openmeter/ent/db/grant/grant.go (4)
FieldAnnotations(20-20)FieldMetadata(28-28)FieldExpiration(38-38)FieldExpiresAt(40-40)
openmeter/credit/grant/events_2.go (4)
api/client/javascript/src/client/schemas.ts (3)
ExpirationPeriod(11563-11563)Metadata(11683-11683)Annotations(11341-11341)api/api.gen.go (3)
ExpirationPeriod(3421-3427)Metadata(4945-4945)Annotations(1036-1036)api/client/go/client.gen.go (3)
ExpirationPeriod(3135-3141)Metadata(4478-4478)Annotations(951-951)pkg/timeutil/recurrence.go (1)
Recurrence(13-18)
api/client/javascript/src/client/schemas.ts (2)
api/api.gen.go (8)
EntitlementGrantCreateInputV2(2867-2902)EntitlementGrantV2(2905-2964)EntitlementMetered(2968-3034)EntitlementMeteredV2(3099-3168)EntitlementMeteredV2CreateInputs(3174-3215)EntitlementV2CreateInputs(3374-3376)GrantV2PaginatedResponse(3733-3745)IssueAfterReset(4835-4841)openmeter/ent/db/entitlement/where.go (1)
IssueAfterReset(130-132)
openmeter/entitlement/metered/connector.go (4)
api/client/javascript/src/client/schemas.ts (1)
Annotations(11341-11341)api/api.gen.go (1)
Annotations(1036-1036)api/client/go/client.gen.go (1)
Annotations(951-951)openmeter/entitlement/metered/entitlement.go (1)
IssueAfterResetMetaTag(13-13)
openmeter/credit/grant/grant.go (4)
api/api.gen.go (2)
ExpirationPeriod(3421-3427)Annotations(1036-1036)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)openmeter/ent/schema/grant.go (5)
Grant(15-17)Grant(19-26)Grant(28-59)Grant(61-66)Grant(68-77)pkg/timeutil/boundedperiod.go (1)
StartBoundedPeriod(5-8)
openmeter/credit/grant/grant_test.go (4)
openmeter/ent/db/grant/where.go (4)
ExpiresAt(110-112)EffectiveAt(105-107)DeletedAt(85-87)VoidedAt(115-117)openmeter/credit/grant/grant.go (1)
Grant(14-58)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)pkg/models/model.go (1)
ManagedModel(119-125)
openmeter/entitlement/driver/metered.go (4)
api/api.gen.go (4)
ExpirationPeriod(3421-3427)Annotations(1036-1036)Metadata(4945-4945)ExpirationDuration(3418-3418)openmeter/credit/grant/expiration.go (1)
ExpirationPeriod(6-12)pkg/convert/ptr.go (1)
ToPointer(10-12)pkg/clock/clock.go (1)
Now(14-21)
openmeter/credit/driver/grant.go (4)
api/api.gen.go (2)
GrantV2PaginatedResponse(3733-3745)EntitlementGrantV2(2905-2964)openmeter/entitlement/metered/entitlement_grant.go (1)
GrantFromCreditGrant(146-160)pkg/clock/clock.go (1)
Now(14-21)openmeter/entitlement/driver/v2/mapping.go (1)
MapEntitlementGrantToAPIV2(199-233)
openmeter/entitlement/driver/v2/customer.go (4)
openmeter/entitlement/entitlement.go (1)
CreateEntitlementInputs(78-103)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)openmeter/entitlement/metered/entitlement_grant.go (1)
CreateEntitlementGrantInputs(162-162)openmeter/entitlement/driver/v2/mapping.go (1)
ParseAPICreateInputV2(235-390)
openmeter/credit/adapter/grant.go (4)
openmeter/ent/db/grant/where.go (6)
ExpiresAt(110-112)And(898-900)EffectiveAtLT(531-533)Or(903-905)ExpiresAtGT(571-573)ExpiresAtIsNil(591-593)api/client/javascript/src/client/schemas.ts (1)
Annotations(11341-11341)api/api.gen.go (1)
Annotations(1036-1036)api/client/go/client.gen.go (1)
Annotations(951-951)
openmeter/ent/db/grant_update.go (3)
api/api.gen.go (2)
Annotations(1036-1036)Metadata(4945-4945)api/client/go/client.gen.go (2)
Annotations(951-951)Metadata(4478-4478)openmeter/ent/db/grant/grant.go (4)
FieldAnnotations(20-20)FieldMetadata(28-28)FieldExpiration(38-38)FieldExpiresAt(40-40)
e2e/entitlement_test.go (3)
api/client/javascript/src/client/schemas.ts (1)
EntitlementMeteredV2CreateInputs(11545-11546)api/api.gen.go (1)
EntitlementMeteredV2CreateInputs(3174-3215)api/client/go/client.gen.go (1)
EntitlementMeteredV2CreateInputs(2888-2929)
openmeter/entitlement/connector.go (3)
openmeter/entitlement/entitlement.go (2)
CreateEntitlementInputs(78-103)Entitlement(233-247)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)openmeter/entitlement/metered/entitlement_grant.go (1)
CreateEntitlementGrantInputs(162-162)
openmeter/credit/engine/run_test.go (3)
api/api.gen.go (1)
ExpirationPeriod(3421-3427)openmeter/ent/db/balancesnapshot/where.go (1)
At(94-96)openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
openmeter/ent/db/grant_create.go (8)
api/client/javascript/src/client/schemas.ts (3)
Annotations(11341-11341)ExpirationPeriod(11563-11563)Metadata(11683-11683)api/api.gen.go (3)
Annotations(1036-1036)ExpirationPeriod(3421-3427)Metadata(4945-4945)api/client/go/client.gen.go (3)
Annotations(951-951)ExpirationPeriod(3135-3141)Metadata(4478-4478)openmeter/ent/db/grant/grant.go (2)
FieldAnnotations(20-20)FieldMetadata(28-28)openmeter/ent/db/entitlement/entitlement.go (2)
FieldAnnotations(67-67)FieldMetadata(23-23)openmeter/billing/profile.go (1)
Metadata(22-22)openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)pkg/set/set.go (1)
Set(5-8)
openmeter/credit/engine/grant.go (2)
openmeter/credit/grant/grant.go (1)
Grant(14-58)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)
e2e/entitlement_parity_test.go (3)
api/api.gen.go (10)
EntitlementMeteredV2CreateInputs(3174-3215)ExpirationPeriod(3421-3427)GrantV2PaginatedResponse(3733-3745)CreateFeatureJSONRequestBody(9073-9073)RecurringPeriodInterval(6394-6396)RecurringPeriodIntervalEnumMONTH(735-735)RecurringPeriodCreateInput(6385-6391)EntitlementGrantCreateInputV2(2867-2902)CreateCustomerEntitlementV2JSONRequestBody(9160-9160)ListCustomerEntitlementGrantsV2Params(8818-8846)e2e/helpers.go (1)
CreateCustomerWithSubject(16-48)pkg/convert/ptr.go (1)
ToPointer(10-12)
openmeter/entitlement/service/service.go (6)
openmeter/entitlement/metered/connector.go (1)
Connector(33-46)openmeter/entitlement/entitlement.go (1)
CreateEntitlementInputs(78-103)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)openmeter/entitlement/metered/entitlement_grant.go (1)
CreateEntitlementGrantInputs(162-162)pkg/framework/transaction/transaction.go (1)
Run(32-50)openmeter/entitlement/errors.go (2)
ErrEntitlementGrantsOnlySupportedForMeteredEntitlements(79-82)ErrEntitlementCreatePropertyMismatch(72-75)
openmeter/entitlement/metered/balance_test.go (1)
openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
openmeter/ent/db/setorclear.go (5)
openmeter/ent/db/grant_update.go (2)
GrantUpdate(20-24)GrantUpdateOne(215-220)api/client/javascript/src/client/schemas.ts (1)
Annotations(11341-11341)api/api.gen.go (1)
Annotations(1036-1036)api/client/go/client.gen.go (1)
Annotations(951-951)pkg/models/annotation.go (1)
Annotations(3-3)
openmeter/entitlement/service/scheduling_test.go (10)
openmeter/meter/service.go (1)
CreateMeterInput(107-117)openmeter/productcatalog/feature/connector.go (1)
CreateFeatureInputs(17-24)openmeter/entitlement/entitlement.go (4)
CreateEntitlementInputs(78-103)EntitlementType(305-305)EntitlementTypeBoolean(313-313)EntitlementTypeMetered(309-309)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)openmeter/entitlement/metered/entitlement_grant.go (2)
CreateEntitlementGrantInputs(162-162)ListEntitlementGrantsParams(23-29)openmeter/credit/grant.go (1)
CreateGrantInput(25-34)openmeter/entitlement/errors.go (1)
ErrEntitlementGrantsOnlySupportedForMeteredEntitlements(79-82)openmeter/entitlement/usageperiod.go (2)
UsagePeriod(56-58)NewUsagePeriodInputFromRecurrence(34-38)pkg/timeutil/recurrence.go (2)
Recurrence(13-18)RecurrencePeriodDaily(320-320)pkg/pagination/page.go (2)
Page(14-17)NewPage(20-25)
openmeter/entitlement/driver/v2/customer_metered.go (4)
api/api.gen.go (3)
EntitlementGrant(2775-2830)EntitlementGrantV2(2905-2964)EntitlementGrantCreateInputV2(2867-2902)pkg/pagination/result.go (1)
MapResult(26-37)openmeter/entitlement/metered/entitlement_grant.go (1)
EntitlementGrant(131-144)openmeter/entitlement/driver/v2/mapping.go (2)
MapEntitlementGrantToAPIV2(199-233)MapAPIGrantV2ToCreateGrantInput(392-430)
test/entitlement/regression/scenario_test.go (3)
api/client/javascript/src/client/schemas.ts (1)
ExpirationPeriod(11563-11563)api/api.gen.go (1)
ExpirationPeriod(3421-3427)api/client/go/client.gen.go (1)
ExpirationPeriod(3135-3141)
openmeter/ent/db/mutation.go (4)
api/api.gen.go (3)
Annotations(1036-1036)ExpirationPeriod(3421-3427)Metadata(4945-4945)pkg/models/annotation.go (1)
Annotations(3-3)openmeter/ent/db/grant/grant.go (4)
FieldAnnotations(20-20)FieldMetadata(28-28)FieldExpiration(38-38)FieldExpiresAt(40-40)pkg/models/metadata.go (1)
Metadata(9-9)
openmeter/ent/db/migrate/schema.go (3)
openmeter/ent/db/customer/customer.go (1)
Columns(112-132)openmeter/ent/db/entitlement/entitlement.go (1)
Columns(136-162)openmeter/ent/db/subscriptionitem/subscriptionitem.go (1)
Columns(105-129)
pkg/timeutil/boundedperiod.go (4)
api/client/javascript/src/client/schemas.ts (1)
Period(11773-11773)api/api.gen.go (1)
Period(5794-5800)api/client/go/client.gen.go (1)
Period(5284-5290)pkg/timeutil/openperiod.go (1)
OpenPeriod(8-11)
tools/migrate/migrations/20250925160958_grant-expiration-optional.down.sql
Show resolved
Hide resolved
|
@hekike please validate you're OK with the summary in PR description (I don't ask you to do a full code review just that we're aligned on the intended change) |
GAlexIHU
force-pushed
the
refactor/entitlement-v2-touchup
branch
from
d69bcd6 to
04fb6cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
openmeter/credit/driver/grant.go (1)
243-263: Potential nil dereference when resolving customersGetCustomer may return (nil, nil) for not found depending on implementation. Accessing cus.UsageAttribution without a nil check risks a panic.
Apply:
- if cus != nil && cus.IsDeleted() { + if cus != nil && cus.IsDeleted() { return ListGrantsV2HandlerRequest{}, models.NewGenericPreConditionFailedError( fmt.Errorf("customer is deleted [namespace=%s customer.id=%s]", cus.Namespace, cus.ID), ) } - - subjectKeys = append(subjectKeys, cus.UsageAttribution.SubjectKeys...) + if cus == nil { + return ListGrantsV2HandlerRequest{}, commonhttp.NewHTTPError(http.StatusNotFound, + fmt.Errorf("customer not found [namespace=%s idOrKey=%s]", ns, idOrKey)) + } + subjectKeys = append(subjectKeys, cus.UsageAttribution.SubjectKeys...)test/customer/customer.go (1)
861-866: Drop the superfluousnilargument.
CreateEntitlementnow takes a variadic list of options. Passing an explicitnilgets forwarded as a concrete option value and will panic when the implementation executes it. Just callCreateEntitlement(ctx, inputs)(omitting the third argument) to use the defaults.- entitlement, err := entitlementService.CreateEntitlement(ctx, entitlement.CreateEntitlementInputs{ + entitlement, err := entitlementService.CreateEntitlement(ctx, entitlement.CreateEntitlementInputs{ Namespace: s.namespace, FeatureID: lo.ToPtr(feature.ID), EntitlementType: entitlement.EntitlementTypeBoolean, UsageAttribution: createdCustomer.GetUsageAttribution(), - }, nil) + })openmeter/credit/grant.go (1)
36-88: Guard againstinput.Annotationsbeing nil.
models.Annotationsis an alias ofmap[string]interface{}. Passing a nil map down to the repo risks triggeringnildereferences later (e.g., in JSON marshalling). Normalise it before storing.- repoInp := grant.RepoCreateInput{ + repoInp := grant.RepoCreateInput{ OwnerID: ownerID.ID, Namespace: ownerID.Namespace, Amount: input.Amount, Priority: input.Priority, EffectiveAt: input.EffectiveAt, - Expiration: input.Expiration, - Annotations: input.Annotations, + Expiration: input.Expiration, + Annotations: lo.FromPtr(lo.ToPtr(input.Annotations)),Alternatively, initialise to an empty map when nil.
openmeter/credit/engine/grant.go (1)
173-205: Deterministic ordering needs matching tie-breakers.You now stable-sort by expiration after created_at, but the final
sort.SliceStableon priority ignores the pre-sorted order when priorities match. We still need to maintain expiration/created_at ordering for equal priorities; switch the last step to a stable sort.- sort.SliceStable(grants, func(i, j int) bool { + slices.SortStableFunc(grants, func(i, j grant.Grant) int { return grants[i].Priority < grants[j].Priority })openmeter/entitlement/driver/metered.go (1)
110-112: Default maxRolloverAmount should default to Amount (per PR objective).Align V1 create path with the new default to avoid inconsistent behavior across APIs.
Apply this diff:
- ResetMaxRollover: defaultx.WithDefault(apiGrant.MaxRolloverAmount, 0), + ResetMaxRollover: defaultx.WithDefault(apiGrant.MaxRolloverAmount, apiGrant.Amount),api/client/javascript/src/zod/index.ts (1)
17608-17614: Purgemetadatafrom the Grant V2 schemaThe standalone Grant V2 payload still exposes
metadataeven though V2 should standardize onannotations. This duplicates state and contradicts the rename communicated to clients. Remove themetadatablock here (and anywhere else in the V2 surfaces) so grants only exposeannotations.- annotations: zod - .record(zod.string(), zod.any()) - .describe( - 'Set of key-value pairs managed by the system. Cannot be modified by user.' - ) - .optional() - .describe('The grant metadata.'), + annotations: zod + .record(zod.string(), zod.any()) + .describe( + 'Set of key-value pairs managed by the system. Cannot be modified by user.' + ) + .optional() + .describe('The grant annotations.'), @@ - metadata: zod - .record(zod.string(), zod.coerce.string())openmeter/entitlement/driver/v2/customer_metered.go (1)
48-55: Bug: request.Params from URL are dropped in ListCustomerEntitlementGrantsV2The handler doesn’t propagate Params into the request struct, so pagination/order filters from the query are ignored.
Apply this diff to forward query params:
return ListCustomerEntitlementGrantsHandlerRequest{ CustomerIDOrKey: params.CustomerIDOrKey, EntitlementIdOrFeatureKey: params.EntitlementIdOrFeatureKey, Namespace: ns, + Params: params.Params, }, nilAdditionally, consider propagating includeDeleted/offset/limit if supported by meteredentitlement.ListEntitlementGrantsParams.
🧹 Nitpick comments (43)
pkg/timeutil/boundedperiod.go (3)
5-8: Add doc comment for exported type.Exported type without a comment will trigger lint warnings. Briefly document bounds and membership semantics.
Apply this diff:
+// StartBoundedPeriod represents a start-bounded time interval. +// Membership semantics: +// - ContainsInclusive: [From, To] (To may be nil → unbounded end) +// - ContainsExclusive: (From, To) +// - Contains: [From, To) type StartBoundedPeriod struct { From time.Time `json:"from"` To *time.Time `json:"to,omitempty"` }
51-56: Avoid returning a pointer to a field of a value receiver (aliasing surprise).This is safe (the field will escape), but the pointer won’t alias the original struct’s field, which can surprise callers. Prefer a pointer receiver or take an explicit local copy.
Two options:
- Option A (preferred): switch to pointer receiver for clearer aliasing
-func (p StartBoundedPeriod) Open() OpenPeriod { +func (p *StartBoundedPeriod) Open() OpenPeriod { return OpenPeriod{ - From: &p.From, + From: &p.From, To: p.To, } }
- Option B (no receiver change): take an explicit local copy to make intent obvious
func (p StartBoundedPeriod) Open() OpenPeriod { - return OpenPeriod{ - From: &p.From, - To: p.To, - } + from := p.From + return OpenPeriod{From: &from, To: p.To} }
10-10: Adjust interface assertion if adopting a pointer receiver for Open().If you choose Option A above, assert the pointer type implements Period.
Apply this diff (only if you switch Open to pointer receiver):
-var _ Period = StartBoundedPeriod{} +var _ Period = (*StartBoundedPeriod)(nil)pkg/timeutil/openperiod_test.go (1)
768-872: Good coverage; add half‑open adjacency case.Add a case where [a,b) and [b,c) just touch. For OpenPeriod this should not overlap.
Apply this diff to extend the "Should work for open periods" tests list:
tests := []struct { name string period1 OpenPeriod period2 OpenPeriod expected bool }{ + { + name: "Half-open touching (no overlap)", + period1: OpenPeriod{From: &before, To: &now}, + period2: OpenPeriod{From: &now, To: &after}, + expected: false, + }, { name: "Open Ended Both", period1: OpenPeriod{From: &before, To: nil}, period2: OpenPeriod{From: &now, To: nil}, expected: true, },openmeter/entitlement/entitlement_types.go (1)
22-22: Replace FIXME with actionable TODO + deprecation planIf SubTypeConnector is slated for removal, add a TODO referencing a tracking issue and outline migration steps for existing connectors to avoid future breakage.
I can draft the deprecation notice and open an issue with a proposed migration checklist.
openmeter/credit/driver/grant.go (1)
301-306: Pagination fields when limit/offset providedIf ListParams used limit/offset, grants.Page may be zeroed. Consider deriving Page/PageSize from Limit/Offset to avoid returning zeros.
- return api.GrantV2PaginatedResponse{ - Items: apiGrants, - TotalCount: grants.TotalCount, - Page: grants.Page.PageNumber, - PageSize: grants.Page.PageSize, - }, nil + resp := api.GrantV2PaginatedResponse{ + Items: apiGrants, + TotalCount: grants.TotalCount, + Page: grants.Page.PageNumber, + PageSize: grants.Page.PageSize, + } + if resp.Page == 0 && request.params.Limit > 0 { + resp.PageSize = request.params.Limit + if request.params.Limit > 0 { + resp.Page = (request.params.Offset / request.params.Limit) + 1 + } + } + return resp, nilopenmeter/entitlement/service/scheduling_test.go (3)
83-87: Use clock.Now() for deterministic anchors in tests.Avoid time.Now() in tests to reduce flakiness and align with existing clock usage.
- Anchor: time.Now(), + Anchor: clock.Now(),
91-96: Stabilize EffectiveAt timestamp source.Prefer clock.Now() (or a fixed test time) over time.Now() for reproducibility.
- EffectiveAt: time.Now().Truncate(time.Minute).Add(time.Minute), + EffectiveAt: clock.Now().Truncate(time.Minute).Add(time.Minute),
127-132: Same: use clock.Now() for EffectiveAt.Keep time source consistent across the test.
- EffectiveAt: time.Now().Truncate(time.Minute).Add(time.Minute), + EffectiveAt: clock.Now().Truncate(time.Minute).Add(time.Minute),openmeter/entitlement/connector.go (1)
61-67: Document the new invariants on Create/Override.Add concise docs to the interface: grants only for metered entitlements; exactly one of input.IssueAfterReset or grants required when metered.
- // Meant for API use primarily + // Meant for API use primarily. + // Notes: + // - Grants are only supported for EntitlementTypeMetered. + // - When EntitlementType is metered, exactly one of input.IssueAfterReset or grants must be provided.tools/migrate/migrations/20250925160650_grant-annotations.up.sql (2)
5-7: Clarify comment or enforce stringification in SQL.The UPDATE does a straight copy; it does not convert values to strings. Either adjust the comment or implement explicit text conversion.
--- a/tools/migrate/migrations/20250925160650_grant-annotations.up.sql +++ b/tools/migrate/migrations/20250925160650_grant-annotations.up.sql - -- NOTE: technically, this might parse the values from strings to other primitive types when we later parse this. - -- convert metadata to annotations: all values will be converted to strings + -- NOTE: metadata is a jsonb object of string values; we copy it 1:1 into annotations. + -- If a future schema allows non-string values in annotations, this copy remains safe and backwards compatible.
11-11: Consider creating the GIN index concurrently to avoid write locks.On large tables, CREATE INDEX can block writes. Use CONCURRENTLY if your migration runner supports non-transactional DDL.
- CREATE INDEX "grant_annotations" ON "grants" USING gin ("annotations"); + -- If your migration tool supports it (and runs this outside a transaction): + -- CREATE INDEX CONCURRENTLY IF NOT EXISTS "grant_annotations" ON "grants" USING gin ("annotations");If the migration framework wraps statements in a transaction, keep the current form to avoid errors; alternatively, split into a separate non-transactional migration. Do you want me to draft both variants?
openmeter/entitlement/driver/metered.go (2)
450-463: Prefer stable JSON stringification when down‑converting annotations to metadata.fmt.Sprintf can be lossy/ambiguous for complex values. JSON encoding is more stable.
Apply this diff:
- result := make(api.Metadata) - if len(annotations) > 0 { - for k, v := range annotations { - result[k] = fmt.Sprintf("%v", v) - } - } + result := make(api.Metadata, len(annotations)) + for k, v := range annotations { + if b, err := json.Marshal(v); err == nil { + result[k] = string(b) + } else { + result[k] = fmt.Sprintf("%v", v) + } + }And add import:
import "encoding/json"
115-117: Tiny simplification: avoid unnecessary lo.FromPtr.You already guarded for nil. Use direct deref for clarity.
- req.GrantInput.Annotations = AnnotationsFromMetadata(lo.FromPtr(apiGrant.Metadata)) + req.GrantInput.Annotations = AnnotationsFromMetadata(*apiGrant.Metadata)api/spec/src/entitlements/v2/grant.tsp (2)
31-36: Typo: “recurr” → “recur”.- /** - * The next time the grant will recurr. - */ + /** + * The next time the grant will recur. + */
85-90: Clarify description: this is annotations, not metadata.- /** - * The grant metadata. - */ + /** + * The grant annotations. + */openmeter/ent/db/grant.go (1)
280-296: Stringer prints annotations and deprecated metadata.Optional: consider omitting metadata in String() in a future cleanup to reduce noise.
openmeter/entitlement/driver/v2/mapping.go (3)
199-214: Prefer EmptyableToPtr for annotations to avoid emitting empty maps.This matches ToMeteredV2 behavior and keeps payloads cleaner.
- Annotations: lo.ToPtr(api.Annotations(grant.Annotations)), + Annotations: lo.EmptyableToPtr(api.Annotations(grant.Annotations)),
216-222: Don’t gate ExpiresAt on Expiration presence.ExpiresAt can theoretically exist independently; set it unconditionally.
if grant.Expiration != nil { apiGrant.Expiration = &api.ExpirationPeriod{ Count: grant.Expiration.Count, Duration: api.ExpirationDuration(grant.Expiration.Duration), } - apiGrant.ExpiresAt = grant.ExpiresAt } + + apiGrant.ExpiresAt = grant.ExpiresAt
411-417: Back-compat: accept deprecated metadata when annotations are absent.Gracefully map metadata → annotations to support older clients using V2.
- if g.Annotations != nil && len(lo.FromPtr(g.Annotations)) > 0 { + if g.Annotations != nil && len(lo.FromPtr(g.Annotations)) > 0 { grantInput.Annotations = make(models.Annotations) for k, v := range lo.FromPtr(g.Annotations) { grantInput.Annotations[k] = v } + } else if g.Metadata != nil && len(lo.FromPtr(g.Metadata)) > 0 { + grantInput.Annotations = make(models.Annotations) + for k, v := range lo.FromPtr(g.Metadata) { + grantInput.Annotations[k] = v + } }api/client/javascript/src/client/schemas.ts (4)
4749-4808: Grant V2 create input looks correct; minor range validation improvementSchema aligns with backend (amount required, expiration optional, annotations vs metadata). Consider encoding min constraints for numeric fields (e.g., amount > 0) in the spec to generate stronger TS/runtime validators.
If you want, I can propose a TSP/OpenAPI patch to add minimum constraints for amount/minRolloverAmount and uint8 bounds for priority.
4810-4917: Make voidedAt read-only for consistency; fix typo in description
- voidedAt is server-controlled like createdAt/deletedAt/expiresAt; mark it read-only for consistency.
- Typo: “recurr” → “recur”.
Apply within this spec block:
- /** - * Format: date-time - * @description The next time the grant will recurr. - * @example 2023-01-01T01:01:01.001Z - */ + /** + * Format: date-time + * @description The next time the grant will recur. + * @example 2023-01-01T01:01:01.001Z + */ nextRecurrence?: Date - /** - * Format: date-time - * @description The time the grant was voided. - * @example 2023-01-01T01:01:01.001Z - */ - voidedAt?: Date + /** + * Format: date-time + * @description The time the grant was voided. + * @example 2023-01-01T01:01:01.001Z + */ + readonly voidedAt?: Date
5130-5143: Deprecations are clear; steer users toissueGood to keep deprecated fields for compatibility. Consider adding a short note pointing to
issueandgrantsas the supported alternatives in V2 to nudge migration.
7141-7155:IssueAfterResetschema looks good; consider positive amount constraintConsider adding minimum: 0 (or >0) to align with “amount should be positive” guidance elsewhere.
openmeter/ent/db/mutation.go (5)
35508-35555: Ensure SetAnnotations removes the cleared marker.Setting after clearing can leave a contradictory state. Delete the cleared flag in SetAnnotations.
func (m *GrantMutation) SetAnnotations(value models.Annotations) { - m.annotations = &value + m.annotations = &value + delete(m.clearedFields, dbgrant.FieldAnnotations) }
35678-35726: Align SetMetadata with clear semantics.Mirror annotations behavior so SetMetadata removes any prior clear.
func (m *GrantMutation) SetMetadata(value map[string]string) { - m.metadata = &value + m.metadata = &value + delete(m.clearedFields, dbgrant.FieldMetadata) }
35912-35918: Remove cleared marker on SetExpiration to avoid conflicts.If ClearExpiration was called earlier, setting a value (or nil) should override it.
func (m *GrantMutation) SetExpiration(gp *grant.ExpirationPeriod) { m.expiration = &gp + delete(m.clearedFields, dbgrant.FieldExpiration) } func (m *GrantMutation) Expiration() (r *grant.ExpirationPeriod, exists bool) { v := m.expiration if v == nil { return
36496-36501: Broaden SetField support to accept map[string]interface{} for annotations.Callers often pass the underlying map type. Accept both models.Annotations and map[string]interface{}.
- case dbgrant.FieldAnnotations: - v, ok := value.(models.Annotations) - if !ok { - return fmt.Errorf("unexpected type %T for field %s", value, name) - } - m.SetAnnotations(v) + case dbgrant.FieldAnnotations: + switch v := value.(type) { + case models.Annotations: + m.SetAnnotations(v) + case map[string]interface{}: + m.SetAnnotations(models.Annotations(v)) + default: + return fmt.Errorf("unexpected type %T for field %s", value, name) + } return nil
36523-36529: LGTM: Metadata SetField remains strict to map[string]string.Consider adding a deprecation comment if metadata will be removed later.
api/openapi.yaml (3)
17808-17833: Add basic bounds to pagination fields (minima)Set non-negative/positive minimums to aid validation and client generators.
Apply this diff:
GrantV2PaginatedResponse: @@ totalCount: type: integer + minimum: 0 description: The total number of items. example: 500 page: type: integer + minimum: 1 description: The page index. example: 1 pageSize: type: integer + minimum: 1 description: The maximum number of items per page. example: 100
16106-16294: Optional: clarify default for maxRolloverAmount (not representable as OpenAPI default)Since default equals “amount,” keep explanation in description; avoid setting a static default value that could mislead.
No code change required; consider adding a note: “Default equals the value of amount (enforced server-side).”
16106-16294: Lint non-standarduint8formats and wording typos
- Replace all
format: uint8with OAS-compliant formats (e.g.int32/int64) or drop in favor ofminimum/maximumconstraints.- Correct typos: “ceiled” → “rounded up” (or “ceiling”), “recurr” → “recur”, “decimal numbers” → “decimal number.”
- Add a CI/grep rule to flag any future occurrences.
api/openapi.cloud.yaml (3)
17350-17362: Add minimum constraints to pagination fieldsHelps clients and codegen by formalizing valid ranges.
totalCount: type: integer description: The total number of items. + minimum: 0 example: 500 page: type: integer description: The page index. + minimum: 1 example: 1 pageSize: type: integer description: The maximum number of items per page. + minimum: 1 example: 100
15873-15885: Non-standard integer format “uint8” may break generatorsOpenAPI generators often don’t recognize “uint8”. Consider format: int32 + min/max, or drop format entirely.
priority: type: integer - format: uint8 + format: int32 minimum: 1 maximum: 255Please confirm your target code generators (Go, TS, Java, etc.) accept custom “uint8”. If not, use int32 with bounds.
15873-15885: Apply the same “uint8” fix consistently (GrantV2 and IssueAfterReset)Use standard int32 format in both locations or remove format for broad compatibility.
# In EntitlementGrantV2.priority - format: uint8 + format: int32 @@ # In IssueAfterReset.priority - format: uint8 + format: int32Also applies to: 19094-19100
openmeter/entitlement/driver/v2/customer_metered.go (1)
71-80: Map includeDeleted/offset/limit if supportedOnly order/page/pageSize are applied. If ListEntitlementGrantsParams supports IncludeDeleted and limit/offset, wire them through for parity with the API surface.
openmeter/entitlement/driver/v2/customer.go (3)
42-45: Decode into the pointer, not pointer-to-pointerAvoid double indirection here. Pass the allocated pointer directly to the decoder.
- inp := &api.EntitlementV2CreateInputs{} - def := CreateCustomerEntitlementHandlerRequest{} - if err := commonhttp.JSONRequestBodyDecoder(r, &inp); err != nil { + inp := new(api.EntitlementV2CreateInputs) + def := CreateCustomerEntitlementHandlerRequest{} + if err := commonhttp.JSONRequestBodyDecoder(r, inp); err != nil { return def, err }
353-356: Apply the same single-pointer decode in overrideMirror the create handler fix to avoid double indirection.
- apiInp := &api.EntitlementV2CreateInputs{} - if err := commonhttp.JSONRequestBodyDecoder(r, &apiInp); err != nil { + apiInp := new(api.EntitlementV2CreateInputs) + if err := commonhttp.JSONRequestBodyDecoder(r, apiInp); err != nil { return OverrideCustomerEntitlementHandlerRequest{}, err }
53-55: Update stale commentComment still references “v1 parser”; it now uses the V2 path.
openmeter/ent/db/setorclear.go (1)
2214-2226: Confirm legacy metadata methods are temporaryBoth SetOrClearAnnotations and SetOrClearMetadata exist for Grant updates. If this is for backward compatibility, please track removal of the metadata variants after clients migrate.
api/spec/src/entitlements/v2/entitlements.tsp (2)
150-181: Document mutual exclusivity: issue/issueAfterReset vs grantsRuntime enforces “exactly one of issueAfterReset (or issue) or grants.” Reflect this in the spec to reduce client misuse (doc note is fine if structural constraints aren’t feasible).
152-161: Fix minor grammar in docs (“its” vs “it’s”)Tiny doc polish.
- * That grant will have it's rollover settings configured in a way that after each reset operation, the balance will return the original amount specified here. + * That grant will have its rollover settings configured so that after each reset operation, the balance returns to the original amount specified here.openmeter/entitlement/service/service.go (1)
78-81: Avoid attaching full grants payload to validation errorAttaching the entire grants slice in error attributes can bloat logs. Consider logging only count or a hash.
- return nil, entitlement.ErrEntitlementGrantsOnlySupportedForMeteredEntitlements.WithAttr("entitlement_type", input.EntitlementType).WithAttr("grants", grants) + return nil, entitlement.ErrEntitlementGrantsOnlySupportedForMeteredEntitlements.WithAttr("entitlement_type", input.EntitlementType).WithAttr("grants_count", len(grants))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/migrate/migrations/atlas.sumis excluded by!**/*.sum
📒 Files selected for processing (61)
api/client/javascript/src/client/schemas.ts(17 hunks)api/client/javascript/src/zod/index.ts(8 hunks)api/openapi.cloud.yaml(13 hunks)api/openapi.yaml(13 hunks)api/spec/src/entitlements/v2/customer.tsp(4 hunks)api/spec/src/entitlements/v2/entitlements.tsp(4 hunks)api/spec/src/entitlements/v2/grant.tsp(1 hunks)api/spec/src/entitlements/v2/grants.tsp(1 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)e2e/entitlement_parity_test.go(7 hunks)e2e/entitlement_test.go(6 hunks)openmeter/credit/adapter/grant.go(3 hunks)openmeter/credit/driver/grant.go(3 hunks)openmeter/credit/engine/engine_test.go(4 hunks)openmeter/credit/engine/grant.go(5 hunks)openmeter/credit/engine/reset_test.go(1 hunks)openmeter/credit/engine/run_test.go(9 hunks)openmeter/credit/grant.go(3 hunks)openmeter/credit/grant/events_2.go(4 hunks)openmeter/credit/grant/grant.go(3 hunks)openmeter/credit/grant/grant_test.go(11 hunks)openmeter/credit/grant/repo.go(1 hunks)openmeter/ent/db/grant.go(10 hunks)openmeter/ent/db/grant/grant.go(3 hunks)openmeter/ent/db/grant/where.go(4 hunks)openmeter/ent/db/grant_create.go(14 hunks)openmeter/ent/db/grant_update.go(9 hunks)openmeter/ent/db/migrate/schema.go(3 hunks)openmeter/ent/db/mutation.go(19 hunks)openmeter/ent/db/runtime.go(1 hunks)openmeter/ent/db/setorclear.go(2 hunks)openmeter/ent/schema/grant.go(2 hunks)openmeter/entitlement/connector.go(1 hunks)openmeter/entitlement/driver/entitlement.go(2 hunks)openmeter/entitlement/driver/metered.go(5 hunks)openmeter/entitlement/driver/v2/customer.go(5 hunks)openmeter/entitlement/driver/v2/customer_metered.go(6 hunks)openmeter/entitlement/driver/v2/mapping.go(3 hunks)openmeter/entitlement/entitlement_grant.go(1 hunks)openmeter/entitlement/entitlement_types.go(1 hunks)openmeter/entitlement/errors.go(1 hunks)openmeter/entitlement/metered/balance_test.go(17 hunks)openmeter/entitlement/metered/connector.go(1 hunks)openmeter/entitlement/metered/entitlement_grant.go(2 hunks)openmeter/entitlement/metered/grant_owner_adapter_test.go(1 hunks)openmeter/entitlement/metered/lateevents_test.go(1 hunks)openmeter/entitlement/metered/reset_test.go(13 hunks)openmeter/entitlement/service/access_test.go(6 hunks)openmeter/entitlement/service/scheduling_test.go(3 hunks)openmeter/entitlement/service/service.go(4 hunks)openmeter/entitlement/service/utils_test.go(3 hunks)openmeter/server/server_test.go(1 hunks)pkg/timeutil/boundedperiod.go(1 hunks)pkg/timeutil/openperiod.go(1 hunks)pkg/timeutil/openperiod_test.go(1 hunks)test/customer/customer.go(1 hunks)test/entitlement/regression/scenario_test.go(13 hunks)tools/migrate/migrations/20250925160650_grant-annotations.down.sql(1 hunks)tools/migrate/migrations/20250925160650_grant-annotations.up.sql(1 hunks)tools/migrate/migrations/20250925160958_grant-expiration-optional.down.sql(1 hunks)tools/migrate/migrations/20250925160958_grant-expiration-optional.up.sql(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tools/migrate/migrations/20250925160958_grant-expiration-optional.down.sql
🚧 Files skipped from review as they are similar to previous changes (14)
- openmeter/credit/engine/run_test.go
- api/spec/src/entitlements/v2/grants.tsp
- openmeter/ent/db/migrate/schema.go
- api/spec/src/entitlements/v2/customer.tsp
- openmeter/entitlement/entitlement_grant.go
- api/spec/src/entitlements/v2/main.tsp
- openmeter/server/server_test.go
- pkg/timeutil/openperiod.go
- openmeter/credit/adapter/grant.go
- openmeter/ent/db/runtime.go
- openmeter/entitlement/service/utils_test.go
- openmeter/entitlement/metered/connector.go
- openmeter/ent/db/grant/grant.go
- openmeter/credit/grant/repo.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-07T12:17:43.129Z
Learnt from: GAlexIHU
PR: openmeterio/openmeter#2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may use variables like `meterSlug` and `namespace` without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
Applied to files:
openmeter/entitlement/service/scheduling_test.goopenmeter/entitlement/service/service.go
🧬 Code graph analysis (34)
openmeter/entitlement/metered/grant_owner_adapter_test.go (1)
openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
pkg/timeutil/openperiod_test.go (2)
pkg/timeutil/closedperiod.go (1)
ClosedPeriod(8-11)pkg/timeutil/openperiod.go (1)
OpenPeriod(8-11)
openmeter/credit/grant.go (3)
api/api.gen.go (2)
ExpirationPeriod(3421-3427)Annotations(1036-1036)openmeter/credit/grant/repo.go (1)
RepoCreateInput(57-69)pkg/timeutil/recurrence.go (1)
Recurrence(13-18)
openmeter/entitlement/metered/reset_test.go (1)
openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
e2e/entitlement_test.go (3)
api/client/javascript/src/client/schemas.ts (1)
EntitlementMeteredV2CreateInputs(11546-11547)api/api.gen.go (1)
EntitlementMeteredV2CreateInputs(3174-3215)api/client/go/client.gen.go (1)
EntitlementMeteredV2CreateInputs(2888-2929)
openmeter/entitlement/service/scheduling_test.go (9)
openmeter/meter/service.go (1)
CreateMeterInput(107-117)openmeter/productcatalog/feature/connector.go (1)
CreateFeatureInputs(17-24)openmeter/entitlement/entitlement.go (4)
CreateEntitlementInputs(78-103)EntitlementType(305-305)EntitlementTypeBoolean(313-313)EntitlementTypeMetered(309-309)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)openmeter/entitlement/metered/entitlement_grant.go (2)
CreateEntitlementGrantInputs(162-162)ListEntitlementGrantsParams(23-29)openmeter/credit/grant.go (1)
CreateGrantInput(25-34)openmeter/entitlement/errors.go (1)
ErrEntitlementGrantsOnlySupportedForMeteredEntitlements(79-82)openmeter/entitlement/usageperiod.go (2)
UsagePeriod(56-58)NewUsagePeriodInputFromRecurrence(34-38)pkg/pagination/page.go (2)
Page(14-17)NewPage(20-25)
openmeter/credit/grant/grant.go (4)
api/api.gen.go (2)
ExpirationPeriod(3421-3427)Annotations(1036-1036)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)openmeter/ent/schema/grant.go (5)
Grant(15-17)Grant(19-26)Grant(28-59)Grant(61-66)Grant(68-77)pkg/timeutil/boundedperiod.go (1)
StartBoundedPeriod(5-8)
openmeter/credit/engine/reset_test.go (3)
api/client/javascript/src/client/schemas.ts (1)
ExpirationPeriod(11564-11564)api/api.gen.go (1)
ExpirationPeriod(3421-3427)api/client/go/client.gen.go (1)
ExpirationPeriod(3135-3141)
openmeter/entitlement/driver/v2/mapping.go (7)
api/api.gen.go (11)
IssueAfterReset(4835-4841)EntitlementGrant(2775-2830)Annotations(1036-1036)ExpirationPeriod(3421-3427)RecurringPeriod(6373-6382)CustomerUsageAttribution(2378-2381)EntitlementMeteredV2CreateInputs(3174-3215)EntitlementType(3365-3365)EntitlementGrantCreateInputV2(2867-2902)Metadata(4945-4945)MeasureUsageFrom(4933-4935)openmeter/entitlement/metered/entitlement_grant.go (2)
EntitlementGrant(131-144)CreateEntitlementGrantInputs(162-162)pkg/timeutil/recurrence.go (1)
Recurrence(13-18)openmeter/entitlement/driver/parser.go (1)
MapAPIPeriodIntervalToRecurrence(341-361)openmeter/entitlement/entitlement.go (2)
CreateEntitlementInputs(78-103)MeasureUsageFromInput(42-44)pkg/slicesx/map.go (1)
MapWithErr(25-49)openmeter/credit/grant.go (1)
CreateGrantInput(25-34)
openmeter/entitlement/service/service.go (8)
openmeter/entitlement/metered/connector.go (1)
Connector(33-46)openmeter/entitlement/entitlement.go (1)
CreateEntitlementInputs(78-103)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)openmeter/entitlement/metered/entitlement_grant.go (1)
CreateEntitlementGrantInputs(162-162)pkg/framework/transaction/transaction.go (1)
Run(32-50)openmeter/entitlement/errors.go (1)
ErrEntitlementGrantsOnlySupportedForMeteredEntitlements(79-82)pkg/clock/clock.go (1)
Now(14-21)pkg/models/errors.go (1)
NewGenericValidationError(138-140)
openmeter/ent/db/grant_update.go (5)
api/client/javascript/src/client/schemas.ts (2)
Annotations(11342-11342)Metadata(11684-11684)api/api.gen.go (2)
Annotations(1036-1036)Metadata(4945-4945)api/client/go/client.gen.go (2)
Annotations(951-951)Metadata(4478-4478)openmeter/ent/db/grant/grant.go (4)
FieldAnnotations(20-20)FieldMetadata(28-28)FieldExpiration(38-38)FieldExpiresAt(40-40)openmeter/ent/db/entitlement/entitlement.go (2)
FieldAnnotations(67-67)FieldMetadata(23-23)
pkg/timeutil/boundedperiod.go (4)
api/client/javascript/src/client/schemas.ts (1)
Period(11774-11774)api/api.gen.go (1)
Period(5794-5800)api/client/go/client.gen.go (1)
Period(5284-5290)pkg/timeutil/openperiod.go (1)
OpenPeriod(8-11)
openmeter/ent/db/grant.go (2)
api/api.gen.go (2)
Annotations(1036-1036)Metadata(4945-4945)openmeter/ent/db/grant/grant.go (3)
FieldAnnotations(20-20)FieldMetadata(28-28)FieldExpiration(38-38)
openmeter/credit/engine/grant.go (3)
openmeter/credit/grant/grant.go (1)
Grant(14-58)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)openmeter/ent/schema/grant.go (5)
Grant(15-17)Grant(19-26)Grant(28-59)Grant(61-66)Grant(68-77)
openmeter/credit/grant/events_2.go (2)
api/api.gen.go (3)
ExpirationPeriod(3421-3427)Metadata(4945-4945)Annotations(1036-1036)openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
e2e/entitlement_parity_test.go (5)
api/client/javascript/src/client/schemas.ts (6)
EntitlementMeteredV2CreateInputs(11546-11547)ExpirationPeriod(11564-11564)GrantV2PaginatedResponse(11587-11588)RecurringPeriodInterval(11819-11820)RecurringPeriodCreateInput(11817-11818)EntitlementGrantCreateInputV2(11539-11540)api/api.gen.go (10)
EntitlementMeteredV2CreateInputs(3174-3215)ExpirationPeriod(3421-3427)GrantV2PaginatedResponse(3733-3745)RecurringPeriodInterval(6394-6396)RecurringPeriodIntervalEnumMONTH(735-735)RecurringPeriodCreateInput(6385-6391)EntitlementGrantCreateInputV2(2867-2902)CreateCustomerEntitlementV2JSONRequestBody(9160-9160)ListCustomerEntitlementGrantsV2Params(8818-8846)ListEntitlementGrantsParams(8740-8743)api/client/go/client.gen.go (10)
EntitlementMeteredV2CreateInputs(2888-2929)ExpirationPeriod(3135-3141)GrantV2PaginatedResponse(3383-3395)RecurringPeriodInterval(5872-5874)RecurringPeriodIntervalEnumMONTH(666-666)RecurringPeriodCreateInput(5863-5869)EntitlementGrantCreateInputV2(2596-2631)CreateCustomerEntitlementV2JSONRequestBody(8518-8518)ListCustomerEntitlementGrantsV2Params(8176-8204)ListEntitlementGrantsParams(8098-8101)e2e/helpers.go (1)
CreateCustomerWithSubject(16-48)pkg/convert/ptr.go (1)
ToPointer(10-12)
openmeter/ent/db/mutation.go (4)
api/api.gen.go (3)
Annotations(1036-1036)ExpirationPeriod(3421-3427)Metadata(4945-4945)pkg/models/annotation.go (1)
Annotations(3-3)openmeter/ent/db/grant/grant.go (4)
FieldAnnotations(20-20)FieldMetadata(28-28)FieldExpiration(38-38)FieldExpiresAt(40-40)pkg/models/metadata.go (1)
Metadata(9-9)
api/client/javascript/src/client/schemas.ts (2)
api/api.gen.go (9)
EntitlementGrantCreateInputV2(2867-2902)EntitlementGrantV2(2905-2964)EntitlementMetered(2968-3034)EntitlementMeteredCreateInputs(3055-3092)EntitlementMeteredV2(3099-3168)EntitlementMeteredV2CreateInputs(3174-3215)EntitlementV2CreateInputs(3374-3376)GrantV2PaginatedResponse(3733-3745)IssueAfterReset(4835-4841)openmeter/ent/db/entitlement/where.go (1)
IssueAfterReset(130-132)
openmeter/entitlement/connector.go (4)
openmeter/entitlement/entitlement.go (2)
CreateEntitlementInputs(78-103)Entitlement(233-247)openmeter/entitlement/entitlement_grant.go (1)
CreateEntitlementGrantInputs(5-7)openmeter/entitlement/metered/entitlement_grant.go (1)
CreateEntitlementGrantInputs(162-162)openmeter/registry/entitlement.go (1)
Entitlement(11-21)
openmeter/credit/driver/grant.go (3)
api/api.gen.go (2)
GrantV2PaginatedResponse(3733-3745)EntitlementGrantV2(2905-2964)openmeter/entitlement/metered/entitlement_grant.go (1)
GrantFromCreditGrant(146-160)openmeter/entitlement/driver/v2/mapping.go (1)
MapEntitlementGrantToAPIV2(199-233)
openmeter/entitlement/errors.go (2)
pkg/models/errors.go (1)
ErrorCode(294-294)pkg/models/validationissue.go (1)
NewValidationIssue(219-230)
openmeter/ent/schema/grant.go (3)
pkg/framework/entutils/mixins.go (3)
AnnotationsMixin(157-159)AnnotationsMixin(162-170)AnnotationsMixin(172-181)openmeter/credit/grant/grant.go (1)
Grant(14-58)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)
openmeter/entitlement/metered/balance_test.go (1)
openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
openmeter/credit/grant/grant_test.go (4)
openmeter/credit/grant/grant.go (1)
Grant(14-58)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)openmeter/ent/schema/grant.go (5)
Grant(15-17)Grant(19-26)Grant(28-59)Grant(61-66)Grant(68-77)pkg/models/model.go (1)
ManagedModel(119-125)
openmeter/entitlement/metered/lateevents_test.go (1)
openmeter/ent/db/grant/where.go (1)
ExpiresAt(110-112)
openmeter/credit/engine/engine_test.go (4)
api/client/javascript/src/client/schemas.ts (1)
ExpirationPeriod(11564-11564)api/api.gen.go (1)
ExpirationPeriod(3421-3427)api/client/go/client.gen.go (1)
ExpirationPeriod(3135-3141)openmeter/credit/grant/expiration.go (1)
ExpirationPeriod(6-12)
openmeter/ent/db/grant_create.go (5)
api/client/javascript/src/client/schemas.ts (2)
Annotations(11342-11342)Metadata(11684-11684)api/client/go/client.gen.go (2)
Annotations(951-951)Metadata(4478-4478)openmeter/ent/db/grant/grant.go (2)
FieldAnnotations(20-20)FieldMetadata(28-28)openmeter/ent/db/entitlement/entitlement.go (2)
FieldAnnotations(67-67)FieldMetadata(23-23)openmeter/billing/profile.go (1)
Metadata(22-22)
openmeter/entitlement/metered/entitlement_grant.go (3)
api/client/javascript/src/client/schemas.ts (1)
Annotations(11342-11342)api/api.gen.go (1)
Annotations(1036-1036)api/client/go/client.gen.go (1)
Annotations(951-951)
openmeter/entitlement/driver/metered.go (3)
api/api.gen.go (4)
ExpirationPeriod(3421-3427)Annotations(1036-1036)Metadata(4945-4945)ExpirationDuration(3418-3418)pkg/convert/ptr.go (1)
ToPointer(10-12)pkg/clock/clock.go (1)
Now(14-21)
openmeter/ent/db/grant/where.go (5)
openmeter/ent/db/entitlement/where.go (4)
AnnotationsIsNil(1265-1267)AnnotationsNotNil(1270-1272)MetadataIsNil(236-238)MetadataNotNil(241-243)openmeter/ent/db/subscriptionitem/where.go (4)
AnnotationsIsNil(358-360)AnnotationsNotNil(363-365)MetadataIsNil(348-350)MetadataNotNil(353-355)openmeter/ent/db/grant.go (2)
Grant(21-65)Grant(88-107)openmeter/ent/schema/grant.go (5)
Grant(15-17)Grant(19-26)Grant(28-59)Grant(61-66)Grant(68-77)openmeter/ent/db/grant/grant.go (4)
FieldAnnotations(20-20)FieldMetadata(28-28)FieldExpiration(38-38)FieldExpiresAt(40-40)
openmeter/entitlement/driver/v2/customer_metered.go (4)
api/api.gen.go (4)
GrantV2PaginatedResponse(3733-3745)EntitlementGrant(2775-2830)EntitlementGrantV2(2905-2964)EntitlementGrantCreateInputV2(2867-2902)pkg/pagination/result.go (1)
MapResult(26-37)openmeter/entitlement/metered/entitlement_grant.go (1)
EntitlementGrant(131-144)openmeter/entitlement/driver/v2/mapping.go (2)
MapEntitlementGrantToAPIV2(199-233)MapAPIGrantV2ToCreateGrantInput(393-431)
openmeter/entitlement/driver/v2/customer.go (6)
api/api.gen.go (1)
EntitlementV2CreateInputs(3374-3376)api/client/go/client.gen.go (1)
EntitlementV2CreateInputs(3088-3090)openmeter/entitlement/driver/v2/mapping.go (1)
ParseAPICreateInputV2(235-391)openmeter/customer/customer.go (2)
CustomerIDOrKey(137-140)GetCustomerInput(273-280)pkg/models/errors.go (1)
NewGenericPreConditionFailedError(233-235)pkg/clock/clock.go (1)
Now(14-21)
test/entitlement/regression/scenario_test.go (2)
api/api.gen.go (1)
ExpirationPeriod(3421-3427)api/client/go/client.gen.go (1)
ExpirationPeriod(3135-3141)
openmeter/ent/db/setorclear.go (5)
openmeter/ent/db/grant_update.go (2)
GrantUpdate(20-24)GrantUpdateOne(215-220)api/client/javascript/src/client/schemas.ts (1)
Annotations(11342-11342)api/api.gen.go (1)
Annotations(1036-1036)api/client/go/client.gen.go (1)
Annotations(951-951)pkg/models/annotation.go (1)
Annotations(3-3)
3 participants
Overview
(this is because I realised we don't support overrides when having multiple grants)
Notes for reviewer
metadatachanged toannotations=> db column renamed as already JSONB,Checklist
Summary by CodeRabbit
New Features
Refactor
Chores