Skip to content

Conversation

@hank-cp
Copy link
Contributor

@hank-cp hank-cp commented Aug 11, 2016

Class loaded by different classLoader are not equal, so using Class as map key will have mare than one entry in that map. Use type name instead.

The failed test case could be reproduce when work with Spring-Boot 1.3+ and DevTools.

…r are not equal, so using Class as map key will have mare than one entry in that map. Use type name instead.
@bartoszwalacik
Copy link
Member

thanks for the PR, does current impl (class as a map key) causes problems or this is only about optimization (limiting the map size) ?

@hank-cp
Copy link
Contributor Author

hank-cp commented Aug 14, 2016

@bartoszwalacik yes, class as map key causing problem. Class loaded by different classloader is not the same instance.

@bartoszwalacik
Copy link
Member

Ok but what kind of problem does it caus in javers?

@hank-cp
Copy link
Contributor Author

hank-cp commented Aug 15, 2016

I capture these screenshots from debugger. You can see that type com.atsoa.modal.engineering.NGGeneralDictionary is added twice with different JaversType.

screen shot 2016-08-15 at 1 47 56 pm

image

My Javers instance is configured as:

    JaversUtil.getCommonJaversBuilder()
                    .registerValueTypeAdapter(new NGGeneralDictionaryTypeAdapter()))
                    .build();

@bartoszwalacik
Copy link
Member

@hank-cp I understand the issue, you have two classes loaded by two classloaders. These classes have the same name but might have different properties.
I dont understand why it is not OK for you. Why you want to map two (possibly different) classes to the same JaVers type?

@hank-cp
Copy link
Contributor Author

hank-cp commented Aug 16, 2016

@bartoszwalacik I don't mean to add the second JaversType (with EntityType). I guess it's added automatically when Javers, when it's trying to find the best matching JaveryType for my class. In this case, Javers couldn't found my custom ValueTypeAdapter by the key, which is loaded by another classloader, so it wired EntityType for the best guess, thus cause my problem.

@bartoszwalacik
Copy link
Member

ok, got it

@bartoszwalacik
Copy link
Member

now I'm on holiday, I will merge this in next week

@bartoszwalacik bartoszwalacik merged commit 2fed7ef into javers:master Aug 28, 2016
@bartoszwalacik
Copy link
Member

one fix
1a9e2ea
(we don't want to break Java7 compatibility)

@bartoszwalacik
Copy link
Member

released in 2.1.2

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

Successfully merging this pull request may close these issues.

2 participants