Merge pull request #72 from gadget-inc/change/idempotent-project-create

change: add a more clear error for duplicate project creation

internal/db/project.go

@@ -2,6 +2,7 @@ package db
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/gadget-inc/dateilager/internal/pb"
@@ -13,6 +14,13 @@ func CreateProject(ctx context.Context, tx pgx.Tx, project int64, packPatterns [
 		INSERT INTO dl.projects (id, latest_version, pack_patterns)
 		VALUES ($1, 0, $2)
 	`, project, packPatterns)
+
+	var projectExistsError = errors.New("ERROR: duplicate key value violates unique constraint \"projects_pkey\" (SQLSTATE 23505)")
+
+	if err != nil && err.Error() == projectExistsError.Error() {
+		return fmt.Errorf("project id already exists")
+	}
+
 	if err != nil {
 		return fmt.Errorf("create project %v, packPatterns %v: %w", project, packPatterns, err)
 	}

js/spec/grpc-client.spec.ts

@@ -3,6 +3,10 @@ import { decodeContent, encodeContent } from "../src";
 import { grpcClient } from "./util";
 
 describe("grpc client operations", () => {
+  afterEach(async () => {
+    await grpcClient.deleteProject(1337n);
+  });
+
   it("can create and read an object", async () => {
     await grpcClient.newProject(1337n, []);
     const content = encodeContent("a v1");
@@ -102,4 +106,17 @@ describe("grpc client operations", () => {
 
     expect(result).toBe(12n);
   });
+
+  it("throws a proper error when recreating the same project", async () => {
+    await grpcClient.newProject(1337n, []);
+
+    let errorCaught;
+    try {
+      await grpcClient.newProject(1337n, []);
+      expect(true).toBe(false);
+    } catch (error) {
+      errorCaught = error;
+    }
+    expect((errorCaught as Error).message).toBe("project id 1337 already exists");
+  });
 });

js/src/grpc-client.ts

@@ -3,12 +3,12 @@ import { ChannelCredentials, credentials, Metadata } from "@grpc/grpc-js";
 import type { Span } from "@opentelemetry/api";
 import { context as contextAPI, trace as traceAPI } from "@opentelemetry/api";
 import { GrpcTransport } from "@protobuf-ts/grpc-transport";
-import type { ClientStreamingCall, RpcOptions } from "@protobuf-ts/runtime-rpc";
+import { RpcError, type ClientStreamingCall, type RpcOptions } from "@protobuf-ts/runtime-rpc";
 import { TextDecoder, TextEncoder } from "util";
 import { trace, tracer } from "./internal/telemetry";
 import type { CloneToProjectResponse, GetUnaryResponse, Objekt, Project, UpdateRequest, UpdateResponse } from "./pb/fs_pb";
 import { FsClient } from "./pb/fs_pb.client";
-
+import { ProjectAlreadyExistsError } from "./utils/errors";
 export type { Objekt, Project };
 
 /**
@@ -119,17 +119,24 @@ export class DateiLagerGrpcClient {
    * @param template     The id of the project to start from.
    */
   public async newProject(project: bigint, packPatterns: string[], template?: bigint): Promise<void> {
-    await trace(
-      "dateilager-grpc-client.new-project",
-      {
-        attributes: {
-          "dl.project": String(project),
-          "dl.pack_patterns": packPatterns,
-          "dl.template": String(template),
+    try {
+      await trace(
+        "dateilager-grpc-client.new-project",
+        {
+          attributes: {
+            "dl.project": String(project),
+            "dl.pack_patterns": packPatterns,
+            "dl.template": String(template),
+          },
         },
-      },
-      () => this._client.newProject({ id: project, packPatterns, template }, this._rpcOptions())
-    );
+        () => this._client.newProject({ id: project, packPatterns, template }, this._rpcOptions())
+      );
+    } catch (error) {
+      if (error instanceof RpcError && error.code == "ALREADY_EXISTS") {
+        throw new ProjectAlreadyExistsError(`project id ${project} already exists`);
+      }
+      throw error;
+    }
   }
 
   /**

js/src/utils/errors.ts

@@ -0,0 +1 @@
+export class ProjectAlreadyExistsError extends Error {}

pkg/api/fs.go

@@ -79,7 +79,11 @@ func (f *Fs) NewProject(ctx context.Context, req *pb.NewProjectRequest) (*pb.New
 
 	err = db.CreateProject(ctx, tx, req.Id, req.PackPatterns)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "FS new project %v, %v", req.Id, err)
+		rpcErrorCode := codes.Internal
+		if err.Error() == "project id already exists" {
+			rpcErrorCode = codes.AlreadyExists
+		}
+		return nil, status.Errorf(rpcErrorCode, "FS new project %v, %v", req.Id, err)
 	}
 
 	if req.Template != nil {

test/client_new_test.go

@@ -40,3 +40,33 @@ func TestClientNewProjectEmptyPackPattern(t *testing.T) {
 		"c": {content: "c v1"},
 	})
 }
+
+func TestClientNewProjectDuplicateReportsError(t *testing.T) {
+	projectId := int64(1)
+
+	tc := util.NewTestCtx(t, auth.Admin, projectId)
+	defer tc.Close()
+
+	c, _, close := createTestClient(tc)
+	defer close()
+
+	err := c.NewProject(tc.Context(), projectId, nil, nil)
+	require.NoError(t, err, "NewProject")
+
+	/** Create Project Again**/
+
+	tcSecond := util.NewTestCtx(t, auth.Admin, projectId)
+	defer tc.Close()
+
+	cSecond, _, closeSecond := createTestClient(tc)
+	defer closeSecond()
+
+	errSecond := cSecond.NewProject(tcSecond.Context(), projectId, nil, nil)
+	want := "project id already exists"
+
+	require.Error(t, errSecond, "NewProject")
+
+	if errSecond == nil {
+		t.Errorf("got %s want %s", errSecond, want)
+	}
+}