feat: add check for uniqueness when creating links with isList=false (#11767)

.changeset/late-shirts-begin.md

@@ -0,0 +1,7 @@
+---
+"@medusajs/link-modules": patch
+"@medusajs/modules-sdk": patch
+"@medusajs/utils": patch
+---
+
+feat: add check for uniqueness when creating links with isList=false

integration-tests/modules/__tests__/link-modules/define-link.spec.ts

@@ -46,6 +46,7 @@ medusaIntegrationTestRunner({
               entity: "Currency",
               primaryKey: "code",
               foreignKey: "currency_code",
+              isList: true,
               alias: "currency",
               args: {
                 methodSuffix: "Currencies",
@@ -58,6 +59,7 @@ medusaIntegrationTestRunner({
               primaryKey: "id",
               foreignKey: "region_id",
               alias: "region",
+              isList: false,
               args: {
                 methodSuffix: "Regions",
               },
@@ -88,9 +90,9 @@ medusaIntegrationTestRunner({
               serviceName: "region",
               entity: "Region",
               fieldAlias: {
-                currency: {
+                currencies: {
                   path: "currency_link.currency",
-                  isList: false,
+                  isList: true,
                   forwardArgumentsOnPath: ["currency_link.currency"],
                 },
               },
@@ -100,7 +102,7 @@ medusaIntegrationTestRunner({
                 primaryKey: "region_id",
                 foreignKey: "id",
                 alias: "currency_link",
-                isList: false,
+                isList: true,
               },
             },
           ],
@@ -145,6 +147,7 @@ medusaIntegrationTestRunner({
               entity: "ProductVariant",
               primaryKey: "id",
               foreignKey: "product_variant_id",
+              isList: true,
               alias: "product_variant",
               args: {
                 methodSuffix: "ProductVariants",
@@ -156,6 +159,7 @@ medusaIntegrationTestRunner({
               entity: "Region",
               primaryKey: "id",
               foreignKey: "region_id",
+              isList: false,
               alias: "region",
               args: {
                 methodSuffix: "Regions",
@@ -187,9 +191,9 @@ medusaIntegrationTestRunner({
               serviceName: "region",
               entity: "Region",
               fieldAlias: {
-                product_variant: {
+                product_variants: {
                   path: "product_variant_link.product_variant",
-                  isList: false,
+                  isList: true,
                   forwardArgumentsOnPath: [
                     "product_variant_link.product_variant",
                   ],
@@ -201,7 +205,7 @@ medusaIntegrationTestRunner({
                 primaryKey: "region_id",
                 foreignKey: "id",
                 alias: "product_variant_link",
-                isList: false,
+                isList: true,
               },
             },
           ],
@@ -249,6 +253,7 @@ medusaIntegrationTestRunner({
               entity: "Currency",
               primaryKey: "code",
               foreignKey: "currency_code",
+              isList: true,
               alias: "currency",
               args: {
                 methodSuffix: "Currencies",
@@ -260,6 +265,7 @@ medusaIntegrationTestRunner({
               entity: "Region",
               primaryKey: "id",
               foreignKey: "region_id",
+              isList: false,
               alias: "region",
               args: {
                 methodSuffix: "Regions",
@@ -291,9 +297,9 @@ medusaIntegrationTestRunner({
               serviceName: "region",
               entity: "Region",
               fieldAlias: {
-                currency: {
+                currencies: {
                   path: "currency_link.currency",
-                  isList: false,
+                  isList: true,
                   forwardArgumentsOnPath: ["currency_link.currency"],
                 },
               },
@@ -303,7 +309,7 @@ medusaIntegrationTestRunner({
                 primaryKey: "region_id",
                 foreignKey: "id",
                 alias: "currency_link",
-                isList: false,
+                isList: true,
               },
             },
           ],
@@ -347,6 +353,7 @@ medusaIntegrationTestRunner({
               entity: "Currency",
               primaryKey: "code",
               foreignKey: "currency_code",
+              isList: true,
               alias: "currency",
               args: {
                 methodSuffix: "Currencies",
@@ -358,6 +365,7 @@ medusaIntegrationTestRunner({
               entity: "Region",
               primaryKey: "id",
               foreignKey: "region_id",
+              isList: true,
               alias: "region",
               args: {
                 methodSuffix: "Regions",
@@ -389,9 +397,9 @@ medusaIntegrationTestRunner({
               serviceName: "region",
               entity: "Region",
               fieldAlias: {
-                currency: {
+                currencies: {
                   path: "currency_link.currency",
-                  isList: false,
+                  isList: true,
                   forwardArgumentsOnPath: ["currency_link.currency"],
                 },
               },
@@ -401,7 +409,7 @@ medusaIntegrationTestRunner({
                 primaryKey: "region_id",
                 foreignKey: "id",
                 alias: "currency_link",
-                isList: false,
+                isList: true,
               },
             },
           ],

packages/core/modules-sdk/src/link.ts

@@ -5,7 +5,13 @@ import {
   ModuleJoinerRelationship,
 } from "@medusajs/types"
 
-import { isObject, Modules, promiseAll, toPascalCase } from "@medusajs/utils"
+import {
+  isObject,
+  MedusaError,
+  Modules,
+  promiseAll,
+  toPascalCase,
+} from "@medusajs/utils"
 import { MedusaModule } from "./medusa-module"
 import { convertRecordsToLinkDefinition } from "./utils/convert-data-to-link-definition"
 import { linkingErrorMessage } from "./utils/linking-error"
@@ -380,18 +386,88 @@ export class Link {
     const allLinks = Array.isArray(link) ? link : [link]
     const serviceLinks = new Map<
       string,
-      [string | string[], string, Record<string, unknown>?][]
+      {
+        linksToCreate: [string | string[], string, Record<string, unknown>?][]
+        linksToValidateForUniqueness: {
+          filters: { [key: string]: string }[]
+          services: string[]
+        }
+      }
     >()
 
     for (const link of allLinks) {
       const service = this.getLinkModuleOrThrow(link)
+      const relationships = service.__joinerConfig.relationships
       const { moduleA, moduleB, moduleBKey, primaryKeys } =
         this.getLinkDataConfig(link)
 
       if (!serviceLinks.has(service.__definition.key)) {
-        serviceLinks.set(service.__definition.key, [])
+        serviceLinks.set(service.__definition.key, {
+          /**
+           * Tuple of foreign key and the primary keys that must be
+           * persisted to the pivot table for representing the
+           * link
+           */
+          linksToCreate: [],
+
+          /**
+           * An array of objects to validate for uniqueness before persisting
+           * data to the pivot table. When a link uses "isList: false", we
+           * have to limit a relationship with this entity to be a one-to-one
+           * or one-to-many
+           */
+          linksToValidateForUniqueness: {
+            filters: [],
+            services: [],
+          },
+        })
       }
 
+      relationships?.forEach((relationship) => {
+        const linksToValidateForUniqueness = serviceLinks.get(
+          service.__definition.key
+        )!.linksToValidateForUniqueness!
+
+        linksToValidateForUniqueness.services.push(relationship.serviceName)
+
+        /**
+         * When isList is set on false on the relationship, then it means
+         * we have a one-to-one or many-to-one relationship with the
+         * other side and we have limit duplicate entries from other
+         * entity. For example:
+         *
+         * - A brand has a many to one relationship with a product.
+         * - A product can have only one brand. Aka (brand.isList = false)
+         * - A brand can have multiple products. Aka (products.isList = true)
+         *
+         * A result of this, we have to ensure that a product_id can only appear
+         * once in the pivot table that is used for tracking "brand<>products"
+         * relationship.
+         */
+        if (relationship.isList === false) {
+          const otherSide = relationships.find(
+            (other) => other.foreignKey !== relationship.foreignKey
+          )
+          if (!otherSide) {
+            return
+          }
+
+          if (moduleBKey === otherSide.foreignKey) {
+            linksToValidateForUniqueness.filters.push({
+              [otherSide.foreignKey]: link[moduleB][moduleBKey],
+            })
+          } else {
+            primaryKeys.forEach((pk) => {
+              if (pk === otherSide.foreignKey) {
+                linksToValidateForUniqueness.filters.push({
+                  [otherSide.foreignKey]: link[moduleA][pk],
+                })
+              }
+            })
+          }
+        }
+      })
+
       const pkValue =
         primaryKeys.length === 1
           ? link[moduleA][primaryKeys[0]]
@@ -403,15 +479,39 @@ export class Link {
         fields.push(link.data)
       }
 
-      serviceLinks.get(service.__definition.key)?.push(fields as any)
+      serviceLinks
+        .get(service.__definition.key)
+        ?.linksToCreate.push(fields as any)
+    }
+
+    for (const [serviceName, data] of serviceLinks) {
+      if (data.linksToValidateForUniqueness.filters.length) {
+        const service = this.modulesMap.get(serviceName)!
+        const existingLinks = await service.list(
+          {
+            $or: data.linksToValidateForUniqueness.filters,
+          },
+          {
+            take: 1,
+          }
+        )
+
+        if (existingLinks.length > 0) {
+          const serviceA = data.linksToValidateForUniqueness.services[0]
+          const serviceB = data.linksToValidateForUniqueness.services[1]
+
+          throw new MedusaError(
+            MedusaError.Types.INVALID_DATA,
+            `Cannot create multiple links between '${serviceA}' and '${serviceB}'`
+          )
+        }
+      }
     }
 
     const promises: Promise<unknown[]>[] = []
-
+    for (const [serviceName, data] of serviceLinks) {
-    for (const [serviceName, links] of serviceLinks) {
       const service = this.modulesMap.get(serviceName)!
-
+      promises.push(service.create(data.linksToCreate))
-      promises.push(service.create(links))
     }
 
     return (await promiseAll(promises)).flat()

packages/core/utils/src/modules-sdk/define-link.ts

@@ -125,7 +125,8 @@ function buildFieldAlias(fieldAliases?: Shortcut | Shortcut[]) {
 }
 
 function prepareServiceConfig(
-  input: DefineLinkInputSource | DefineReadOnlyLinkInputSource
+  input: DefineLinkInputSource | DefineReadOnlyLinkInputSource,
+  defaultOptions?: { isList?: boolean }
 ) {
   let serviceConfig = {} as ModuleLinkableKeyConfig
 
@@ -137,7 +138,7 @@ function prepareServiceConfig(
       alias: source.alias ?? camelToSnakeCase(source.field ?? ""),
       field: input.field ?? source.field,
       primaryKey: source.primaryKey,
-      isList: false,
+      isList: defaultOptions?.isList ?? false,
       deleteCascade: false,
       module: source.serviceName,
       entity: source.entity,
@@ -152,7 +153,7 @@ function prepareServiceConfig(
       alias: source.alias ?? camelToSnakeCase(source.field ?? ""),
       field: input.field ?? source.field,
       primaryKey: source.primaryKey,
-      isList: input.isList ?? false,
+      isList: input.isList ?? defaultOptions?.isList ?? false,
       deleteCascade: input.deleteCascade ?? false,
       module: source.serviceName,
       entity: source.entity,
@@ -183,8 +184,8 @@ export function defineLink(
   rightService: DefineLinkInputSource | DefineReadOnlyLinkInputSource,
   linkServiceOptions?: ExtraOptions | ReadOnlyExtraOptions
 ): DefineLinkExport {
-  const serviceAObj = prepareServiceConfig(leftService)
+  const serviceAObj = prepareServiceConfig(leftService, { isList: true })
-  const serviceBObj = prepareServiceConfig(rightService)
+  const serviceBObj = prepareServiceConfig(rightService, { isList: false })
 
   if (linkServiceOptions?.readOnly) {
     return defineReadOnlyLink(
@@ -373,6 +374,7 @@ ${serviceBObj.module}: {
             methodSuffix: serviceAMethodSuffix,
           },
           deleteCascade: serviceAObj.deleteCascade,
+          isList: serviceAObj.isList,
         },
         {
           serviceName: serviceBObj.module,
@@ -384,6 +386,7 @@ ${serviceBObj.module}: {
             methodSuffix: serviceBMethodSuffix,
           },
           deleteCascade: serviceBObj.deleteCascade,
+          isList: serviceBObj.isList,
         },
       ],
       extends: [

packages/modules/link-modules/src/services/link.ts

@@ -3,11 +3,12 @@ import {
   InjectManager,
   InjectTransactionManager,
   MedusaContext,
+  MikroOrmBaseRepository,
   ModulesSdkUtils,
 } from "@medusajs/framework/utils"
 
 type InjectedDependencies = {
-  linkRepository: any
+  linkRepository: MikroOrmBaseRepository
 }
 
 export default class LinkService<TEntity> {