From 2e18b3f98cedb60ce6af1e7d0c9b1165ad0a5a1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20GIL?= Date: Thu, 26 Mar 2020 16:19:12 -0300 Subject: [PATCH] Verifying that target is not transient before accessing its id --- ...rksWithTargetProxiesConstraintsSpec.groovy | 131 ++++++++++++++++++ .../builtin/UniqueConstraint.groovy | 8 +- 2 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 grails-datastore-gorm-tck/src/main/groovy/grails/gorm/tests/BuiltinUniqueConstraintWorksWithTargetProxiesConstraintsSpec.groovy diff --git a/grails-datastore-gorm-tck/src/main/groovy/grails/gorm/tests/BuiltinUniqueConstraintWorksWithTargetProxiesConstraintsSpec.groovy b/grails-datastore-gorm-tck/src/main/groovy/grails/gorm/tests/BuiltinUniqueConstraintWorksWithTargetProxiesConstraintsSpec.groovy new file mode 100644 index 00000000000..7bd8b4318a8 --- /dev/null +++ b/grails-datastore-gorm-tck/src/main/groovy/grails/gorm/tests/BuiltinUniqueConstraintWorksWithTargetProxiesConstraintsSpec.groovy @@ -0,0 +1,131 @@ +package grails.gorm.tests + +import grails.persistence.Entity +import org.grails.datastore.mapping.proxy.EntityProxy + +class BuiltinUniqueConstraintWorksWithTargetProxiesConstraintsSpec extends GormDatastoreSpec { + + void "modified domains object works as expected"() { + given: "I have a Domain OBJECT" + final Referenced object = new Referenced(name: "object").save(failOnError: true) + assert !(object instanceof EntityProxy) + + and: "I have another Domain OBJECT with the same name" + final Referenced another = new Referenced(name: "object") + assert !(object instanceof EntityProxy) + + when: "I try to validate the another object" + another.validate() + + then: "another should have an error on name because it is duplicated" + another.hasErrors() + another.errors.hasFieldErrors("name") + another.errors.getFieldError("name").codes.contains("unique.name") + + cleanup: + object?.delete(flush: true) + } + + void "modified Referenced domains object works as expected"() { + given: "I have a Domain OBJECT" + final Referenced object = new Referenced(name: "object").save(failOnError: true) + assert !(object instanceof EntityProxy) + and: "a root referencing it" + final Root parent = new Root(ref: object).save(failOnError: true) + assert !(parent instanceof EntityProxy) + + and: "I have another Domain OBJECT with the same name" + final Referenced anotherReferenced = new Referenced(name: "object") + assert !(object instanceof EntityProxy) + final Root anotherRoot = new Root(ref: anotherReferenced) + assert !(parent instanceof EntityProxy) + + when: "I try to validate the another object" + anotherRoot.validate() + + then: "another should have an error on name because it is duplicated" + anotherRoot.hasErrors() + anotherRoot.errors.hasFieldErrors("Referenced.name") + anotherRoot.errors.getFieldError("Referenced.name").codes.contains("unique.name") + + cleanup: + object?.delete(flush: true) + parent?.delete(flush: true) + } + + void "unmodified Referenced proxies object doesnt fail unique constraint checking"() { + given: "I have a Domain OBJECT" + Long ReferencedId, parentId + Root.withNewSession { + Root.withNewTransaction { + final Referenced object = new Referenced(name: "object").save(failOnError: true) + final Root parent = new Root(ref: object).save(failOnError: true) + + ReferencedId = object.id + parentId = parent.id + } + } + and: + int tries = 20 + while (!Root.exists(parentId) && !Referenced.exists(ReferencedId) && tries-- > 0) { + sleep(50) + } + + and: "I access the parent, forcing the Referenced to be initialized" + + def parent = Root.findAll()[0] + assert parent.ref instanceof EntityProxy + parent.ref.name == "object" + + when: "I try to validate the the parent (which then tries to validate the Referenced)" + parent.validate() + + then: "parent.Referenced should not have any errors!" + !parent.hasErrors() + !parent.errors.hasFieldErrors("Referenced.name") + !parent.errors.getFieldError("Referenced.name").codes.contains("unique.name") + + cleanup: + + Root.withNewSession { + Root.withNewTransaction { + Referenced.get(ReferencedId)?.delete(flush: true) + Root.get(parentId)?.delete(flush: true) + } + } + tries = 20 + while (Root.exists(parentId) && Referenced.exists(ReferencedId) && tries-- > 0) { + sleep(50) + } + } + + @Override + List getDomainClasses() { + [Referenced, Root] + } +} + +@Entity +class Referenced { + + String name + + static constraints = { + name nullable: false, unique: true + } +} + +@Entity +class Root { + + Referenced ref + + static constraints = { + ref nullable: false + } + + static mapping = { + ref lazy: false + id generator: 'snowflake' + } +} \ No newline at end of file diff --git a/grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/validation/constraints/builtin/UniqueConstraint.groovy b/grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/validation/constraints/builtin/UniqueConstraint.groovy index 93287685f0f..1ea7c5e9e19 100644 --- a/grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/validation/constraints/builtin/UniqueConstraint.groovy +++ b/grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/validation/constraints/builtin/UniqueConstraint.groovy @@ -138,7 +138,13 @@ class UniqueConstraint extends AbstractConstraint { if (shouldValidate) { def existingId = detachedCriteria.get() if (existingId != null) { - def targetId = reflector.getIdentifier(target) + // We are merely verifying that the object is not transient here + def targetId + if (proxyHandler.isProxy(target)) { + targetId = proxyHandler.getIdentifier(target) + } else { + targetId = reflector.getIdentifier(target) + } if (targetId != existingId) { def args = [constraintPropertyName, constraintOwningClass, propertyValue] as Object[] rejectValue(target, errors, "unique", args, getDefaultMessage("default.not.unique.message"))