-
Notifications
You must be signed in to change notification settings - Fork 583
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
Remember instances for annotated entities too #254
Conversation
Conflicts: library/build.gradle sugartestapp/src/androidTest/java/me/jivimberg/android/sugartestapp/robolectric/SugarRecordTest.java sugartestapp/src/main/java/me/jivimberg/android/sugartestapp/model/annotated/Car.java
Conflicts: library/src/com/orm/SugarRecord.java
I'm going to take a look at this tomorrow. This is a pretty big bug that should be fixed! |
The fix is pretty straightforward. The only thing is that I had to add the Guava dependency to create the concurrent map with weak keys to avoid a memory leak. |
I'm close to merging this in, but it's failing an important case: Model model = new Model("Test");
long id = SugarRecord.save(model);
Model query = SugarRecord.findById(Model.class, 1);
assertEquals(id, new Long(query)); The expectation is that the queried model retains the |
@jivimberg do you have any ideia of how to remove guava dependency? Another problem is that MapMaker is deprecated (https://github.com/google/guava/wiki/MapMakerMigration) is there no way to fix this issue without using a Map? |
It seams that the MapMaker call can be replaced by a CacheBuilder which still has the option of specifying weakKeys |
@jivimberg the problem with CacheBuilder is that it has guava dependency that make the size of this lib bigger, can we just add part of guava ? |
Oh! I see. I guess we could easily take a look at the CacheBuilder implementation. The key thing here is to have weak reference for the keys in the map so we don't run into a memory leak |
This solves issues: #215 and #243 as well as #185.
Basically I'm using a Map with weak keys to keep track of the row ID for a given object instance. In the past the id field was used for the entities that extend SugarRecord, but that's not possible for annotated entities.