Merge pull request prisma#4398 from prisma/fix-custom-id-field-name

mavilein · web-flow · commit aa3ef6560679 · 2019-04-18T13:09:45.000+02:00
must not rely on hardcoded name for id field
diff --git a/server/connectors/api-connector-jdbc/src/main/scala/com/prisma/api/connector/jdbc/database/ImportActions.scala b/server/connectors/api-connector-jdbc/src/main/scala/com/prisma/api/connector/jdbc/database/ImportActions.scala
@@ -53,7 +53,7 @@ trait ImportActions extends BuilderBase with SharedJdbcExtensions {
           e.getUpdateCounts.zipWithIndex
             .filter(element => element._1 == Statement.EXECUTE_FAILED)
             .map { failed =>
-              val failedId = argsWithIndex.find(_._2 == failed._2).get._1.rootGC.idField.value
+              val failedId = argsWithIndex.find(_._2 == failed._2).get._1.rootGC.idFieldByName(mutaction.model.idField_!.name).value
               s"Failure inserting ${model.dbName} with Id: $failedId. Cause: ${removeConnectionInfoFromCause(e.getCause())}"
             }
             .toVector
diff --git a/server/connectors/api-connector-jdbc/src/main/scala/com/prisma/api/connector/jdbc/database/NodeActions.scala b/server/connectors/api-connector-jdbc/src/main/scala/com/prisma/api/connector/jdbc/database/NodeActions.scala
@@ -40,7 +40,7 @@ trait NodeActions extends BuilderBase with FilterConditionBuilder with ScalarLis
           rs.next()
           rs.getId(model)
         } else {
-          argsWithIdIfNecessary.idField
+          argsWithIdIfNecessary.idFieldByName(model.idField_!.name)
         }
       }
     )
diff --git a/server/connectors/api-connector/src/main/scala/com/prisma/api/connector/NodeSelector.scala b/server/connectors/api-connector/src/main/scala/com/prisma/api/connector/NodeSelector.scala
@@ -11,9 +11,8 @@ object NodeSelector {
 
 case class NodeSelector(model: Model, field: ScalarField, fieldGCValue: GCValue) {
   require(field.isUnique, s"NodeSelectors may be only instantiated for unique fields! ${field.name} on ${model.name} is not unique.")
-  lazy val value         = fieldGCValue.value
-  lazy val fieldName     = field.name
-  lazy val isId: Boolean = field.name == "id"
+  lazy val value     = fieldGCValue.value
+  lazy val fieldName = field.name
 }
 
 object NodeAddress {
diff --git a/server/connectors/api-connector/src/main/scala/com/prisma/api/schema/Errors.scala b/server/connectors/api-connector/src/main/scala/com/prisma/api/schema/Errors.scala
@@ -33,10 +33,6 @@ object APIErrors {
   case class DataItemDoesNotExist(model: String, uniqueField: String, value: String)
       extends ClientApiError(s"'$model' has no item with $uniqueField '$value'", 3002)
 
-  object DataItemDoesNotExist {
-    def apply(model: String, id: String): DataItemDoesNotExist = DataItemDoesNotExist(model, "id", id)
-  }
-
   case class DataItemAlreadyExists(modelId: String, id: String) extends ClientApiError(s"'$modelId' already has an item with id '$id'", 3004)
 
   case class ExtraArguments(arguments: List[String], model: String)
@@ -101,7 +97,7 @@ object APIErrors {
       extends ClientApiError(s"No Node for the model $modelName with value $value for $fieldName found.", 3039)
 
   case class NodeNotFoundForWhereError(where: NodeSelector)
-    extends ClientApiError(s"No Node for the model ${where.model.name} with value ${where.value} for ${where.field.name} found.", 3039)
+      extends ClientApiError(s"No Node for the model ${where.model.name} with value ${where.value} for ${where.field.name} found.", 3039)
 
   case class NullProvidedForWhereError(modelName: String)
       extends ClientApiError(s"You provided an invalid argument for the where selector on $modelName.", 3040)
@@ -111,8 +107,12 @@ object APIErrors {
 
   case class NodeSelectorInfo(model: String, field: String, value: GCValue)
 
-  case class NodesNotConnectedNative(relationName: String, parentName: String, parentWhere: Option[NodeSelectorInfo], childName: String, childWhere: Option[NodeSelectorInfo])
-    extends ClientApiError(pathErrorMessageNative(relationName, parentName, parentWhere, childName, childWhere), errorCode = 3041)
+  case class NodesNotConnectedNative(relationName: String,
+                                     parentName: String,
+                                     parentWhere: Option[NodeSelectorInfo],
+                                     childName: String,
+                                     childWhere: Option[NodeSelectorInfo])
+      extends ClientApiError(pathErrorMessageNative(relationName, parentName, parentWhere, childName, childWhere), errorCode = 3041)
 
   case class RequiredRelationWouldBeViolated(relation: Relation)
       extends ClientApiError(
@@ -121,10 +121,10 @@ object APIErrors {
       )
 
   case class RequiredRelationWouldBeViolatedNative(relationName: String, modelAName: String, modelBName: String)
-    extends ClientApiError(
-      s"The change you are trying to make would violate the required relation '$relationName' between $modelAName and $modelBName",
-      3042
-    )
+      extends ClientApiError(
+        s"The change you are trying to make would violate the required relation '$relationName' between $modelAName and $modelBName",
+        3042
+      )
 
   case class MongoConflictingUpdates(model: String, override val message: String)
       extends ClientApiError(
diff --git a/server/libs/gc-values/src/main/scala/com/prisma/gc_values/GcValues.scala b/server/libs/gc-values/src/main/scala/com/prisma/gc_values/GcValues.scala
@@ -27,10 +27,6 @@ object RootGCValue {
 }
 
 case class RootGCValue(map: Map[String, GCValue]) extends GCValue {
-  def idField = map.get("id") match {
-    case Some(id) => id.asInstanceOf[IdGCValue]
-    case None     => sys.error("There was no field with name 'id'.")
-  }
 
   def idFieldByName(name: String) = map.get(name) match {
     case Some(id) => id.asInstanceOf[IdGCValue]
diff --git a/server/servers/api/src/main/scala/com/prisma/api/import_export/BulkImport.scala b/server/servers/api/src/main/scala/com/prisma/api/import_export/BulkImport.scala
@@ -81,6 +81,7 @@ class BulkImport(project: Project)(implicit apiDependencies: ApiDependencies) {
     ImportList(ImportIdentifier(typeName, id), field, gcValue)
   }
 
+  // TODO: do4gr should think about whether Import/Export works with custom names for id fields. I think it does but that should be validated.
   def parseIdGCValue(input: JsObject, model: Model): IdGCValue = model.idField_!.typeIdentifier match {
     case TypeIdentifier.UUID => UuidGCValue.parse_!(input.value("id").as[String])
     case TypeIdentifier.Cuid => StringIdGCValue(input.value("id").as[String])
diff --git a/server/servers/api/src/main/scala/com/prisma/api/mutactions/SubscriptionEvents.scala b/server/servers/api/src/main/scala/com/prisma/api/mutactions/SubscriptionEvents.scala
@@ -32,11 +32,10 @@ object SubscriptionEvents {
   }
 
   private def fromUpdateResult(project: Project, mutationId: Id, result: UpdateNodeResult): PublishSubscriptionEvent = {
+    val model = result.mutaction.model
     val previousValues: Map[String, Any] = result.previousValues.data
       .filterValues(v => v != NullGCValue && !v.isInstanceOf[RootGCValue])
-      .toMapStringAny + ("id" -> result.previousValues.id.value)
-
-    val model = result.mutaction.model
+      .toMapStringAny + (model.idField_!.name -> result.previousValues.id.value)
 
     PublishSubscriptionEvent(
       project = project,
@@ -52,11 +51,10 @@ object SubscriptionEvents {
   }
 
   private def fromDeleteResult(project: Project, mutationId: Id, result: DeleteNodeResult): PublishSubscriptionEvent = {
+    val model = result.mutaction.model
     val previousValues = result.previousValues.data
       .filterValues(v => v != NullGCValue && !v.isInstanceOf[RootGCValue])
-      .toMapStringAny + ("id" -> result.previousValues.id.value)
-
-    val model = result.mutaction.model
+      .toMapStringAny + (model.idField_!.name -> result.previousValues.id.value)
 
     PublishSubscriptionEvent(
       project = project,
diff --git a/server/servers/api/src/main/scala/com/prisma/api/mutactions/validation/InputValueValidation.scala b/server/servers/api/src/main/scala/com/prisma/api/mutactions/validation/InputValueValidation.scala
@@ -39,6 +39,7 @@ object InputValueValidation {
   }
 
   def scalarFieldsWithValues(model: Model, args: PrismaArgs): List[ScalarField] = {
-    model.scalarFields.filter(field => args.getFieldValue(field.name).isDefined).filter(_.name != "id")
+    // TODO: do4gr should check whether we should indeed filter out the id field here. (bring your own id changed that maybe)
+    model.scalarFields.filter(field => args.getFieldValue(field.name).isDefined).filter(!_.isId)
   }
 }
diff --git a/server/servers/api/src/main/scala/com/prisma/subscriptions/resolving/FilteredResolver.scala b/server/servers/api/src/main/scala/com/prisma/subscriptions/resolving/FilteredResolver.scala
@@ -26,7 +26,7 @@ object FilteredResolver extends SangriaExtensions {
 
     def removeTopLevelIdFilter(element: Filter) =
       element match {
-        case e: ScalarFilter => e.field.name != "id"
+        case e: ScalarFilter => !e.field.isId
         case _               => true
       }
 
diff --git a/server/servers/api/src/test/scala/com/prisma/api/ApiTestDatabase.scala b/server/servers/api/src/test/scala/com/prisma/api/ApiTestDatabase.scala
@@ -50,7 +50,7 @@ case class ApiTestDatabase()(implicit dependencies: TestApiDependencies) extends
     runMutaction(CreateModelTable(project, model))
 
     model.scalarNonListFields
-      .filter(f => f.name != ReservedFields.idFieldName)
+      .filter(f => !f.isId)
       .map(field => CreateColumn(project, model, field))
       .foreach(runMutaction)
 
diff --git a/server/servers/api/src/test/scala/com/prisma/api/mutations/BringYourOwnIdSpec.scala b/server/servers/api/src/test/scala/com/prisma/api/mutations/BringYourOwnIdSpec.scala
@@ -201,4 +201,25 @@ class BringYourOwnIdSpec extends FlatSpec with Matchers with ApiSpecBase with Sc
       res2.toString should be("""{"data":{"updateParent":{"p":"Parent","id":"Own Id","childOpt":{"c":"test 3","id":"Own Id 3"}}}}""")
     }
   }
+
+  "An id field with a custom name" should "work" in {
+    val dataModel =
+      """
+        |type Blog {
+        |  myId: ID! @id
+        |  name: String!
+        |}
+      """.stripMargin
+
+    val project = SchemaDsl.fromStringV11() { dataModel }
+    database.setup(project)
+
+    val res = server.query(
+      s"""mutation {
+         |  createBlog(data: {name: "MyBlog"}){ name }
+         |}""",
+      project = project
+    )
+    res should be("""{"data": { "createBlog": { "name": "MyBlog" } } }""".parseJson)
+  }
 }
diff --git a/server/shared-models/src/main/scala/com/prisma/shared/models/ReservedFields.scala b/server/shared-models/src/main/scala/com/prisma/shared/models/ReservedFields.scala
@@ -1,89 +1,9 @@
 package com.prisma.shared.models
 
-import com.prisma.shared.models.FieldBehaviour.IdBehaviour
-
 object ReservedFields {
   val idFieldName              = "id"
   val embeddedIdFieldName      = "_id"
   val mongoInternalIdFieldName = "_id"
   val updatedAtFieldName       = "updatedAt"
   val createdAtFieldName       = "createdAt"
-  val reservedFieldNames       = Vector(idFieldName, updatedAtFieldName, createdAtFieldName)
-
-  def reservedFieldFor(name: String): FieldTemplate = {
-    name match {
-      case x if x == idFieldName        => idField
-      case x if x == createdAtFieldName => createdAtField
-      case x if x == updatedAtFieldName => updatedAtField
-      case _                            => throw new Exception(s"Unknown reserved field: $name")
-    }
-  }
-
-  private val createdAtField: FieldTemplate = {
-    FieldTemplate(
-      name = createdAtFieldName,
-      typeIdentifier = TypeIdentifier.DateTime,
-      isRequired = true,
-      isList = false,
-      isUnique = false,
-      isHidden = true,
-      enum = None,
-      defaultValue = None,
-      relationName = None,
-      relationSide = None,
-      manifestation = None,
-      behaviour = None
-    )
-  }
-
-  private val updatedAtField: FieldTemplate = {
-    FieldTemplate(
-      name = updatedAtFieldName,
-      typeIdentifier = TypeIdentifier.DateTime,
-      isRequired = true,
-      isList = false,
-      isUnique = false,
-      isHidden = true,
-      enum = None,
-      defaultValue = None,
-      relationName = None,
-      relationSide = None,
-      manifestation = None,
-      behaviour = None
-    )
-  }
-
-  private val idField: FieldTemplate = {
-    FieldTemplate(
-      name = idFieldName,
-      typeIdentifier = TypeIdentifier.Cuid,
-      isRequired = true,
-      isList = false,
-      isUnique = true,
-      isHidden = true,
-      enum = None,
-      defaultValue = None,
-      relationName = None,
-      relationSide = None,
-      manifestation = None,
-      behaviour = Some(IdBehaviour(strategy = FieldBehaviour.IdStrategy.Auto))
-    )
-  }
-
-  val embeddedIdField: FieldTemplate = {
-    FieldTemplate(
-      name = embeddedIdFieldName,
-      typeIdentifier = TypeIdentifier.Cuid,
-      isRequired = true,
-      isList = false,
-      isUnique = true,
-      isHidden = true,
-      enum = None,
-      defaultValue = None,
-      relationName = None,
-      relationSide = None,
-      manifestation = None,
-      behaviour = Some(IdBehaviour(strategy = FieldBehaviour.IdStrategy.Auto))
-    )
-  }
 }

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ trait ImportActions extends BuilderBase with SharedJdbcExtensions {`
`53`	`53`	`e.getUpdateCounts.zipWithIndex`
`54`	`54`	`.filter(element => element._1 == Statement.EXECUTE_FAILED)`
`55`	`55`	`.map { failed =>`
`56`		`- val failedId = argsWithIndex.find(_._2 == failed._2).get._1.rootGC.idField.value`
	`56`	`+ val failedId = argsWithIndex.find(_._2 == failed._2).get._1.rootGC.idFieldByName(mutaction.model.idField_!.name).value`
`57`	`57`	`s"Failure inserting ${model.dbName} with Id: $failedId. Cause: ${removeConnectionInfoFromCause(e.getCause())}"`
`58`	`58`	`}`
`59`	`59`	`.toVector`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ trait NodeActions extends BuilderBase with FilterConditionBuilder with ScalarLis`
`40`	`40`	`rs.next()`
`41`	`41`	`rs.getId(model)`
`42`	`42`	`} else {`
`43`		`- argsWithIdIfNecessary.idField`
	`43`	`+ argsWithIdIfNecessary.idFieldByName(model.idField_!.name)`
`44`	`44`	`}`
`45`	`45`	`}`
`46`	`46`	`)`
Original file line number	Diff line number	Diff line change
`@@ -27,10 +27,6 @@ object RootGCValue {`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`case class RootGCValue(map: Map[String, GCValue]) extends GCValue {`
`30`		`- def idField = map.get("id") match {`
`31`		`- case Some(id) => id.asInstanceOf[IdGCValue]`
`32`		`- case None => sys.error("There was no field with name 'id'.")`
`33`		`- }`
`34`	`30`
`35`	`31`	`def idFieldByName(name: String) = map.get(name) match {`
`36`	`32`	`case Some(id) => id.asInstanceOf[IdGCValue]`
Original file line number	Diff line number	Diff line change
`@@ -81,6 +81,7 @@ class BulkImport(project: Project)(implicit apiDependencies: ApiDependencies) {`
`81`	`81`	`ImportList(ImportIdentifier(typeName, id), field, gcValue)`
`82`	`82`	`}`
`83`	`83`
	`84`	`+ // TODO: do4gr should think about whether Import/Export works with custom names for id fields. I think it does but that should be validated.`
`84`	`85`	`def parseIdGCValue(input: JsObject, model: Model): IdGCValue = model.idField_!.typeIdentifier match {`
`85`	`86`	`case TypeIdentifier.UUID => UuidGCValue.parse_!(input.value("id").as[String])`
`86`	`87`	`case TypeIdentifier.Cuid => StringIdGCValue(input.value("id").as[String])`
Original file line number	Diff line number	Diff line change
`@@ -39,6 +39,7 @@ object InputValueValidation {`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`def scalarFieldsWithValues(model: Model, args: PrismaArgs): List[ScalarField] = {`
`42`		`- model.scalarFields.filter(field => args.getFieldValue(field.name).isDefined).filter(_.name != "id")`
	`42`	`+ // TODO: do4gr should check whether we should indeed filter out the id field here. (bring your own id changed that maybe)`
	`43`	`+ model.scalarFields.filter(field => args.getFieldValue(field.name).isDefined).filter(!_.isId)`
`43`	`44`	`}`
`44`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ object FilteredResolver extends SangriaExtensions {`
`26`	`26`
`27`	`27`	`def removeTopLevelIdFilter(element: Filter) =`
`28`	`28`	`element match {`
`29`		`- case e: ScalarFilter => e.field.name != "id"`
	`29`	`+ case e: ScalarFilter => !e.field.isId`
`30`	`30`	`case _ => true`
`31`	`31`	`}`
`32`	`32`