update uv_zip and add tests

11 files changed, 144 insertions, 32 deletions

diff --git a/gyp/asset-fs.gypi b/gyp/asset-fs.gypi
index 0606eaf7f8..f8ec0e3558 100644
--- a/gyp/asset-fs.gypi
+++ b/gyp/asset-fs.gypi
@@ -25,6 +25,9 @@
         'libraries': [
           '<@(uv_static_libs)',
         ],
+        'defines': [
+          '-DMBGL_ASSET_FS'
+        ],
       },
 
       'conditions': [
@@ -37,6 +40,20 @@
         }],
       ],
 
+      'direct_dependent_settings': {
+        'conditions': [
+          ['OS == "mac"', {
+            'xcode_settings': {
+              'OTHER_CFLAGS': [ '<@(defines)' ],
+              'OTHER_CPLUSPLUSFLAGS': [ '<@(defines)' ],
+            }
+          }, {
+            'cflags': [ '<@(defines)' ],
+            'cflags_cc': [ '<@(defines)' ],
+          }]
+        ],
+      },
+
       'link_settings': {
         'conditions': [
           ['OS == "mac"', {
diff --git a/gyp/asset-zip.gypi b/gyp/asset-zip.gypi
index 25df2a3d8b..5c57aa18b5 100644
--- a/gyp/asset-zip.gypi
+++ b/gyp/asset-zip.gypi
@@ -33,6 +33,9 @@
           '<@(uv_static_libs)',
           '<@(zip_static_libs)',
         ],
+        'defines': [
+          '-DMBGL_ASSET_ZIP'
+        ],
       },
 
       'conditions': [
@@ -47,6 +50,20 @@
         }],
       ],
 
+      'direct_dependent_settings': {
+        'conditions': [
+          ['OS == "mac"', {
+            'xcode_settings': {
+              'OTHER_CFLAGS': [ '<@(defines)' ],
+              'OTHER_CPLUSPLUSFLAGS': [ '<@(defines)' ],
+            }
+          }, {
+            'cflags': [ '<@(defines)' ],
+            'cflags_cc': [ '<@(defines)' ],
+          }]
+        ],
+      },
+
       'link_settings': {
         'conditions': [
           ['OS == "mac"', {
diff --git a/include/mbgl/storage/default/shared_request_base.hpp b/include/mbgl/storage/default/shared_request_base.hpp
index a0f3e8d4e3..7745bef2b5 100644
--- a/include/mbgl/storage/default/shared_request_base.hpp
+++ b/include/mbgl/storage/default/shared_request_base.hpp
@@ -78,8 +78,10 @@ protected:
 public:
     const Resource resource;
 
-private:
+protected:
     DefaultFileSource *source = nullptr;
+
+private:
     std::set<Request *> observers;
 };
 
diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp
index 21048e99e5..86e2414041 100644
--- a/include/mbgl/storage/default_file_source.hpp
+++ b/include/mbgl/storage/default_file_source.hpp
@@ -18,8 +18,8 @@ class SharedRequestBase;
 
 class DefaultFileSource : public FileSource {
 public:
-    DefaultFileSource(FileCache *cache);
-    DefaultFileSource(FileCache *cache, uv_loop_t *loop);
+    DefaultFileSource(FileCache *cache, const std::string &root = "");
+    DefaultFileSource(FileCache *cache, uv_loop_t *loop, const std::string &root = "");
     ~DefaultFileSource();
 
     Request *request(const Resource &resource, uv_loop_t *loop, Callback callback);
@@ -30,6 +30,9 @@ public:
     void notify(SharedRequestBase *sharedRequest, const std::set<Request *> &observers,
                 std::shared_ptr<const Response> response, FileCache::Hint hint);
 
+public:
+    const std::string assetRoot;
+
 private:
     struct ActionDispatcher;
     struct AddRequestAction;
diff --git a/platform/default/asset_request_fs.cpp b/platform/default/asset_request_fs.cpp
index 24856e520b..1e32ee3135 100644
--- a/platform/default/asset_request_fs.cpp
+++ b/platform/default/asset_request_fs.cpp
@@ -3,7 +3,6 @@
 #include <mbgl/util/std.hpp>
 #include <mbgl/util/util.hpp>
 #include <mbgl/util/uv.hpp>
-#include <mbgl/platform/platform.hpp>
 
 #include <uv.h>
 
@@ -63,7 +62,7 @@ AssetRequestImpl::AssetRequestImpl(AssetRequest *request_, uv_loop_t *loop) : re
         path = url.substr(8);
     } else {
         // This is a relative path. Prefix with the application root.
-        path = platform::assetRoot() + "/" + url.substr(8);
+        path = request->source->getAssetRoot() + "/" + url.substr(8);
     }
 
     uv_fs_open(loop, &req, path.c_str(), O_RDONLY, S_IRUSR, fileOpened);
diff --git a/platform/default/asset_request_zip.cpp b/platform/default/asset_request_zip.cpp
index 9a5f598bb6..fd27c4b959 100644
--- a/platform/default/asset_request_zip.cpp
+++ b/platform/default/asset_request_zip.cpp
@@ -2,8 +2,8 @@
 #include <mbgl/storage/default/thread_context.hpp>
 #include <mbgl/android/jni.hpp>
 #include <mbgl/storage/response.hpp>
+#include <mbgl/platform/log.hpp>
 #include <mbgl/util/std.hpp>
-#include <mbgl/platform/platform.hpp>
 
 #include "uv_zip.h"
 
@@ -26,12 +26,12 @@ public:
     AssetZipContext(uv_loop_t *loop);
     ~AssetZipContext();
 
-    uv_zip_t *getHandle();
-    void returnHandle(uv_zip_t *zip);
+    uv_zip_t *getHandle(const std::string &path);
+    void returnHandle(const std::string &path, uv_zip_t *zip);
 
 private:
     // A list of resuable uv_zip handles to avoid creating and destroying them all the time.
-    std::forward_list<uv_zip_t *> handles;
+    std::map<std::string, std::forward_list<uv_zip_t *>> handles;
 };
 
 // -------------------------------------------------------------------------------------------------
@@ -42,28 +42,30 @@ template<> pthread_once_t ThreadContext<AssetZipContext>::once = PTHREAD_ONCE_IN
 AssetZipContext::AssetZipContext(uv_loop_t *loop_) : ThreadContext(loop_) {
 }
 
-uv_zip_t *AssetZipContext::getHandle() {
-    if (!handles.empty()) {
-        auto zip = handles.front();
-        handles.pop_front();
+uv_zip_t *AssetZipContext::getHandle(const std::string &path) {
+    auto &list = handles[path];
+    if (!list.empty()) {
+        auto zip = list.front();
+        list.pop_front();
         return zip;
     } else {
         return nullptr;
     }
 }
 
-void AssetZipContext::returnHandle(uv_zip_t *zip) {
-    uv_zip_cleanup(zip);
-    handles.push_front(zip);
+void AssetZipContext::returnHandle(const std::string &path, uv_zip_t *zip) {
+    handles[path].push_front(zip);
 }
 
 AssetZipContext::~AssetZipContext() {
     // Close all zip handles
-    for (auto zip : handles) {
-        uv_zip_discard(loop, zip, [](uv_zip_t *zip_) {
-            uv_zip_cleanup(zip_);
-            delete zip_;
-        });
+    for (auto &list : handles) {
+        for (auto zip : list.second) {
+            uv_zip_discard(loop, zip, [](uv_zip_t *zip_) {
+                uv_zip_cleanup(zip_);
+                delete zip_;
+            });
+        }
     }
     handles.clear();
 }
@@ -82,6 +84,7 @@ public:
 private:
     AssetZipContext &context;
     AssetRequest *request = nullptr;
+    const std::string root;
     const std::string path;
     std::unique_ptr<Response> response;
     uv_buf_t buffer;
@@ -111,8 +114,9 @@ AssetRequestImpl::~AssetRequestImpl() {
 AssetRequestImpl::AssetRequestImpl(AssetRequest *request_, uv_loop_t *loop)
     : context(*AssetZipContext::Get(loop)),
       request(request_),
-      path(request->resource.url.substr(8)) {
-    auto zip = context.getHandle();
+      root(request->source->assetRoot),
+      path(std::string { "assets/" } + request->resource.url.substr(8)) {
+    auto zip = context.getHandle(root);
     if (zip) {
         archiveOpened(zip);
     } else {
@@ -124,9 +128,12 @@ void AssetRequestImpl::openZipArchive() {
     uv_fs_t *req = new uv_fs_t();
     req->data = this;
 
+    assert(request);
+    assert(request->source);
+
     // We're using uv_fs_open first to obtain a file descriptor. Then, uv_zip_fdopen will operate
     // on a read-only file.
-    uv_fs_open(context.loop, req, platform::assetRoot().c_str(), O_RDONLY, S_IRUSR, [](uv_fs_t *fsReq) {
+    uv_fs_open(context.loop, req, root.c_str(), O_RDONLY, S_IRUSR, [](uv_fs_t *fsReq) {
         if (fsReq->result < 0) {
             auto impl = reinterpret_cast<AssetRequestImpl *>(fsReq->data);
             impl->notifyError(uv::getFileRequestError(fsReq));
@@ -191,6 +198,10 @@ void AssetRequestImpl::fileStated(uv_zip_t *zip) {
             response->modified = zip->stat->mtime;
         }
 
+        if (zip->stat->valid & ZIP_STAT_INDEX) {
+            response->etag = std::to_string(zip->stat->index);
+        }
+
         uv_zip_fopen(context.loop, zip, path.c_str(), 0, INVOKE_MEMBER(fileOpened));
     }
 }
@@ -239,7 +250,7 @@ void AssetRequestImpl::fileClosed(uv_zip_t *zip) {
 void AssetRequestImpl::cleanup(uv_zip_t *zip) {
     MBGL_VERIFY_THREAD(tid);
 
-    context.returnHandle(zip);
+    context.returnHandle(root, zip);
     delete this;
 }
 
diff --git a/platform/default/uv_zip.c b/platform/default/uv_zip.c
index 7cb3c516d6..51bd15e8e2 100644
--- a/platform/default/uv_zip.c
+++ b/platform/default/uv_zip.c
@@ -172,6 +172,7 @@ int uv_zip_open(uv_loop_t* loop, uv_zip_t *zip, const char *path, zip_flags_t fl
     assert(zip);
     assert(path);
     assert(strlen(path));
+    zip->result = 0;
     zip->path = path;
     zip->flags = flags;
     zip->cb = cb;
@@ -182,6 +183,7 @@ int uv_zip_fdopen(uv_loop_t* loop, uv_zip_t *zip, uv_file fd, int flags, uv_zip_
     assert(loop);
     assert(zip);
     assert(fd);
+    zip->result = 0;
     zip->path = malloc(sizeof(uv_file));
     *(uv_file *)zip->path = fd;
     zip->flags = flags;
@@ -194,6 +196,8 @@ int uv_zip_stat(uv_loop_t* loop, uv_zip_t *zip, const char *fname, zip_flags_t f
     assert(zip);
     assert(fname);
     assert(strlen(fname));
+    assert(zip->archive);
+    zip->result = 0;
     zip->path = fname;
     zip->flags = flags;
     zip->cb = cb;
@@ -205,8 +209,11 @@ int uv_zip_fopen(uv_loop_t* loop, uv_zip_t *zip, const char *fname, zip_flags_t
     assert(zip);
     assert(fname);
     assert(strlen(fname));
+    assert(zip->archive);
+    zip->result = 0;
     zip->path = fname;
     zip->flags = flags;
+    zip->file = NULL;
     zip->cb = cb;
     return uv_queue_work(loop, &zip->work, uv__zip_work_fopen, uv__zip_after_work);
 }
@@ -215,6 +222,8 @@ int uv_zip_fclose(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_zip_
     assert(loop);
     assert(zip);
     assert(file);
+    assert(zip->archive);
+    zip->result = 0;
     zip->file = file;
     zip->cb = cb;
     return uv_queue_work(loop, &zip->work, uv__zip_work_fclose, uv__zip_after_work);
@@ -225,6 +234,8 @@ int uv_zip_fread(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_buf_t
     assert(zip);
     assert(file);
     assert(buf);
+    assert(zip->archive);
+    zip->result = 0;
     zip->file = file;
     zip->buf = buf;
     zip->cb = cb;
@@ -234,6 +245,8 @@ int uv_zip_fread(uv_loop_t* loop, uv_zip_t *zip, struct zip_file *file, uv_buf_t
 int uv_zip_discard(uv_loop_t* loop, uv_zip_t *zip, uv_zip_cb cb) {
     assert(loop);
     assert(zip);
+    assert(!zip->file);
+    zip->result = 0;
     zip->cb = cb;
     return uv_queue_work(loop, &zip->work, uv__zip_work_discard, uv__zip_after_work);
 }
diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp
index e40aaff649..6d27adbe0e 100644
--- a/src/mbgl/storage/default_file_source.cpp
+++ b/src/mbgl/storage/default_file_source.cpp
@@ -4,6 +4,7 @@
 #include <mbgl/storage/default/http_request.hpp>
 
 #include <mbgl/storage/response.hpp>
+#include <mbgl/platform/platform.hpp>
 
 #include <mbgl/util/async_queue.hpp>
 #include <mbgl/util/util.hpp>
@@ -49,8 +50,9 @@ struct DefaultFileSource::StopAction {
 };
 
 
-DefaultFileSource::DefaultFileSource(FileCache *cache_)
-    : loop(uv_loop_new()),
+DefaultFileSource::DefaultFileSource(FileCache *cache_, const std::string &root)
+    : assetRoot(root.empty() ? platform::assetRoot() : root),
+      loop(uv_loop_new()),
       cache(cache_),
       queue(new Queue(loop, [this](Action &action) {
           mapbox::util::apply_visitor(ActionDispatcher{*this}, action);
@@ -63,8 +65,9 @@ DefaultFileSource::DefaultFileSource(FileCache *cache_)
       }) {
 }
 
-DefaultFileSource::DefaultFileSource(FileCache *cache_, uv_loop_t *loop_)
-    : loop(loop_),
+DefaultFileSource::DefaultFileSource(FileCache *cache_, uv_loop_t *loop_, const std::string &root)
+    : assetRoot(root.empty() ? platform::assetRoot() : root),
+      loop(loop_),
       cache(cache_),
       queue(new Queue(loop, [this](Action &action) {
           mapbox::util::apply_visitor(ActionDispatcher{*this}, action);
diff --git a/test/fixtures/storage/assets.zip b/test/fixtures/storage/assets.zip
new file mode 100644
index 0000000000..3127798145
--- /dev/null
+++ b/test/fixtures/storage/assets.zip
diff --git a/test/storage/directory_reading.cpp b/test/storage/directory_reading.cpp
index bf472b5c79..3ee4dd1721 100644
--- a/test/storage/directory_reading.cpp
+++ b/test/storage/directory_reading.cpp
@@ -4,12 +4,16 @@
 
 #include <mbgl/storage/default_file_source.hpp>
 
-TEST_F(Storage, ReadDirectory) {
+TEST_F(Storage, AssetReadDirectory) {
     SCOPED_TEST(ReadDirectory)
 
     using namespace mbgl;
 
+#ifdef MBGL_ASSET_ZIP
+    DefaultFileSource fs(nullptr, uv_default_loop(), "test/fixtures/storage/assets.zip");
+#else
     DefaultFileSource fs(nullptr, uv_default_loop());
+#endif
 
     fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" }, uv_default_loop(), [&](const Response &res) {
         EXPECT_EQ(Response::Error, res.status);
@@ -17,7 +21,11 @@ TEST_F(Storage, ReadDirectory) {
         EXPECT_EQ(0, res.expires);
         EXPECT_EQ(0, res.modified);
         EXPECT_EQ("", res.etag);
+#ifdef MBGL_ASSET_ZIP
+        EXPECT_EQ("No such file", res.message);
+#elif MBGL_ASSET_FS
         EXPECT_EQ("illegal operation on a directory", res.message);
+#endif
         ReadDirectory.finish();
     });
 
diff --git a/test/storage/file_reading.cpp b/test/storage/file_reading.cpp
index 622eb2b8e8..7e14fdcb15 100644
--- a/test/storage/file_reading.cpp
+++ b/test/storage/file_reading.cpp
@@ -5,12 +5,16 @@
 #include <mbgl/storage/default_file_source.hpp>
 #include <mbgl/platform/platform.hpp>
 
-TEST_F(Storage, EmptyFile) {
+TEST_F(Storage, AssetEmptyFile) {
     SCOPED_TEST(EmptyFile)
 
     using namespace mbgl;
 
+#ifdef MBGL_ASSET_ZIP
+    DefaultFileSource fs(nullptr, uv_default_loop(), "test/fixtures/storage/assets.zip");
+#else
     DefaultFileSource fs(nullptr, uv_default_loop());
+#endif
 
     fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" },
                uv_default_loop(),
@@ -27,12 +31,43 @@ TEST_F(Storage, EmptyFile) {
     uv_run(uv_default_loop(), UV_RUN_DEFAULT);
 }
 
-TEST_F(Storage, NonExistentFile) {
+TEST_F(Storage, AssetNonEmptyFile) {
+    SCOPED_TEST(NonEmptyFile)
+
+    using namespace mbgl;
+
+#ifdef MBGL_ASSET_ZIP
+    DefaultFileSource fs(nullptr, uv_default_loop(), "test/fixtures/storage/assets.zip");
+#else
+    DefaultFileSource fs(nullptr, uv_default_loop());
+#endif
+
+    fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" },
+               uv_default_loop(),
+               [&](const Response &res) {
+        EXPECT_EQ(Response::Successful, res.status);
+        EXPECT_EQ(16ul, res.data.size());
+        EXPECT_EQ(0, res.expires);
+        EXPECT_LT(1420000000, res.modified);
+        EXPECT_NE("", res.etag);
+        EXPECT_EQ("", res.message);
+        EXPECT_EQ("content is here\n", res.data);
+        NonEmptyFile.finish();
+    });
+
+    uv_run(uv_default_loop(), UV_RUN_DEFAULT);
+}
+
+TEST_F(Storage, AssetNonExistentFile) {
     SCOPED_TEST(NonExistentFile)
 
     using namespace mbgl;
 
+#ifdef MBGL_ASSET_ZIP
+    DefaultFileSource fs(nullptr, uv_default_loop(), "test/fixtures/storage/assets.zip");
+#else
     DefaultFileSource fs(nullptr, uv_default_loop());
+#endif
 
     fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" },
                uv_default_loop(),
@@ -42,7 +77,11 @@ TEST_F(Storage, NonExistentFile) {
         EXPECT_EQ(0, res.expires);
         EXPECT_EQ(0, res.modified);
         EXPECT_EQ("", res.etag);
+#ifdef MBGL_ASSET_ZIP
+        EXPECT_EQ("No such file", res.message);
+#elif MBGL_ASSET_FS
         EXPECT_EQ("no such file or directory", res.message);
+#endif
         NonExistentFile.finish();
     });