feat: on ledger creation return the existing ledger if exists#904
feat: on ledger creation return the existing ledger if exists#904
Conversation
00760c5 to
edc222d
Compare
| description: Ledger Exists | ||
| x-go-type-import: | ||
| path: github.com/openmeterio/openmeter/internal/credit | ||
| x-go-type: credit.LedgerAlreadyExistsProblemResponse |
There was a problem hiding this comment.
Duplication of type required due to this: oapi-codegen/oapi-codegen#1610
| // code. The Problem's Status field will be set to match the status argument, | ||
| // and the Title will be set to the default Go status text for that code. | ||
| func NewStatusProblem(ctx context.Context, err error, status int) Problem { | ||
| func NewStatusProblem(ctx context.Context, err error, status int) *StatusProblem { |
There was a problem hiding this comment.
why did you change this? (just curious)
There was a problem hiding this comment.
Check this: https://go.dev/play/p/GGaWrq1Gu3n
The overal issue is that if an interface is composed with a struct, then json marshaling will treat that as a field member as opposed to being part of the same object.
If we want to define per service problems, like this: https://github.com/openmeterio/openmeter/pull/904/files#diff-f49bfff64b9ead64bbdc915228760a76fb213fcbc0474605aef0c0a5f47c8ad5R19 without putting them into models, we need to have access to the *StatusProblem.
Now I could have done a typecast there, but then if something changes in NewStatusProblem's implementation that might break runtime due to the typecast, so it's cleaner to expose the struct instead.
|
|
||
| func (e *LedgerAlreadyExistsError) Error() string { | ||
| return fmt.Sprintf("ledger (%s.%s) already exitst for subject %s", e.Namespace, e.LedgerID, e.Subject) | ||
| return fmt.Sprintf("ledger (%s.%s) already exitst for subject %s", e.Namespace, e.Ledger.ID, e.Ledger.Subject) |
There was a problem hiding this comment.
Should we omit the namespace from the error? It's largely transparent anyway. It might be more confusing than useful.
Overview
If a ledger for the specified subject already exists, let's return the existing ledger ID as a 409 response, so that no new query will be needed to fetch the ledger by subject.
Example response:
{ "type": "about:blank", "title": "Conflict", "status": 409, "detail": "ledger (default.01HXVNDJR532E8GTBVSC2XK5D4) already exitst for subject subject-1", "instance": "urn:request:Peters-MacBook-Pro-2.local/ZzyNVpzatY-000001", "conflictingEntity": { "id": "01HXVNDJR532E8GTBVSC2XK5D4", "subject": "subject-1" } }Fixes #(issue)
Notes for reviewer
Checklist