[ios, macos] Offline regions merge. #12791

fabian-guerra · 2018-08-31T23:50:04Z

1ec5

Please update the iOS and macOS changelogs to mention the new method specifically. If possible, also add unit tests for this functionality.

1ec5 · 2018-09-01T06:55:01Z

platform/darwin/src/MGLOfflineStorage.h

@@ -174,6 +178,10 @@ MGL_EXPORT
 */
 @property (class, nonatomic, readonly) MGLOfflineStorage *sharedOfflineStorage;

+#pragma mark - Merging Offline Regions
+
+- (void)mergeOfflineRegions:(NSString *)databaseURL withCompletionHandler:(nullable MGLOfflineMergeCompletionHandler)completion;


Decide whether the method will take an NSURL (as implied by databaseURL) or a string.

A more Cocoa name for this method would be either -addContentsOfFile:withCompletionHandler: or -addContentsOfURL:withCompletionHandler:. The primary effect of this method is to add offline packs; resolving conflicts (merging) is merely an edge case that can be documented in a documentation comment.

1ec5 · 2018-09-06T22:04:21Z

platform/darwin/src/MGLOfflineStorage.h

+ @param completion The completion handler to call once the file has been added.
+ This handler is executed asynchronously on the main queue.
+ */
+- (void)addContentesOfFile:(NSString *)filePath withCompletionHandler:(nullable MGLOfflinePacksAdditionCompletionHandler)completion;


Typo: contents (×4).

1ec5 · 2018-09-06T22:04:47Z

platform/darwin/src/MGLOfflineStorage.h

+ @param completion The completion handler to call once the file has been added.
+ This handler is executed asynchronously on the main queue.
+ */
+- (void)addContentesOfFile:(NSString *)filePath withCompletionHandler:(nullable MGLOfflinePacksAdditionCompletionHandler)completion;


For Swift, we should also add -addContentsOfURL:withCompletionHandler: alongside this method. Swift strongly prefers using URLs for file paths.

1ec5 · 2018-09-06T22:08:56Z

platform/darwin/src/MGLOfflineStorage.mm

+
+- (void)addContentesOfFile:(NSString *)filePath withCompletionHandler:(MGLOfflinePacksAdditionCompletionHandler)completion {
+    NSFileManager *fileManager = [NSFileManager defaultManager];
+    //NSAssert([fileManager isWritableFileAtPath:filePath], @"The file %@ must be writable.", filePath);


NSAssert() is only called on debug builds and is intended to flag errors in our part in developing the SDK. Use NSException directly for any errors on the application developer’s part, or call the completion handler with an NSError if this error is out of the application developer’s control.

1ec5 · 2018-09-06T22:09:16Z

platform/ios/CHANGELOG.md

@@ -17,6 +17,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT

 * Added the `MGLShapeOfflineRegion` class for creating an offline pack that covers an arbitrary shape. ([#11447](https://github.com/mapbox/mapbox-gl-native/pull/11447))
 * Fixed crashes when offline storage encountered certain SQLite errors. ([#12224](https://github.com/mapbox/mapbox-gl-native/pull/12224))
+* Added an `-[MGLOfflineStorage addContentesOfFile:withCompletionHandler:]` method to add the content of a database into the main offline database. ([#12791](https://github.com/mapbox/mapbox-gl-native/pull/12791))


to add pregenerated offline packs to offline storage

1ec5 · 2018-09-06T22:10:32Z

platform/darwin/src/MGLOfflineStorage.h

+#pragma mark - Adding Contents of File
+
+/**
+ Adds the contents of the file path.


Adds the offline packs located at the given file path to offline storage.

1ec5 · 2018-09-06T22:11:46Z

platform/darwin/src/MGLOfflineStorage.h

+
+ Once the content is added, then the `completion` block is executed with the
+ packs passed in.
+


Also say something about the file format if possible, and that this method is used to preload pregenerated offline packs that are bundled with the application or downloaded manually by the application.

1ec5 · 2018-09-06T22:12:59Z

platform/darwin/test/MGLOfflineStorageTests.mm

+
+    MGLOfflineStorage *os = [MGLOfflineStorage sharedOfflineStorage];
+    [os addContentesOfFile:filePath withCompletionHandler:^(NSArray<MGLOfflinePack *> * _Nonnull packs, NSError * _Nullable error) {
+        XCTAssertNotNil(packs, @"Adding contents to a file should return the newly added packages.");


Also assert that the packs are included in MGLOfflineStorage.packs by this point.

1ec5 · 2018-09-06T22:13:49Z

platform/darwin/test/MGLOfflineStorageTests.mm

+                                                                    nameKey: name,
+                                                                    }];
+
+    [[MGLOfflineStorage sharedOfflineStorage] addPackForRegion:region withContext:context completionHandler:^(MGLOfflinePack * _Nullable completionHandlerPack, NSError * _Nullable error) {


This test seems duplicative of existing tests of -addPackForRegion:withContext:completionHandler:.

I need to set the offline region to barcelona because the file I am adding is from that region and need to test that the offline database is ok before merging.

1ec5 · 2018-09-06T22:14:33Z

platform/darwin/test/MGLOfflineStorageTests.mm

+        XCTAssertNotNil(packs, @"Adding contents to a file should return the newly added packages.");
+        XCTAssertNil(error, @"Adding contents to a file should not return an error.");
+    }];
+}


Also test the cases where the path points to something other than an offline database or points to a read-only file.

fabian-guerra · 2018-09-07T23:29:32Z

I changed the original block signature from:

(^MGLOfflinePacksAdditionCompletionHandler)(NSArray<MGLOfflinePack *> *packs, NSError * _Nullable error)

to

(^MGLOfflinePacksAdditionCompletionHandler)(NSError * _Nullable error)

Because the resulting packs contains packs that were updated or added and didn't make sense to pass this information to the user as the packs with this changes should be reloaded again.

The current implementation has a completion handler for use cases where the UI needs to responds to when the file has merged into the current offline database.

1ec5 · 2018-09-08T00:00:53Z

platform/darwin/src/MGLOfflineStorage.h

+ @param error Contains a pointer to an error object (if any) indicating why the
+ pack could not be created or added.
+ */
+typedef void (^MGLOfflinePacksAdditionCompletionHandler)(NSError * _Nullable error);


This completion handler is named too similarly to MGLOfflinePackAdditionCompletionHandler. How about MGLMultipleOfflinePackAdditionCompletionHandler or MGLBatchedOfflinePackAdditionCompletionHandler?

1ec5 · 2018-09-08T00:03:01Z

platform/darwin/src/MGLOfflineStorage.h

+ @param error Contains a pointer to an error object (if any) indicating why the
+ pack could not be created or added.
+ */
+typedef void (^MGLOfflinePacksAdditionCompletionHandler)(NSError * _Nullable error);


Because the resulting packs contains packs that were updated or added and didn't make sense to pass this information to the user as the packs with this changes should be reloaded again.

Delegate methods always accept the delegator as the first parameter, in order to provide the delegate with enough context to distinguish between multiple delegators. Likewise, completion handlers should always include context about the operation that has completed. That way, the caller can pass the same block into multiple call sites, as a form of functional programming.

Likewise, completion handlers should always include context about the operation that has completed.

For this case specifically I don't consider it would be useful (although I agree with you about the context) pass the packs that were added/updated, because those are not the same packs after we reload the packs. For the test case the contents of the barcelona.db returns 4 packages which you can't explicitly know which were updated/added. Since there is no relation between the packs that are stored and the ones that this api returns as updated/added I considered this to mislead the developer to think it would be possible to perform any action on the returned packs.

What about the mbgl::OfflineRegions that DefaultFileSource::mergeOfflineRegions() passes into its callback? Correct me if I’m wrong, but these are the regions in the merged database that originate from the sideloaded database:

mapbox-gl-native/platform/default/mbgl/storage/offline_database.cpp

Lines 691 to 694 in 6e171a6

mapbox::sqlite::Query queryRegions{ getStatement(

"SELECT r.id, r.definition, r.description "

"FROM side.regions sr "

"JOIN regions r ON sr.definition = r.definition") };

For the purposes of distinguishing between multiple uses of a callback, I think it would additionally be useful for this completion handler to accept the file URL or file path of the sideloaded database.

/cc @asheemmamoowala

these are the regions in the merged database that originate from the sideloaded database:

@1ec5 - these regions may be newly added or updated regions. The completion handler can return the set of packs in the SDK binding, but I don't think that is useful enough by itself. For Observers of the MGLOfflineStorage.packs property will not be able to tell which packs were added vs updated. If the internal array is simply reloaded, again they observers would lose context of what mutations have occurred to the array.

Making KVO compliant changes to the packs property seems like a valuable change. I think that would look like:

adding any new packs to the packs property, similarly to how addPack does it: [[strongSelf mutableArrayValueForKey:@"packs"] addObject:pack];

Matching region ids returned from core to existing packs and invalidating those OR replacing them with new objects using [NSMutableArray replaceObjectAtIndex] or some other KVO compliant method.

1ec5 · 2018-09-08T00:03:38Z

platform/darwin/src/MGLOfflineStorage.h

+ The file must be a valid offline region database bundled with the application
+ or downloaded separately.
+
+ Refer to this <a href="https://www.mapbox.com/ios-sdk/maps/examples/offline-pack/">example</a>


This example shows how to add a single offline pack programmatically, not from a file.

I couldn't find anything better, perhaps just write how it would be possible to create additional offline databases?

We don’t have any documentation on creating offline databases, other than this informal wiki page. Let’s leave out this paragraph and add it back in once an example or official documentation is available.

Remember to remove this paragraph until we have an example ready about this specific workflow.

/ref #12791 (comment)

1ec5 · 2018-09-08T00:04:52Z

platform/darwin/src/MGLOfflineStorage.h

+- (void)addContentsOfFile:(NSString *)filePath withCompletionHandler:(nullable MGLOfflinePacksAdditionCompletionHandler)completion;
+
+/**
+ Adds the offline packs located at the given URL to offline storage.


It’s important to emphasize that this URL is a file URL; otherwise, developers may think they can directly pass in the URL to a Web API. That would be very cool but rather out of scope for this PR.

Adds offline packs located at the given file URL to offline storage.

1ec5 · 2018-09-08T00:25:10Z

platform/ios/CHANGELOG.md

@@ -17,6 +17,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT

 * Added the `MGLShapeOfflineRegion` class for creating an offline pack that covers an arbitrary shape. ([#11447](https://github.com/mapbox/mapbox-gl-native/pull/11447))
 * Fixed crashes when offline storage encountered certain SQLite errors. ([#12224](https://github.com/mapbox/mapbox-gl-native/pull/12224))
+* Added an `-[MGLOfflineStorage addContentesOfFile:withCompletionHandler:]` method to add pregenerated offline packs to offline storage. ([#12791](https://github.com/mapbox/mapbox-gl-native/pull/12791))


Typo: “contents”, not “contentes”. Also, there are two new methods now.

1ec5 · 2018-09-11T19:07:11Z

platform/darwin/src/MGLOfflineStorage.mm

+    [self _addContentsOfFile:filePath withCompletionHandler:^(NSError * _Nullable error) {
+        if (error && completion) {
+            completion(error);
+        } else {


If there’s an error but no completion block is provided, the code below invalidates all the packs, which seems wasteful. Perhaps this line should say else if (!error)?

1ec5 · 2018-09-11T19:11:39Z

platform/darwin/src/MGLOfflineStorage.mm

+}
+
+- (void)addContentsOfURL:(NSURL *)fileURL withCompletionHandler:(MGLBatchedOfflinePackAdditionCompletionHandler)completion {
+    [self addContentsOfFile:fileURL.absoluteString withCompletionHandler:completion];


-[NSURL absoluteString] returns a string beginning with file:///, not a file path. Instead, check that the scheme is file (raising an exception otherwise) and pass in path.

Add a test of this method that passes in a valid file URL.

1ec5 · 2018-09-11T19:19:32Z

platform/darwin/src/MGLOfflineStorage.h

+ @param error Contains a pointer to an error object (if any) indicating why the
+ pack could not be created or added.
+ */
+typedef void (^MGLOfflinePacksAdditionCompletionHandler)(NSError * _Nullable error);


What about the mbgl::OfflineRegions that DefaultFileSource::mergeOfflineRegions() passes into its callback? Correct me if I’m wrong, but these are the regions in the merged database that originate from the sideloaded database:

mapbox-gl-native/platform/default/mbgl/storage/offline_database.cpp

Lines 691 to 694 in 6e171a6

mapbox::sqlite::Query queryRegions{ getStatement(

"SELECT r.id, r.definition, r.description "

"FROM side.regions sr "

"JOIN regions r ON sr.definition = r.definition") };

For the purposes of distinguishing between multiple uses of a callback, I think it would additionally be useful for this completion handler to accept the file URL or file path of the sideloaded database.

/cc @asheemmamoowala

1ec5 · 2018-09-11T19:21:04Z

platform/darwin/src/MGLOfflineStorage.mm

+        }
+        if (completion) {
+            dispatch_async(dispatch_get_main_queue(), [&, completion, error, packs](void) {
+                completion(error);


At this point, we can pass in both filePath, and packs. Otherwise, all the work above to build up packs is for naught.

1ec5 · 2018-09-11T19:23:19Z

platform/darwin/src/MGLOfflineStorage.mm

+- (void)addContentsOfFile:(NSString *)filePath withCompletionHandler:(MGLBatchedOfflinePackAdditionCompletionHandler)completion {
+    NSFileManager *fileManager = [NSFileManager defaultManager];
+    if (![fileManager isWritableFileAtPath:filePath]) {
+        [NSException raise:NSInvalidArgumentException format:@"The file path must be writable"];


Include the file path in the exception message.

1ec5 · 2018-09-11T19:25:59Z

platform/darwin/src/MGLOfflineStorage.mm

+        NSMutableArray *packs;
+        if (!result) {
+            error = [NSError errorWithDomain:MGLErrorDomain code:-1 userInfo:@{
+                                                                               NSLocalizedDescriptionKey: @(mbgl::util::toString(result.error()).c_str()),


The error strings coming from mbgl are fairly low-level and specific, such as:

mapbox-gl-native/platform/default/mbgl/storage/offline_database.cpp

Line 661 in 6e171a6

throw std::runtime_error("Merge database has incorrect user_version");

Since a localized description is meant for display to the user, it should be something higher-level like “Unable to add offline packs from the file at…”. (Remember to make this string localizable.) The error string from mbgl should go in NSLocalizedFailureReasonKey.

Most of the errors coming from offline_database are low level I may be wrong but I think that we rely on the developer to display the proper error message after receiving an NSError object.

That may be, but the convention is still for the localized description to be as user-friendly as possible, including localization, and for the failure reason to contain a more technical explanation. See MGLMapViewImpl::onDidFailLoadingMap() for inspiration.

1ec5 · 2018-09-11T19:27:50Z

platform/darwin/test/MGLOfflineStorageTests.mm

+        XCTestExpectation *fileAdditionCompletionHandlerExpectation = [self expectationWithDescription:@"add database content completion handler"];
+        MGLOfflineStorage *os = [MGLOfflineStorage sharedOfflineStorage];
+        [os addContentsOfFile:filePath withCompletionHandler:^(NSError * _Nullable error) {
+            XCTAssertNil(error, @"Adding contents to a file should not return an error.");


Also assert that new packs are included in MGLOfflineStorage.packs by this point.

/ref #12791 (comment)

1ec5 · 2018-09-11T19:28:34Z

platform/darwin/test/MGLOfflineStorageTests.mm

+        }];
+        [self waitForExpectationsWithTimeout:2 handler:nil];
+    }
+    // File non existent


Also test passing in a read-only file.

1ec5

It’s pretty cool that you’ve found a way to avoid invalidating packs indiscriminately. I didn’t realize I sent you on a side quest with #12791 (comment). I was thinking we’d only need to wrap and return the packs mbgl passes into the callback and invalidate all the existing packs, but your changes make me realize that most of the code assumes there’s only one Objective-C object wrapping each of the mbgl::OfflineRegions represented in MGLOfflineStorage.packs, so this complex operation does turn out to be necessary.

1ec5 · 2018-09-13T05:48:26Z

platform/darwin/src/MGLOfflineStorage.h

+/**
+ A block to be called once the contents of a file are copied into the current packs.
+
+ @param fileURL Contains the URL to the file packs used.


“Contains” is unnecessary indirection. It’s only necessary for inout pointers, usually of type NSError ** but not NSError *. Also, refer to “offline packs” rather than “file packs”.

The file URL of the offline database containing the offline packs that were copied.

1ec5 · 2018-09-13T06:20:14Z

platform/darwin/src/MGLOfflineStorage.mm

+            for (MGLOfflinePack *pack in strongSelf.packs) {
+                [packsDictionary setObject:pack forKey:@(pack.mbglOfflineRegion->getID())];
+                [packsIndex setObject:@(index) forKey:@(pack.mbglOfflineRegion->getID())];
+                index++;


This is a good opportunity to use -[NSArray enumerateObjectsUsingBlock:] instead of fast enumeration. The block accepts an index as a parameter.

1ec5 · 2018-09-13T10:26:38Z

platform/darwin/src/MGLOfflineStorage.mm

+            MGLOfflineStorage *strongSelf = weakSelf;
+            for (MGLOfflinePack *pack in strongSelf.packs) {
+                [packsDictionary setObject:pack forKey:@(pack.mbglOfflineRegion->getID())];
+                [packsIndex setObject:@(index) forKey:@(pack.mbglOfflineRegion->getID())];


We can avoid this redundant dictionary with the following approach:

// Map IDs to new packs. NSMutableDictionary *packsByIdentifier = [NSMutableDictionary dictionary]; for (MGLOfflinePack *pack in packs) { [packsByIdentifier setObject:pack forKey:@(pack.mbglOfflineRegion->getID())]; } id mutablePacks = [strongSelf mutableArrayValueForKey:@"packs"]; [strongSelf.packs enumerateObjectsUsingBlock:(^)(MGLOfflinePack *pack, NSUInteger idx, BOOL *stop) { MGLOfflinePack *newPack = packsByIdentifier[@(pack.mbglOfflineRegion->getID())]; if (newPack) { [newPack invalidate]; [mutablePacks replaceObjectAtIndex:idx withObject:pack]; // By the end of this enumeration, packsByIdentifier only contains newly-added packs. packsByIdentifier[@(pack.mbglOfflineRegion->getID())] = nil; } }]; // Add any remaining packs to the end of the array. [mutablePacks addObjectsFromArray:packsByIdentifier.allValues];

To reduce the amount of work done via KVC, we could build up an NSIndexSet and an array of replacement packs (which happens to be packs), then pass both objects into -replaceObjectsAtIndexes:withObjects:. A further optimization would be to change packsByIdentifier into an std::unordered_map.

To reduce the amount of work done via KVC

Even though calling -replaceObjectsAtIndexes:withObjects: may be less elegant than the code above, it may be desirable because it prevents unnecessary thrashing on the part of key-value observers that would occur if each pack is replaced one by one.

1ec5 · 2018-09-13T17:30:33Z

platform/default/mbgl/storage/offline_database.cpp

-            "JOIN regions r ON sr.definition = r.definition") };
+                                                         "SELECT DISTINCT r.id, r.definition, r.description "
+                                                         "FROM side.regions sr "
+                                                         "JOIN regions r ON sr.definition = r.definition  AND sr.description IS r.description") };


I defer to @mapbox/gl-core about this change.

Looks good, but let's remove the added indentation to make it easier to read.

…ded.

…ation.

…letionHandler.

…ter the test.

1ec5

A couple nits about documentation, but one edge case I think we’ll need to address before merging.

1ec5 · 2018-09-17T18:55:52Z

platform/darwin/src/MGLOfflineStorage.h

+ The file must be a valid offline region database bundled with the application
+ or downloaded separately.
+
+ Refer to this <a href="https://www.mapbox.com/ios-sdk/maps/examples/offline-pack/">example</a>


Remember to remove this paragraph until we have an example ready about this specific workflow.

/ref #12791 (comment)

1ec5 · 2018-09-17T18:57:04Z

platform/darwin/src/MGLOfflineStorage.h

+ that were copied.
+ @param packs Contains an array of all known offline packs, or `nil` if there was
+ an error creating or adding the pack.
+ @param error Contains a pointer to an error object (if any) indicating why the


These two parameters don’t need “contains”, either, because they’re just simple pointers, not double pointers.

/ref #12791 (comment)

1ec5 · 2018-09-17T19:10:09Z

platform/darwin/src/MGLOfflineStorage.mm

+                [mutablePacks replaceObjectsAtIndexes:replaceIndexSet withObjects:packs];
+            }
+
+            [mutablePacks addObjectsFromArray:packsByIdentifier.allValues];


This line is intended to add any new offline packs to MGLOfflineStorage.packs. In that case, packs will be longer than replaceIndexSet in the call to -replaceObjectsAtIndexes:withObjects:, which will probably trigger an exception. (We should assert that the two have the same length beforehand.) Even if it doesn’t, this line would add those same packs to the array redundantly.

Instead of this line, is it possible to add more indices to the end of replaceIndexSet to account for those extra packs? If not, then we’d need to build up a separate array of packs that are getting replaced in the -enumerateObjectsUsingBlock: call above, then group the calls to -replaceObjectsAtIndexes:withObjects: and -addObjectsFromArray: to avoid unnecessary KVO notifications.

This is a good scenario to test, by the way. XCTest has conveniences for testing KVO notifications.

1ec5

One more request, and then it’s good to merge.

1ec5 · 2018-09-17T22:05:54Z

platform/darwin/src/MGLOfflineStorage.mm

+                [mutablePacks replaceObjectsAtIndexes:replaceIndexSet withObjects:replacePacksArray];
+            }
+
+            [mutablePacks addObjectsFromArray:packsByIdentifier.allValues];


I just realized MGLOfflineStorage.packs is documented to be ordered:

mapbox-gl-native/platform/darwin/src/MGLOfflineStorage.h

Line 192 in f3efa62

An array of all known offline packs, in the order in which they were created.

NSDictionary.allValues has undefined order, so unfortunately we need to build up a second array alongside replacePacksArray.

fabian-guerra self-assigned this Aug 31, 2018

1ec5 reviewed Sep 1, 2018

View reviewed changes

1ec5 mentioned this pull request Sep 1, 2018

Copyedit iOS, macOS changelogs #12792

Merged

fabian-guerra force-pushed the fabian-offline-merge-12693 branch from 9059c3a to 111604d Compare September 5, 2018 21:49

fabian-guerra added feature iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Sep 5, 2018

fabian-guerra added this to the ios-v4.4.0-frappé milestone Sep 5, 2018

fabian-guerra force-pushed the fabian-offline-merge-12693 branch from 111604d to 31b9dcc Compare September 6, 2018 22:02

1ec5 reviewed Sep 6, 2018

View reviewed changes

fabian-guerra force-pushed the fabian-offline-merge-12693 branch from aa31557 to 6b016b6 Compare September 7, 2018 23:46

1ec5 reviewed Sep 8, 2018

View reviewed changes

1ec5 added the offline label Sep 8, 2018

fabian-guerra force-pushed the fabian-offline-merge-12693 branch from b6c6924 to 10f3647 Compare September 10, 2018 21:43

1ec5 reviewed Sep 11, 2018

View reviewed changes

fabian-guerra force-pushed the fabian-offline-merge-12693 branch from e9489ec to 513da89 Compare September 12, 2018 23:45

1ec5 reviewed Sep 13, 2018

View reviewed changes

friedbunny modified the milestones: ios-v4.4.0-frappé, ios-v4.5.0 Sep 13, 2018

fabian-guerra added 8 commits September 13, 2018 15:42

[ios, macos] Add offline regions merge.

68e26db

[ios, macos] Update changelogs.

3753b48

[ios, macos] Fix refreshing the offline packs after new content is ad…

448a56e

…ded.

[ios, macos] Update MGLOfflineStorage's add contents of file document…

15773e7

…ation.

[ios, macos] Add MGLOfflineStorage test cases for adding file contents.

dbe3df8

[ios, macos] Add offline merge test database.

f3da3b8

[ios, macos] Add packs parameter to MGLBatchedOfflinePackAdditionComp…

b755da9

…letionHandler.

[core] Fix a query mege duplication.

fb1aa4d

[ios, macos] Add localization strings.

b5e50e1

fabian-guerra force-pushed the fabian-offline-merge-12693 branch from c55e38a to b5e50e1 Compare September 14, 2018 00:01

[core] Fix query readability.

2545c81

fabian-guerra force-pushed the fabian-offline-merge-12693 branch 17 times, most recently from 2812a11 to 230f2d2 Compare September 15, 2018 06:36

[ios, macos] Fix MGLOfflineStorageTest, cleanup the cache database af…

a5142d6

…ter the test.

fabian-guerra force-pushed the fabian-offline-merge-12693 branch from 230f2d2 to a5142d6 Compare September 17, 2018 18:21

1ec5 reviewed Sep 17, 2018

View reviewed changes

[ios, macos] Fix offline packs, updating triggers a crash.

2478c59

fabian-guerra force-pushed the fabian-offline-merge-12693 branch from 7b3b687 to 2478c59 Compare September 17, 2018 21:50

1ec5 approved these changes Sep 17, 2018

View reviewed changes

fabian-guerra merged commit a938c3e into master Sep 17, 2018

fabian-guerra deleted the fabian-offline-merge-12693 branch September 17, 2018 22:31

friedbunny mentioned this pull request Sep 19, 2018

[build, ios] Add canary Xcode 10 CircleCI job #12911

Merged

friedbunny mentioned this pull request Jun 18, 2019

[ios] Simplify -[MGLOfflineStorageTests testAddFileContent] #14948

Merged


		Once the content is added, then the `completion` block is executed with the
		packs passed in.

	mapbox::sqlite::Query queryRegions{ getStatement(
	"SELECT r.id, r.definition, r.description "
	"FROM side.regions sr "
	"JOIN regions r ON sr.definition = r.definition") };

[ios, macos] Offline regions merge. #12791

[ios, macos] Offline regions merge. #12791

Conversation

fabian-guerra commented Aug 31, 2018 • edited Loading

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabian-guerra commented Sep 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Sep 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabian-guerra commented Aug 31, 2018 •

edited

Loading

1ec5 Sep 11, 2018 •

edited

Loading

1ec5 Sep 11, 2018 •

edited

Loading

1ec5 Sep 13, 2018 •

edited

Loading