Grails
  1. Grails
  2. GRAILS-8860

Service Unit Test with @TestFor using a domain class with an embedded non-domain class results in an exception

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.1, 2.0.3
    • Fix Version/s: 2.1.5, 2.2.2, 2.3-M1
    • Component/s: Testing
    • Labels:
      None
    • Patch Submitted:
      Yes
    • Patch attached:
      Yes

      Description

      I'm a bit at a loss to isolate the following situation in a isolated testcase. The testcase I tried to create runs without any problems

      But the situation causes us problems in migrating our project from Grails 1.3.7 to 2.0.x.
      I've got a domain class that contains an embedded Amount object. This Amount class is not a Domain class. Just a simple wrapper around BigDecimal and contained in a model package.

      A simplified setup of the domain class:

      package domain
      
      import model.Amount
      
      class Record {
      
          String name
          Amount amount
      
          static embedded = ['amount']
      
          static constraints = {
              amount  ()
              name    ()
          }
      }
      

      This works fine, however a new test (Using the @TestFor and @Mock annotations) results in the following stacktrace, which suggests that the Amount object is treated as a GORM object?

      Invalid property 'id' of bean class [my.package.Amount]: Bean property 'id' is not writable or has an invalid setter method. Does the parameter type of the setter match the return type of the getter?
      
      org.springframework.beans.NotWritablePropertyException: Invalid property 'id' of bean class [my.package.Amount]: Bean property 'id' is not writable or has an invalid setter method. Does the parameter type of the setter match the return type of the getter?
      	at org.grails.datastore.mapping.engine.EntityAccess.setProperty(EntityAccess.java:70)
      	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister$NativeEntryModifyingEntityAccess.setProperty(NativeEntryEntityPersister.java:1372)
      	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.refreshObjectStateFromNativeEntry(NativeEntryEntityPersister.java:361)
      	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.refreshObjectStateFromNativeEntry(NativeEntryEntityPersister.java:408)
      	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.createObjectFromNativeEntry(NativeEntryEntityPersister.java:338)
      	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.retrieveEntity(NativeEntryEntityPersister.java:305)
      	at org.grails.datastore.mapping.engine.EntityPersister.retrieve(EntityPersister.java:158)
      	at org.grails.datastore.mapping.core.AbstractSession.retrieve(AbstractSession.java:513)
      	at org.grails.datastore.mapping.simple.query.SimpleMapQuery.populateQueryResult(SimpleMapQuery.groovy:720)
      	at org.grails.datastore.mapping.simple.query.SimpleMapQuery.executeSubQuery(SimpleMapQuery.groovy:630)
      	at org.grails.datastore.mapping.simple.query.SimpleMapQuery.executeQuery(SimpleMapQuery.groovy:63)
      	at org.grails.datastore.mapping.query.Query.list(Query.java:486)
      	at org.grails.datastore.gorm.finders.AbstractFindByFinder.invokeQuery(AbstractFindByFinder.java:34)
      	at org.grails.datastore.gorm.finders.AbstractFindByFinder$1.doInSession(AbstractFindByFinder.java:26)
      	at org.grails.datastore.mapping.core.DatastoreUtils.execute(DatastoreUtils.java:301)
      	at org.grails.datastore.gorm.finders.AbstractFinder.execute(AbstractFinder.java:40)
      	at org.grails.datastore.gorm.finders.AbstractFindByFinder.doInvokeInternal(AbstractFindByFinder.java:24)
      	at org.grails.datastore.gorm.finders.DynamicFinder.invoke(DynamicFinder.java:151)
      	at org.grails.datastore.gorm.finders.DynamicFinder.invoke(DynamicFinder.java:352)
      	at org.grails.datastore.gorm.GormStaticApi$_methodMissing_closure2.doCall(GormStaticApi.groovy:105)
      	at org.grails.datastore.gorm.validation.constraints.UniqueConstraint$_processValidate_closure1.doCall(UniqueConstraint.groovy:47)
      	at org.grails.datastore.gorm.validation.constraints.UniqueConstraint$_withManualFlushMode_closure2.doCall(UniqueConstraint.groovy:129)
      	at org.grails.datastore.gorm.GormStaticApi$_withSession_closure17.doCall(GormStaticApi.groovy:543)
      	at org.grails.datastore.mapping.core.DatastoreUtils.execute(DatastoreUtils.java:301)
      	at org.grails.datastore.gorm.AbstractDatastoreApi.execute(AbstractDatastoreApi.groovy:34)
      	at org.grails.datastore.gorm.GormStaticApi.withSession(GormStaticApi.groovy:542)
      	at org.grails.datastore.gorm.validation.constraints.UniqueConstraint.withManualFlushMode(UniqueConstraint.groovy:125)
      	at org.grails.datastore.gorm.validation.constraints.UniqueConstraint.processValidate(UniqueConstraint.groovy:39)
      	at org.grails.datastore.mapping.validation.ValidatingEventListener.doValidate(ValidatingEventListener.java:83)
      	at org.grails.datastore.mapping.validation.ValidatingEventListener.beforeInsert(ValidatingEventListener.java:60)
      	at org.grails.datastore.mapping.validation.ValidatingEventListener.onPersistenceEvent(ValidatingEventListener.java:47)
      	at org.grails.datastore.mapping.engine.event.AbstractPersistenceEventListener.onApplicationEvent(AbstractPersistenceEventListener.java:46)
      	at org.grails.datastore.mapping.engine.EntityPersister.cancelInsert(EntityPersister.java:227)
      	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister.executeInsert(NativeEntryEntityPersister.java:1329)
      	at org.grails.datastore.mapping.engine.NativeEntryEntityPersister$1.run(NativeEntryEntityPersister.java:698)
      	at org.grails.datastore.mapping.core.impl.PendingOperationExecution.executePendingOperation(PendingOperationExecution.java:33)
      	at org.grails.datastore.mapping.core.AbstractSession.flushPendingOperations(AbstractSession.java:323)
      	at org.grails.datastore.mapping.core.AbstractSession.flushPendingInserts(AbstractSession.java:315)
      	at org.grails.datastore.mapping.core.AbstractSession.flush(AbstractSession.java:237)
      	at org.grails.datastore.mapping.query.Query.flushBeforeQuery(Query.java:585)
      	at org.grails.datastore.mapping.query.Query.list(Query.java:484)
      	at org.grails.datastore.gorm.finders.AbstractFindByFinder.invokeQuery(AbstractFindByFinder.java:34)
      	at org.grails.datastore.gorm.finders.AbstractFindByFinder$1.doInSession(AbstractFindByFinder.java:26)
      	at org.grails.datastore.mapping.core.DatastoreUtils.execute(DatastoreUtils.java:301)
      	at org.grails.datastore.gorm.finders.AbstractFinder.execute(AbstractFinder.java:40)
      	at org.grails.datastore.gorm.finders.AbstractFindByFinder.doInvokeInternal(AbstractFindByFinder.java:24)
      	at org.grails.datastore.gorm.finders.DynamicFinder.invoke(DynamicFinder.java:151)
      	at org.grails.datastore.gorm.finders.DynamicFinder.invoke(DynamicFinder.java:352)
      	at org.grails.datastore.gorm.GormStaticApi.methodMissing(GormStaticApi.groovy:108)
      	at my.package.MyService.doSomething(MyService.groovy:XXX)

        Activity

        Hide
        Aaron Long added a comment -

        I'm not sure if the getOrCreateEmbeddedEntity is called from other places that I'm not seeing, but my reference search only shows two callers and both are always in a block where (embedded == true and isPersistentEntity() == false).

        It seems like the getOrCreateEmbeddedEntity() could do away with the try/catch and just do:

           protected PersistentEntity getOrCreateEmbeddedEntity(PersistentEntity entity, MappingContext context, Class type) {
                PersistentEntity associatedEntity = context.getPersistentEntity(type.getName());
                if (associatedEntity == null) {
                    // If this is a persistent entity, add and initialize, otherwise
                    // assume it's embedded
                    if( isPersistentEntity(type) ) {
                        if (entity.isExternal()) {
                            associatedEntity = context.addExternalPersistentEntity(type);
                            associatedEntity.initialize();
                        }
                        else {
                            associatedEntity = context.addPersistentEntity(type);
                            associatedEntity.initialize();
                        }
                    }
                    else {
                        PersistentEntity embeddedEntity = context.createEmbeddedEntity(type);
                        embeddedEntity.initialize();
                        return embeddedEntity;
                    }
                }
                else {
                    if(!associatedEntity.isInitialized()) {
                        associatedEntity.initialize();
                    }
                }
                return associatedEntity;
            }
        

        I tested this code and it fixed the problem for me with no other changes. Although, like I said, from the context I see that method is never called where isPersistentEntity == true.

        Show
        Aaron Long added a comment - I'm not sure if the getOrCreateEmbeddedEntity is called from other places that I'm not seeing, but my reference search only shows two callers and both are always in a block where (embedded == true and isPersistentEntity() == false). It seems like the getOrCreateEmbeddedEntity() could do away with the try/catch and just do: protected PersistentEntity getOrCreateEmbeddedEntity(PersistentEntity entity, MappingContext context, Class type) { PersistentEntity associatedEntity = context.getPersistentEntity(type.getName()); if (associatedEntity == null ) { // If this is a persistent entity, add and initialize, otherwise // assume it's embedded if ( isPersistentEntity(type) ) { if (entity.isExternal()) { associatedEntity = context.addExternalPersistentEntity(type); associatedEntity.initialize(); } else { associatedEntity = context.addPersistentEntity(type); associatedEntity.initialize(); } } else { PersistentEntity embeddedEntity = context.createEmbeddedEntity(type); embeddedEntity.initialize(); return embeddedEntity; } } else { if (!associatedEntity.isInitialized()) { associatedEntity.initialize(); } } return associatedEntity; } I tested this code and it fixed the problem for me with no other changes. Although, like I said, from the context I see that method is never called where isPersistentEntity == true.
        Hide
        Aaron Long added a comment -

        Just for the trifecta of fixes , I think the addPersistentEntityInternal could just reverse the order of the registerEntityWithContext() and the initializePersistentEntity() methods so that we initialize first and THEN register. That would avoid the side-effect of a failed initialization.

        Show
        Aaron Long added a comment - Just for the trifecta of fixes , I think the addPersistentEntityInternal could just reverse the order of the registerEntityWithContext() and the initializePersistentEntity() methods so that we initialize first and THEN register. That would avoid the side-effect of a failed initialization.
        Hide
        Aaron Long added a comment -

        Using the version of getOrCreateEmbeddedEntity posted above, I have no instances of the 'id' error and I'm down to only 6 failing of 1334. The remaining failures seem to be unrelated to this specific issue.

        I'll submit a pull request if there are no objections.

        Show
        Aaron Long added a comment - Using the version of getOrCreateEmbeddedEntity posted above, I have no instances of the 'id' error and I'm down to only 6 failing of 1334. The remaining failures seem to be unrelated to this specific issue. I'll submit a pull request if there are no objections.
        Show
        Aaron Long added a comment - https://github.com/SpringSource/grails-data-mapping/pull/99
        Hide
        Graeme Rocher added a comment -

        Pull request merged. Thanks.

        Show
        Graeme Rocher added a comment - Pull request merged. Thanks.

          People

          • Assignee:
            Graeme Rocher
            Reporter:
            Arjan Schaaf
          • Votes:
            5 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development