Handle the delete properly, fix capactio#632

hub-js/graphql/local/schema.graphql

@@ -327,7 +327,7 @@ input UpdateTypeInstanceInput {
 }
 
 input UpdateTypeInstanceBackendInput {
-  context: Any!
+  context: Any
 }
 
 input UpdateTypeInstancesInput {
@@ -483,7 +483,7 @@ type Mutation {
         '
           SET spec.value = apoc.convert.toJson(item.typeInstance.value) RETURN spec
         ',
-          storageRef.abstract AND item.typeInstance.value IS NULL, // built-in:  no value, so use old one
+          storageRef.abstract AND item.typeInstance.value IS NULL, // built-in: no value, so use old one
         '
           MATCH (latestRevision)-[:SPECIFIED_BY]->(latestSpec: TypeInstanceResourceVersionSpec)
           SET spec.value = latestSpec.value RETURN spec
@@ -494,6 +494,7 @@ type Mutation {
         ',
       {spec:spec, latestRevision: latestRevision, item: item}) YIELD value
 
+      // Handle the `backend.context`
       WITH ti, tir, spec, latestRevision, item
       CALL apoc.do.when(
           item.typeInstance.backend IS NOT NULL,

hub-js/src/local/mutation/cypher-errors.ts

@@ -1,4 +1,5 @@
 export enum CustomCypherErrorCode {
+  BadRequest = 400,
   Conflict = 409,
   NotFound = 404,
 }

hub-js/src/local/mutation/delete-type-instance.ts

@@ -2,6 +2,7 @@ import { Transaction } from "neo4j-driver";
 import { Context } from "./context";
 import {
   CustomCypherErrorCode,
+  CustomCypherErrorOutput,
   tryToExtractCustomCypherError,
 } from "./cypher-errors";
 import { logger } from "../../logger";
@@ -41,10 +42,12 @@ export async function deleteTypeInstance(
                 WITH ti
                 WITH ti
                 MATCH (ti)-[:USES]->(others:TypeInstance)
-                WITH count(others) as othersLen
-                RETURN  othersLen > 1 as isUsed
+                MATCH (ti)-[:STORED_IN]->(backendRef: TypeInstanceBackendReference)
+                WITH backendRef, collect(others.id) as allUsedIds
+                WITH  [x IN allUsedIds WHERE x <> backendRef.id ] as usedIds
+                RETURN  usedIds
             }
-            CALL apoc.util.validate(isUsed, apoc.convert.toJson({code: 400}), null)
+            CALL apoc.util.validate(size(usedIds) > 0, apoc.convert.toJson({ids: usedIds, code: 400}), null)
 
             WITH ti
             MATCH (ti)-[:CONTAINS]->(tirs: TypeInstanceResourceVersion)
@@ -89,6 +92,8 @@ export async function deleteTypeInstance(
         { id: args.id, ownerID: args.ownerID || null }
       );
 
+      // NOTE: Use map to ensure that external storage is not called multiple time for the same ID
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
       const deleteExternally = new Map<string, any>();
       result.records.forEach((record) => {
         const out = record.get("out");
@@ -120,6 +125,9 @@ export async function deleteTypeInstance(
         case CustomCypherErrorCode.NotFound:
           err = Error(`TypeInstance was not found`);
           break;
+        case CustomCypherErrorCode.BadRequest:
+          err = generateBadRequestError(customErr);
+          break;
         default:
           err = Error(`Unexpected error code ${customErr.code}`);
           break;
@@ -133,3 +141,13 @@ export async function deleteTypeInstance(
     await neo4jSession.close();
   }
 }
+
+function generateBadRequestError(customErr: CustomCypherErrorOutput) {
+  if (!Object.prototype.hasOwnProperty.call(customErr, "ids")) {
+    // it shouldn't happen
+    return Error(`ypeInstance is used by other TypeInstances`);
+  }
+  return Error(
+    `TypeInstance is used by other TypeInstances, you must first remove ${customErr.ids}`
+  );
+}

hub-js/src/local/mutation/lock-type-instances.ts

@@ -1,7 +1,7 @@
 import { Transaction } from "neo4j-driver";
 import { Context } from "./context";
 import { logger } from "../../logger";
-import { TypeInstanceBackendDetails, TypeInstanceBackendInput } from "../types/type-instance";
+import { TypeInstanceBackendDetails } from "../types/type-instance";
 import { LockInput } from "../storage/service";
 
 export interface LockingTypeInstanceInput {
@@ -47,7 +47,11 @@ export async function lockTypeInstances(
             SET ti.lockedBy = $in.ownerID
             RETURN true as executed`
       );
-      const lockExternals = await getTypeInstanceStoreExternally(tx, args.in.ids, args.in.ownerID);
+      const lockExternals = await getTypeInstanceStoredExternally(
+        tx,
+        args.in.ids,
+        args.in.ownerID
+      );
       await context.delegatedStorage.Lock(...lockExternals);
 
       return args.in.ids;
@@ -112,7 +116,7 @@ export async function switchLocking(
   const resultRow: LockingResult = {
     allIDs: record.get("allIDs"),
     lockedIDs: record.get("lockedIDs"),
-    lockingProcess: record.get("lockingProcess")
+    lockingProcess: record.get("lockingProcess"),
   };
 
   validateLockingProcess(resultRow, args.in.ids);
@@ -125,17 +129,13 @@ function validateLockingProcess(result: LockingResult, expIDs: [string]) {
     const foundIDs = result.allIDs.map((item) => item.properties.id);
     const notFoundIDs = expIDs.filter((x) => !foundIDs.includes(x));
     if (notFoundIDs.length !== 0) {
-      errMsg.push(
-        `TypeInstances with IDs "${notFoundIDs.join("\", \"")}" were not found`
-      );
+      errMsg.push(`TypeInstances with IDs "${notFoundIDs}" were not found`);
     }
 
     const lockedIDs = result.lockedIDs.map((item) => item.properties.id);
     if (lockedIDs.length !== 0) {
       errMsg.push(
-        `TypeInstances with IDs "${lockedIDs.join(
-          "\", \""
-        )}" are locked by different owner`
+        `TypeInstances with IDs "${lockedIDs}" are locked by different owner`
       );
     }
 
@@ -152,10 +152,11 @@ function validateLockingProcess(result: LockingResult, expIDs: [string]) {
   }
 }
 
-export async function getTypeInstanceStoreExternally(
+export async function getTypeInstanceStoredExternally(
   tx: Transaction,
   ids: string[],
-  lockedBy: string): Promise<LockInput[]> {
+  lockedBy: string
+): Promise<LockInput[]> {
   const result = await tx.run(
     `
            UNWIND $ids as id
@@ -183,15 +184,18 @@ export async function getTypeInstanceStoreExternally(
     { ids: ids }
   );
 
-  const output = result.records.map((record) => record.get("value") as ExternallyStoredOutput);
-  return output.filter(x => !x.backend.abstract).map(x => {
-    return {
-      backend: x.backend,
-      typeInstance: {
-        id: x.typeInstanceId,
-        lockedBy: lockedBy
-      }
-    };
-  });
-
+  const output = result.records.map(
+    (record) => record.get("value") as ExternallyStoredOutput
+  );
+  return output
+    .filter((x) => !x.backend.abstract)
+    .map((x) => {
+      return {
+        backend: x.backend,
+        typeInstance: {
+          id: x.typeInstanceId,
+          lockedBy: lockedBy,
+        },
+      };
+    });
 }

hub-js/src/local/mutation/unlock-type-instances.ts

@@ -1,6 +1,10 @@
 import { Transaction } from "neo4j-driver";
 import { Context } from "./context";
-import { getTypeInstanceStoreExternally, LockingTypeInstanceInput, switchLocking } from "./lock-type-instances";
+import {
+  getTypeInstanceStoredExternally,
+  LockingTypeInstanceInput,
+  switchLocking,
+} from "./lock-type-instances";
 import { logger } from "../../logger";
 
 interface UnLockTypeInstanceInput extends LockingTypeInstanceInput {}
@@ -24,7 +28,11 @@ export async function unlockTypeInstances(
             SET ti.lockedBy = null
             RETURN true as executed`
       );
-      const unlockExternals = await getTypeInstanceStoreExternally(tx, args.in.ids, args.in.ownerID);
+      const unlockExternals = await getTypeInstanceStoredExternally(
+        tx,
+        args.in.ids,
+        args.in.ownerID
+      );
       await context.delegatedStorage.Unlock(...unlockExternals);
 
       return args.in.ids;

hub-js/src/local/mutation/update-type-instances.ts

@@ -101,7 +101,7 @@ function generateConflictError(customErr: CustomCypherErrorOutput) {
 // Simplified version of: https://github.com/neo4j-graphql/neo4j-graphql-js/blob/381ef0302bbd11ecd0f94f978045cdbc61c39b8e/src/utils.js#L57
 // We know the variable name as the mutation is written by us, and this function is not meant to be generic.
 function extractUpdateMutationResult(result: QueryResult) {
-  let data = result.records.map((record) => record.get("typeInstance"));
+  const data = result.records.map((record) => record.get("typeInstance"));
   // handle Integer fields
   // @ts-ignore
   return _.cloneDeepWith(data, (field) => {

hub-js/src/local/storage/service.ts

@@ -1,9 +1,11 @@
 import {
   GetValueRequest,
   OnCreateRequest,
-  OnDeleteRequest, OnLockRequest, OnUnlockRequest,
+  OnDeleteRequest,
+  OnLockRequest,
+  OnUnlockRequest,
   OnUpdateRequest,
-  StorageBackendDefinition
+  StorageBackendDefinition,
 } from "../../generated/grpc/storage_backend";
 import { createChannel, createClient, Client } from "nice-grpc";
 import { Driver } from "neo4j-driver";
@@ -113,7 +115,7 @@ export default class DelegatedStorageService {
         value: new TextEncoder().encode(
           JSON.stringify(input.typeInstance.value)
         ),
-        context: this.encode(input.backend.context)
+        context: this.encode(input.backend.context),
       };
       const res = await cli.onCreate(req);
 
@@ -124,7 +126,7 @@ export default class DelegatedStorageService {
       const updateCtx = JSON.parse(res.context.toString());
       mapping = {
         ...mapping,
-        [input.typeInstance.id]: updateCtx
+        [input.typeInstance.id]: updateCtx,
       };
     }
 
@@ -157,7 +159,7 @@ export default class DelegatedStorageService {
         newValue: new TextEncoder().encode(
           JSON.stringify(input.typeInstance.newValue)
         ),
-        context: this.encode(input.backend.context)
+        context: this.encode(input.backend.context),
       };
 
       await cli.onUpdate(req);
@@ -188,7 +190,7 @@ export default class DelegatedStorageService {
       const req: GetValueRequest = {
         typeInstanceId: input.typeInstance.id,
         resourceVersion: input.typeInstance.resourceVersion,
-        context: this.encode(input.backend.context)
+        context: this.encode(input.backend.context),
       };
       const res = await cli.getValue(req);
 
@@ -201,7 +203,7 @@ export default class DelegatedStorageService {
       const decodeRes = JSON.parse(res.value.toString());
       result = {
         ...result,
-        [input.typeInstance.id]: decodeRes
+        [input.typeInstance.id]: decodeRes,
       };
     }
 
@@ -227,7 +229,7 @@ export default class DelegatedStorageService {
 
       const req: OnDeleteRequest = {
         typeInstanceId: input.typeInstance.id,
-        context: this.encode(input.backend.context)
+        context: this.encode(input.backend.context),
       };
       await cli.onDelete(req);
     }
@@ -253,7 +255,7 @@ export default class DelegatedStorageService {
       const req: OnLockRequest = {
         typeInstanceId: input.typeInstance.id,
         lockedBy: input.typeInstance.lockedBy,
-        context: this.encode(input.backend.context)
+        context: this.encode(input.backend.context),
       };
       await cli.onLock(req);
     }
@@ -278,13 +280,12 @@ export default class DelegatedStorageService {
 
       const req: OnUnlockRequest = {
         typeInstanceId: input.typeInstance.id,
-        context: this.encode(input.backend.context)
+        context: this.encode(input.backend.context),
       };
       await cli.onUnlock(req);
     }
   }
 
-
   private async storageInstanceDetailsFetcher(
     id: string
   ): Promise<StorageInstanceDetails> {
@@ -350,17 +351,18 @@ export default class DelegatedStorageService {
     return this.registeredClients.get(id);
   }
 
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
   private static convertToJSONIfObject(val: any) {
     if (val instanceof Array || val instanceof Object) {
       return JSON.stringify(val);
     }
     return val;
   }
 
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
   private encode(val: any) {
     return new TextEncoder().encode(
       DelegatedStorageService.convertToJSONIfObject(val)
     );
   }
-
 }