Merge pull request #904 from openmeterio/feature/upsert-ledger-2

feat: on ledger creation return the existing ledger if exists

api/openapi.yaml

@@ -794,6 +794,8 @@ paths:
           $ref: "#/components/responses/BadRequestProblemResponse"
         "401":
           $ref: "#/components/responses/UnauthorizedProblemResponse"
+        "409":
+          $ref: "#/components/responses/LedgerAlreadyExistsProblemResponse"
         default:
           $ref: "#/components/responses/UnexpectedProblemResponse"    
 
@@ -1250,6 +1252,65 @@ components:
             type: string
           example:
             stripePaymentId: "pi_4OrAkhLvyihio9p51h9iiFnB"
+    LedgerAlreadyExistsProblem:
+      description: Ledger Exists
+      x-go-type-import:
+        path: github.com/openmeterio/openmeter/internal/credit
+      x-go-type: credit.LedgerAlreadyExistsProblemResponse
+      type: object
+      # TODO: until https://github.com/deepmap/oapi-codegen/pull/1610 is not merged, we
+      # need to copy the Problem schema here
+      # 
+      # Afterwards the schema should be this
+      #allOf:
+      #- $ref: "#/components/schemas/Problem"
+      #- type: object
+      #  properties:
+      #    conflictingEntity:
+      #      $ref: "#/components/schemas/Ledger"
+      properties:
+        type:
+          type: string
+          format: uri
+          description: Type contains a URI that identifies the problem type.
+          example: "urn:problem-type:bad-request"
+        title:
+          type: string
+          description: A a short, human-readable summary of the problem type.
+          example: Bad Request
+        status:
+          type: integer
+          format: int32
+          description: The HTTP status code generated by the origin server for this occurrence of the problem.
+          minimum: 400
+          maximum: 599
+          example: 400
+        detail:
+          type: string
+          description: A human-readable explanation specific to this occurrence of the problem.
+          example: "body must be a JSON object"
+        instance:
+          type: string
+          format: uri
+          description: A URI reference that identifies the specific occurrence of the problem.
+          example: "urn:request:local/JMOlctsKV8-000001"
+        conflictingEntity:
+          $ref: "#/components/schemas/Ledger"
+      additionalProperties: true
+      required:
+        - type
+        - title
+        - status
+        - detail
+      example:
+        type: "about:blank"
+        title: "Conflict"
+        status: 409
+        detail: "ledger (default.01HXVNDJR532E8GTBVSC2XK5D4) already exitst for subject subject-1"
+        instance: "urn:request:local/JMOlctsKV8-000001"
+        conflictingEntity:
+          id: 01HXVNDJR532E8GTBVSC2XK5D4
+          subject: "subject-1"
     Ledger:
       x-go-type-import:
         path: github.com/openmeterio/openmeter/internal/credit
@@ -1383,6 +1444,11 @@ components:
           format: date-time
           example: "2023-01-01T00:00:00Z"
     LedgerGrantBalance:
+      # TODO: until https://github.com/deepmap/oapi-codegen/pull/1610 is not merged, the
+      # generated go type will be invalid (it will be credit.Grant instead of GrantBalance).
+      # 
+      # Given that we are not using this type as a return type of a call, this is just
+      # an inconsistency we can ignore.
       x-go-type-import:
         path: github.com/openmeterio/openmeter/internal/credit
       x-go-type: credit.GrantBalance
@@ -1396,6 +1462,11 @@ components:
               type: number
               example: 100
     FeatureBalance:
+      # TODO: until https://github.com/deepmap/oapi-codegen/pull/1610 is not merged, the
+      # generated go type will be invalid (it will be credit.Feature instead of FeatureBalance).
+      # 
+      # Given that we are not using this type as a return type of a call, this is just
+      # an inconsistency we can ignore.
       x-go-type-import:
         path: github.com/openmeterio/openmeter/internal/credit
       x-go-type: credit.FeatureBalance
@@ -2045,6 +2116,22 @@ components:
             title: "Unauthorized"
             status: 401
             detail: "missing or invalid token"
+    LedgerAlreadyExistsProblemResponse:
+      description: Ledger Exists
+      content:
+        application/problem+json:
+          schema:
+            $ref: "#/components/schemas/LedgerAlreadyExistsProblem"
+          example:
+            type: "urn:problem-type:exists"
+            title: "ledger already exists"
+            status: 409
+            detail: "ledger exists"
+            conflictingEntry:
+              id: 01HXVM0S0CMZJSQ23G9MKXRQP7
+              subject: "subject-1"
+              metadata:
+                stripePaymentId: "pi_4OrAkhLvyihio9p51h9iiFnB"
     NotFoundProblemResponse:
       description: Not Found
       content:

config.example.yaml

@@ -13,8 +13,8 @@ telemetry:
 #     expiration: 768h # 32d
 
 # Entitlements
-# entitlements:
-#   enabled: true
+#entitlements:
+#  enabled: true
 
 # Consumer portal
 # portal:

e2e/e2e_test.go

@@ -558,6 +558,24 @@ func TestCredit(t *testing.T) {
 		ledgerID = resp.JSON201.ID
 	})
 
+	t.Run("Create Ledger for same subject", func(t *testing.T) {
+		resp, err := client.CreateLedgerWithResponse(context.Background(), api.CreateLedgerJSONRequestBody{
+			Subject:  subject,
+			Metadata: ledgerMeta,
+		})
+
+		require.NoError(t, err)
+		require.Equal(t, http.StatusConflict, resp.StatusCode(), "Invalid status code [response_body=%s]", string(resp.Body))
+
+		require.Equal(t,
+			credit.Ledger{
+				ID:       ledgerID,
+				Subject:  subject,
+				Metadata: ledgerMeta,
+			},
+			resp.ApplicationproblemJSON409.ConflictingEntity)
+	})
+
 	t.Run("Create Grant", func(t *testing.T) {
 		effectiveAt, _ := time.Parse(time.RFC3339, "2024-01-01T00:01:00Z")
 
@@ -589,7 +607,7 @@ func TestCredit(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, http.StatusCreated, resp.StatusCode(), "Invalid status code [response_body=%s]", resp.Body)
 
-		expected := &api.LedgerGrantBalance{
+		expected := &credit.Grant{
 			ID:          resp.JSON201.ID,
 			LedgerID:    ledgerID,
 			Type:        credit.GrantTypeUsage,

internal/credit/credit.go

@@ -227,12 +227,11 @@ func (e *LockErrNotObtainedError) Error() string {
 
 type LedgerAlreadyExistsError struct {
 	Namespace string
-	LedgerID  ulid.ULID
-	Subject   string
+	Ledger    Ledger
 }
 
 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)
 }
 
 type LedgerNotFoundError struct {

internal/credit/postgres_connector/ledger.go

@@ -34,8 +34,7 @@ func (c *PostgresConnector) CreateLedger(ctx context.Context, namespace string,
 		}
 		return credit.Ledger{}, &credit.LedgerAlreadyExistsError{
 			Namespace: namespace,
-			LedgerID:  existingLedgerEntity.ID.ULID,
-			Subject:   ledgerIn.Subject,
+			Ledger:    mapDBLedgerToModel(existingLedgerEntity),
 		}
 	}
 

internal/credit/postgres_connector/ledger_test.go

@@ -51,8 +51,10 @@ func TestLedgerCreation(t *testing.T) {
 		assert.True(t, ok, "We got an already exists error")
 		assert.Equal(t, &credit.LedgerAlreadyExistsError{
 			Namespace: namespace,
-			Subject:   ledgerSubject,
-			LedgerID:  existingLedgerID,
+			Ledger: credit.Ledger{
+				Subject: ledgerSubject,
+				ID:      existingLedgerID,
+			},
 		}, details)
 	})
 

internal/credit/problem.go

@@ -0,0 +1,24 @@
+package credit
+
+import (
+	"context"
+	"net/http"
+
+	"github.com/openmeterio/openmeter/pkg/models"
+)
+
+type LedgerAlreadyExistsProblemResponse struct {
+	*models.StatusProblem
+	ConflictingEntity Ledger `json:"conflictingEntity"`
+}
+
+func (p *LedgerAlreadyExistsProblemResponse) Respond(w http.ResponseWriter, r *http.Request) {
+	models.RespondProblem(p, w, r)
+}
+
+func NewLedgerAlreadyExistsProblem(ctx context.Context, err error, existingEntry Ledger) models.Problem {
+	return &LedgerAlreadyExistsProblemResponse{
+		StatusProblem:     models.NewStatusProblem(ctx, err, http.StatusConflict),
+		ConflictingEntity: existingEntry,
+	}
+}

internal/server/router/credit_ledger.go

@@ -67,11 +67,11 @@ func (a *Router) CreateLedger(w http.ResponseWriter, r *http.Request) {
 	if err != nil {
 
 		if existsError, ok := err.(*credit.LedgerAlreadyExistsError); ok {
-			err := fmt.Errorf("ledger already exists for subject: %s, existing ledger %s.%s",
-				existsError.Subject,
-				existsError.Namespace,
-				existsError.LedgerID.String())
-			models.NewStatusProblem(ctx, err, http.StatusBadRequest).Respond(w, r)
+			credit.NewLedgerAlreadyExistsProblem(
+				ctx,
+				err,
+				existsError.Ledger,
+			).Respond(w, r)
 			return
 		}
 		a.config.ErrorHandler.HandleContext(ctx, err)

pkg/models/problem.go

@@ -32,6 +32,7 @@ type Problem interface {
 	Error() string
 	ProblemType() ProblemType
 	ProblemTitle() string
+	ProblemStatus() int
 }
 
 // StatusProblem is the RFC 7807 response body without additional fields.
@@ -50,6 +51,8 @@ type StatusProblem struct {
 	Instance string `json:"instance,omitempty"`
 }
 
+var _ Problem = (*StatusProblem)(nil)
+
 func (p *StatusProblem) Error() string {
 	if p.Err == nil {
 		return fmt.Sprintf("[%s] %s", p.Title, p.Detail)
@@ -66,27 +69,36 @@ func (p *StatusProblem) ProblemType() ProblemType {
 	return p.Type
 }
 
+func (p *StatusProblem) ProblemStatus() int {
+	return p.Status
+}
+
 func (p *StatusProblem) ProblemTitle() string {
 	return p.Title
 }
 
 // Respond will render the problem as JSON to the provided ResponseWriter.
 func (p *StatusProblem) Respond(w http.ResponseWriter, r *http.Request) {
+	RespondProblem(p, w, r)
+}
+
+// Respond will render the problem as JSON to the provided ResponseWriter.
+func RespondProblem(problem Problem, w http.ResponseWriter, r *http.Request) {
 	// Respond
 	buf := &bytes.Buffer{}
 	enc := json.NewEncoder(buf)
 	enc.SetEscapeHTML(true)
-	_ = enc.Encode(p)
+	_ = enc.Encode(problem)
 
 	w.Header().Set("Content-Type", ProblemContentType)
-	w.WriteHeader(p.Status)
+	w.WriteHeader(problem.ProblemStatus())
 	_, _ = w.Write(buf.Bytes())
 }
 
 // NewStatusProblem will generate a problem for the provided HTTP status
 // 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 {
 	var instance string
 	reqID := middleware.GetReqID(ctx)
 	if reqID != "" {