A few questions on our favorites handling and backup

[sonora]

Reviewing our code, could anybody please assist clarifying the following (I can offer to fix most, but don't want to fix phantoms):

1. Is it intentional that since 1fd5171 the favorites backup files (in OsmAnd's backup folder) are just named <date_time>.gpx.zip, without any "favorites" prefix.

2. I notice since 1fd5171 we now only keep 10 files history, it used to be 20?

3. Is is intentional that after (Separate favs #15735) the backup files are simply suffixed with ".zip", but are not actually zipped any more?

4. We have BACKUP_MAX_PER_DAY = 2, but I find more backup files for some days on my device. Could be a bug, or just me testing and switching between builds. We still mean to enforce this, right?

5. Backward compatibility: I think this is intentional, but just to be sure: In builds prior to (Separate favs #15735) you could place/update a file with the OLD naming convention "favourites.gpx" in OsmAnd's data folder root, and it was automatically evaluated (favorites imported). Now this only works (with both old and new name convention) if you place the file in the new "favorites" subfolder. I think this behavior is intentional. Do we think this breaks anything? I.e. do we also need to look in root for such a legacy favourites.gpx to consider?

6. I am uncertain about method AppInitializer.restoreBackupForFavoritesFiles(): Is it still needed? If yes, does it need adjusting to the recent changes of now using a favorites folder and separate files?

7. A technicality: We maintain two separate constants for the same thing? IndexConstants.GPX_FILE_EXT and FavoritesFileHelper.GPX_FILE_EXT ?

8. Another technicality: The names of our classes and methods have always used a mixture of "Favorite" (e.g. FavoritesSettingsItem.java) and "Favourite" (e.g. FavouritesFileHelper.java), which makes debugging more tedious than it could be. Is it worth or too risky to change all code to using "Favorites" only?

[vshcherb]

1. Yes cause now it's a special folder
2. Yes 10 is enough, they are pretty big
3. If it is and reproducible then it's a bug! please open and describe how to check
4. don't know, not critical
5. no, i don't think it's something to fix. This is import only folder, so it's not a primary source of information i.e. if you place empty files favorites won't be deleted
6. probably not, I'm not sure what it means anymore
7. Exactly, IndexConstants.GPX_FILE_EXT should be used
8. FavoritesSettingsItem is only for import/export they are actually different entities they could reference FavoritesSettingsItem -> Favourite but that's not my level understanding for now

[sonora]

Ok, thanks! For me to track:

• (1) OK!
• (2) Ok, we limit to 3 per day, so you normally have at least 4 days' worth, sounds fine.
• (3) We had a regression here, I fixed in 5ed46af
• (4) Code checked, looks fine. It limits to BACKUP_MAX_PER_DAY + 1 = 3 per day, that's what had confused me.
  • There is one small issue, but it's a rare use case: We use the file's modify date to delete all excess files of the same day. If you copied your 10 backup files manually, e.g. by a restore operation or manual transfer after an SD card replacement, all 10 files may end up with the same (current) modify date and OsmAnd will delete 7 of them on the next occasion.
• (5) OK!
• (6) Checked and tested - found obsolete. (e896b8a)
• (7) All constants cleaned up on the occasion. (1e914db)
• (8) Maybe you misunderstand: Today we have ~47 classes with "Favorites" in their name. But we also have ~15 (other) classes with "Favourites". For methods, it will likely be similarly mixed. My question is if there is enough value to rename all those containing "Favourites" so that when searching favorite-relevant code you do not always have to look up two different strings.
  • I just notice how widespread the dualism is in our code: 194 code results "favorites" (https://github.com/osmandapp/OsmAnd/search?q=Favorite&type=code) vs. 106 code results "favourites" (https://github.com/osmandapp/OsmAnd/search?q=Favourite&type=code).
  • Probably not be worth the effort.

EDIT: @vshcherb All issues resolved/fixed, I will leave it at that. Thx!