Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[android] Unify cache and offline databases #4383

Merged
merged 1 commit into from
Mar 21, 2016

Conversation

zugaldia
Copy link
Member

Per #4362, we're now unifying the ambient and the offline databases in one location. This PR:

  1. Moves them both to a permanent storage folder.
  2. Deletes any previous ambient database file.

/cc: @jfirebaugh @1ec5

@1ec5
Copy link
Contributor

1ec5 commented Mar 18, 2016

Noting for posterity that, with this change, the Android SDK continues to have multiple DefaultFileSource instances, whereas the iOS SDK now maintains only one DefaultFileSource instance as of #4372. As far as I know, the main consequence is that the limit of 20 simultaneous connections is enforced per map view / offline manager rather than per process.

@jfirebaugh
Copy link
Contributor

On the iOS side, #4377 ensures that the beta.1 offline database is preserved. It looks like this PR will not take that measure, since it changes the file name used by OfflineManager from "mbgl-offline.db" to "mbgl-cache.db".

Options:

  • Keep the PR as-is, and let people know that they'll need to recreate offline regions in beta.2. (If we do this, we should ensure that "mbgl-offline.db" is removed.)
  • Restore "mbgl-offline.db" as the name used by OfflineManager, and change the name here to match.
  • Keep using "mbgl-cache.db", but write a migration that moves "mbgl-offline.db" to "mbgl-cache.db".

I like the idea of preserving existing beta.1 offline databases in one of the latter two ways, but I don't feel super strongly about it. What do you think?

@zugaldia
Copy link
Member Author

Restore "mbgl-offline.db" as the name used by OfflineManager, and change the name here to match.

@jfirebaugh Good catch. Tackled by f5b750b.

@Override
public void run() {
try {
String path = context.getCacheDir().getAbsolutePath() + File.separator + DATABASE_NAME;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now DATABASE_NAME needs to be replaced with "mbgl-cache.db" here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed: 7fcfc2c.

@jfirebaugh
Copy link
Contributor

👍

Tested and confirmed that an existing offline region is preserved, and then used when there is not network access.

@zugaldia zugaldia merged commit 682b404 into release-ios-3.2.0-android-4.0.0 Mar 21, 2016
@zugaldia zugaldia deleted the 4362-android-unify branch March 21, 2016 18:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants