Delete images from the database

[ramack]

after #21 we have the local cache database. After the image (or album) list was downloaded via REST we shall update the database to ensure full synchronicity, which essentially means to delete the images and albums which we didn't receive via REST.

[ramack]

Yes, I think that would be the best to synchronize the lists, as fully recreating the cache would actually mean that we don't cache at all :-)

[remi-martin]

I agree, I have some difficulties to set this up because I have never used RxAndroid observers. I'll try to understand their uses first.

[ramack]

I fully understand. This is quite weird when you start with it, I also had trouble to get into it when I have take over the code for the app, but it is quite mighty and seems a really good fit to our needs, so I kept it in...

[remi-martin]

I have too many troubles with ReactiveX, I've made a lot of tests and read a lot of documentation but I'm still unable to make it work as intended. I'm leaving this issue for now

[remi-martin]

Hi @ramack, I realy need your help to understand what's going on in the ImageRepository.getImages() function. I hope you will be available as soon as possible...

[ramack]

ReactiveX is not always intuitive, but if you have questions don't hesitate to ask

[remi-martin]

I have finally understand what blocked me with the getImages() function. Now I'm wondering how to know when to delete the images from the database. In order to know which images are present in the cache and not on the server, I need to know when we have all the images of the server inside the observer to compare them with the db, and I don't know how to do that.

[remi-martin]

I have a little problem with the getImages / getCategories in ImagesRepository / CategoriesRepository because we are updating the cache while retrieving the datas to display them. I think it would be better to synchronize the cache with the server, and then use the datas from the cache / database only, what do you think ?

[coolo]

Yes, I had the same feedback. That cache synchronization needs to be moved out of there and made async

[remi-martin]

Ok @coolo, I'm still unconfortable with the architecture and reactiveX, but I'll try to split those functions. I'm not sure for the async because we need to be sure that the update if finished before we start displaying categories and images.

[coolo]

well, for a mobile app you want to show what's in the cache even without being able to fetch it - e.g. while on a train. And download the index async and refresh if it's mismatches.

And if you miss the images in cache, you want to download/refresh the images as you scroll through them. But also async - right now it's all in one blocking function to sync the album.

[remi-martin]

Hi @ramack, I tried something in the last commits, deletion of images and albums from the server seems to work, but we cannot refresh the display anymore. We have to re-enter the album in order to see the changes. I can probably add an issue for this.

[remi-martin]

Hi @ramack, I've made a branch "bugfix_downloadSize" in which I fixed the error described above and removed (commented) the downloadSize from the settings fragment view only because it was not working properly and it's not a feature we need at this time. I would like you to check if the image deletion work well in this branch before we put that in production.

[ramack]

in the first view it seems to work to remove the images, so great! What I don't fully catch is the removal of the downloadSize option, as in ImageRepository.java:updateImages() the preference is still used. So I don't think it can work as expected on a fresh install. During my trials I had once a crash in line ImageRepository.java:333 due to existingVariant being null

[ramack]

and neither the entires in the Image and ImageVariant tables nor the file in the cache are removed but only the mapping to the category. - Is this intended because we can have the same image ID in several albums? Should we check on the server if the photo was completely deleted or just removed from an album and get rid of the above mentioned entries as well if not used in other albums?

[remi-martin]

Well, I don't know. I just think we should keep the variants just in case, and delete the whole cache when the application is destroyed. For the downloadSize It's just to be sure that the users cannot change it's value, because that's what creates the bugs. I'll check the null variant error later.

Do you think there is any Major bug we need to fix before a release ?

[ramack]

it's at least better than was is out there right now I'd say. I'd even vote for shifting some of the tickets allocated to 1.1.0 to 1.2.0 and get a beta out soon!

[remi-martin]

Right, sounds good. You have settled most of the issues, can you filter them ? Or do you want me to ask plegall for what he considers as urgent for the next release ?

[ramack]

I only picked what seems important but as too much time passed by priority shifts towards a next release soon I'd say. Feel free to shift whatever you think is not so important. @plegall was already in favor of releasing a bugfix version on top 1.0.2 so I guess I would not mind if we redefine the scope of 1.1.0 and we have already some new design and some features that are worth a minor release step.