Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Morphia annotations #62

Closed
ctmay4 opened this issue Nov 26, 2014 · 14 comments
Closed

Morphia annotations #62

ctmay4 opened this issue Nov 26, 2014 · 14 comments

Comments

@ctmay4
Copy link

ctmay4 commented Nov 26, 2014

I am trying to test Javers with entities I save into a MongoDB database. I am using Morphia (https://github.com/mongodb/morphia) annotations to handle the saves and gets. Javers does not seem to like those annotations however. Here is a simple test I put together to show the problem:

import org.javers.core.Javers;
import org.javers.core.JaversBuilder;
import org.javers.core.diff.Diff;
import org.junit.Test;
import org.mongodb.morphia.annotations.Embedded;
import org.mongodb.morphia.annotations.Entity;
import org.mongodb.morphia.annotations.Property;

public class JaVersTest {

    @Embedded
    @Entity(noClassnameStored = true)
    public class StringRange {

        @Property("low")
        private String _low;
        @Property("high")
        private String _high;

        public StringRange() {
        }

        public StringRange(String low, String high) {
            _low = low;
            _high = high;
        }
    }

    @Test
    public void testBasic() {
        Javers javers = JaversBuilder.javers().build();

        StringRange range1 = new StringRange("06", "09");
        StringRange range2 = new StringRange("07", "09");

        Diff diff = javers.compare(range1, range2);
        System.out.println("diff: " + javers.toJson(diff));
    }
}

This gives the following error:

JaversException: ENTITY_WITHOUT_ID JaVers bootstrap error - Class 'com.imsweb.seerapi.lab.staging.JaVersTest$StringRange' has no Id property. Use @Id annotation to mark unique Entity identifier
    at org.javers.core.metamodel.clazz.Entity.findDefaultIdProperty(Entity.java:39)
    at org.javers.core.metamodel.clazz.Entity.<init>(Entity.java:23)
    at org.javers.core.metamodel.clazz.ManagedClassFactory.create(ManagedClassFactory.java:83)
    at org.javers.core.metamodel.clazz.ManagedClassFactory.create(ManagedClassFactory.java:65)
    at org.javers.core.metamodel.clazz.ManagedClassFactory.inferFromAnnotations(ManagedClassFactory.java:45)

However if I comment out only the "@entity" annotation it works:

diff: {
  "changes": [
    {
      "changeType": "ValueChange",
      "globalId": {
        "valueObject": "com.imsweb.seerapi.lab.staging.JaVersTest$StringRange",
        "cdoId": "/"
      },
      "property": "_low",
      "left": "06",
      "right": "07"
    }
  ]
}

I don't have the option of removing my Morphia annotations. Is there any other way to get this to work? Would it be possible to support Morphia annotations like the other ones that are supported?

@pszymczyk
Copy link
Contributor

Hi

Javers correctly recognized Morphia annotations - recognized StringRange as Entity and looking for property anootated as @id.

Currently Javers can recognize objects as:
-Entities - classes with own identity, identified by unique id
-Value Objects - classes without identity

so there is two solutions in this case, you can add property annatated as @id, or remove @entity annotation.

StringRange is typical Value Objects so i would advice to remove @entity annotation, but i don't know is Morphia required @entity annotation to store objects in db?

@ctmay4
Copy link
Author

ctmay4 commented Nov 26, 2014

Yes, Morphia requires @entity to save it in the database. I have a complex tree of objects that I am eventually trying to get audit trail for. I took one of the lower level simple ones to build the test. @id is already used on many of my entities that have identifiers and Javars seems to handle them fine. However the StringRange object above is a Value Object so it does not have an identifier.

So I can't remove @entity and there is no identifier. Is there any other option?

@pszymczyk
Copy link
Contributor

I think we should change the algorithm of recognizing classes as entities, i will try to reproduce your case in a test and resolve this problem

@ctmay4
Copy link
Author

ctmay4 commented Nov 26, 2014

Sounds great. Let me know if you need anything else from me.

@bartoszwalacik
Copy link
Member

Hi Chuck
, interesting case.
So you have @entity ann on your class, required by other framework, But you want Javers treats these class as ValueObject.

Obvioulsly we need priorities in Javers config.
When you explicitly register a class as ValueObject in JaversBuilder it should override types infered from annotations.

Give us a while, we'll check how fast we can add these feature

@ctmay4
Copy link
Author

ctmay4 commented Nov 27, 2014

Thanks. On a side note, Morphia also has its own @id annotation, but I think Jarvers was picking that up correctly. I will test again and let you know.

@ctmay4
Copy link
Author

ctmay4 commented Nov 27, 2014

I take that back. Using the Morphia @id annotation give a strange error about _time. This is the most basic example of using Morphia. All top-level Morphia entities need an @id annotated field.

import org.bson.types.ObjectId;
import org.javers.core.Javers;
import org.javers.core.JaversBuilder;
import org.javers.core.diff.Diff;
import org.junit.Test;
import org.mongodb.morphia.annotations.Entity;
import org.mongodb.morphia.annotations.Id;
import org.mongodb.morphia.annotations.Property;

public class JaVersTest {

    @Entity(noClassnameStored = true)
    public class IdTest {

        @Id
        private ObjectId _id;
        @Property
        private String _name;

        public IdTest() {
        }

        public IdTest(ObjectId id, String name) {
            _id = id;
            _name = name;
        }
    }

    @Test
    public void testBasic() {
        Javers javers = JaversBuilder.javers().build();

        ObjectId id = ObjectId.get();

        IdTest obj1 = new IdTest(id, "Name");
        IdTest obj2 = new IdTest(id, "New Name");

        Diff diff = javers.compare(obj1, obj2);
        System.out.println("diff: " + javers.toJson(diff));
    }
}

Gives this output:

[main] INFO org.javers.core.metamodel.clazz.ManagedClassFactoryModule - using FIELD mappingStyle
[main] INFO org.javers.core.JaversBuilder - using fake InMemoryRepository, register actual implementation via JaversBuilder.registerJaversRepository()
[main] INFO org.javers.core.JaversBuilder - javers instance is up & ready

java.lang.RuntimeException: error getting unwrap from field '_time'
    at org.javers.common.reflection.ReflectionUtil.invokeField(ReflectionUtil.java:187)
    at org.javers.common.reflection.ReflectionUtil.invokeFieldEvenIfPrivate(ReflectionUtil.java:179)
    at org.javers.core.metamodel.property.FieldProperty.get(FieldProperty.java:44)
    at org.javers.core.metamodel.object.CdoWrapper.getPropertyValue(CdoWrapper.java:35)
    at org.javers.core.graph.ObjectNode.getPropertyValue(ObjectNode.java:78)
    at org.javers.core.diff.RealNodePair.isNullOnBothSides(RealNodePair.java:29)
    at org.javers.core.diff.DiffFactory.appendPropertyChanges(DiffFactory.java:108)
    at org.javers.core.diff.DiffFactory.createAndAppendChanges(DiffFactory.java:96)
    at org.javers.core.diff.DiffFactory.create(DiffFactory.java:61)
    at org.javers.core.diff.DiffFactory.compare(DiffFactory.java:54)
    at org.javers.core.Javers.compare(Javers.java:103)

The _time reference is from org.bson.types.ObjectId.

@bartoszwalacik
Copy link
Member

StringRange class is definitely a ValueObject.
We gonna implement mapping priorities. This let you override default annotations behaviour and to map StringRange class as ValueObject

bartoszwalacik added a commit that referenced this issue Nov 27, 2014
@bartoszwalacik
Copy link
Member

You should map StringRange explicitly as ValueObject and Javers will ignore annotations. I've checked this.

Use JaversBuilder.registerValueObject(Class) method:

http://javers.org/javadoc_1.0.0/org/javers/core/JaversBuilder.html#registerValueObject-java.lang.Class-

let me know if this solves your issue

@bartoszwalacik
Copy link
Member

i mean it works well with current JaVers version, try:

javersBuilder.registerValueObject(StringRange.class )

@ctmay4
Copy link
Author

ctmay4 commented Nov 27, 2014

Tested and the StringRange example works fine when I register the value object.

Is the second test example with @id behavior expected?

@bartoszwalacik
Copy link
Member

Not sure which example do you mean?

I've just our updated mapping documentation:
http://javers.org/documentation/configuration/#mapping-configuration

I've reflected your case in the mapping hints section:

In some cases, annotations could be misleading for JaVers. For example Morphia framework uses @entity annotation for each persistent class (even for ValueObjects). This could cause wrong JaVers mapping. As a solution, use explicit mapping with the JaversBuilder methods, as it has the highest priority.

Can we close this issue?

@pszymczyk
Copy link
Contributor

We suggest to close this issue and could you open another one for the second example because it looks like another issue.

@ctmay4
Copy link
Author

ctmay4 commented Nov 28, 2014

Ok..I will close this issue now. I am going to put together a more complete test for the second issue. If I cannot work it out, I will open another issue. Thanks for all your help.

@ctmay4 ctmay4 closed this as completed Nov 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants