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

Added DBRefUnproxyObjectAccessHook #738

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@joha78

joha78 commented Nov 11, 2018

This extension solves problems with MongoDBs DBRef(lazy=true) annotation

Jörg Hälker
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 12, 2018

nice, but we need some test, I will write some

@joha78

This comment has been minimized.

joha78 commented Nov 14, 2018

Thank you very much. I thought about doing it myself but I am not very familiar with groovy

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 15, 2018

There is a problem with dependency management here.
Adding dep to spring-data-mongo brakes JaVers, because it downgrades mong-db-driver version from 3.6 to 3.4
Look like we need to upgrade the whole javers to Spring 5 and Sprint Boot 2

@joha78

This comment has been minimized.

joha78 commented Nov 15, 2018

I found another problem. If I use the new DBRefUnproxyObjectAccessHook I get some strange things in MongoDB jv_snapshots:

nbrDest: "EE4344D",
toPlanet: Object {
           entity: "de.spacesystems.Planet"
           cdoId: Object {
                timestamp: 1498562478
                machineIdentifier:14306734
                processIdentifier: 26005
                counter: 13820548
           },
handlerStatus: "ConfirmedByHandler"

Obviously something is wrong with the cdoID ?!
Any suggestions what could be wrong?

Update:
Found out that it is persisting an InstanceId, investigating further
Shouldn't it be using the LocalID the DBRefUnproxyObjectAccessHook has extracted and not the GlobalID?
I think it has something todo with @ShallowReference. => NO, the CdoID is still wrong persisted if I remove the @ShallowReferences

I pause my investigations now.

@joha78

This comment has been minimized.

joha78 commented Nov 15, 2018

There is a problem with dependency management here.
Adding dep to spring-data-mongo brakes JaVers, because it downgrades mong-db-driver version from 3.6 to 3.4
Look like we need to upgrade the whole javers to Spring 5 and Sprint Boot 2

I am not sure why this can be. I just reused an existing dependency in the new sub-project of javers.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 15, 2018

It looks like ObjectId, this problem is described here https://javers.org/documentation/repository-configuration/#custom-json-serialization and it's another issue to add out-of-the box support for mongo ObjectId's in JaVers.
In this issue we are focusing on DBRefs and making JaVers work with spring-data-mongo. I'll try to upgrade JaVers to Spring 5 and Sprint Boot 2.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 18, 2018

@joha78 I wrote a sketch of a test here https://github.com/javers/javers/pull/742/files
I can't find docs about DBRefs proxies. How does it works? How can I observe uninitialized DBRefs proxy?

@joha78

This comment has been minimized.

joha78 commented Nov 21, 2018

I am not exactly sure about your question. As far as I know you can't observe uninitialized DBRefs proxy (interface LazyLoadingProxy). But if you call LazyLoadingProxy.getTarget() you get the fresh loaded Entity. Have a look at DefaultDbRefResolver to see how it works. Behind the scenes it is using a LazyLoadingInterceptor.

@joha78

This comment has been minimized.

joha78 commented Nov 21, 2018

I don't know how to update the javers/joha78-master branch.
With this changes its possible to prove that there is a proxy and that it works:

@SpringBootTest(classes = [TestApplication])
@ActiveProfiles("test")
//@Ignore //TODO
class DBRefUnproxyObjectAccessHookTest extends Specification {

    @Autowired
    MyDummyEntityRepository dummyEntityRepository

    @Autowired
    MyDummyRefEntityRepository dummyRefEntityRepository

    def "should unproxy and commit DBRef to JaVers"() {
        given:

        def refEntity = new MyDummyRefEntity()
        refEntity.setName("Bert")
        refEntity = dummyRefEntityRepository.save(refEntity);

        def author = new MyDummyEntity();
        author.setRefEntity(refEntity)
        author = dummyEntityRepository.save(author)

        def loaded = dummyEntityRepository.findById(author.getId()).get()
        println loaded
        println loaded.class
        println loaded.refEntity
        println loaded.refEntity.class

        assert loaded.refEntity instanceof LazyLoadingProxy

        def realObj = ((LazyLoadingProxy) loaded.refEntity).getTarget()

        assert realObj instanceof MyDummyRefEntity
        assert "Bert".equals(realObj.name)
        assert "Bert".equals(loaded.refEntity.name)
    }
}
public class MyDummyEntity {
    private String id;

    @DBRef(lazy = true)
    private MyDummyRefEntity refEntity;

    @Id
    public String getId() {
        return id;
    }

    public void setId(String id) {
        this.id = id;
    }

    public MyDummyRefEntity getRefEntity() {
        return refEntity;
    }

    public void setRefEntity(MyDummyRefEntity refEntity) {
        this.refEntity = refEntity;
    }
}
@JaversSpringDataAuditable
public interface MyDummyEntityRepository extends CrudRepository<MyDummyEntity, String>{
}
public class MyDummyRefEntity {

    private  String id;

    private String name;

    @Id
    public String getId() {
        return id;
    }

    public void setId(String id) {
        this.id = id;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
}
public interface MyDummyRefEntityRepository extends CrudRepository<MyDummyRefEntity, String>{
}
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 23, 2018

ok will check it. I want to have a test and see how it fails without the new hook and how it passes with the hook.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 23, 2018

btw, could you please provide the second PR to javers/joha78-master branch with this test?

@joha78

This comment has been minimized.

joha78 commented Nov 26, 2018

btw, could you please provide the second PR to javers/joha78-master branch with this test?

I did, you should have a pull request.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 28, 2018

ok, I merged your second PR and made few fixes. Now I think the hook works and it's added to the starter.

see https://github.com/javers/javers/pull/742/files
is it what you need?

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 29, 2018

merged from another branch, will be released as 4.0.0-RC5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment