[core] get rid of Source::getUrlOrTileset virtual

8 files changed, 93 insertions, 61 deletions

diff --git a/include/mbgl/style/source.hpp b/include/mbgl/style/source.hpp
index e664467597..031803b4bf 100644
--- a/include/mbgl/style/source.hpp
+++ b/include/mbgl/style/source.hpp
@@ -98,12 +98,20 @@ public:
     /**
      * @brief Returns resource for loading, if required by this source; returns `nullopt` otherwise.
      *
+     * If already loaded, this will return nullopt as well for Tiled sources (SourceTypeInfo::TileSet == Yes)
+     *
      * @return optional<Resource>
      */
     virtual optional<Resource> getResource() const { return nullopt; }
-    // TODO: Convert below to
-    // `virtual const Tileset* getTileset() const;`
-    virtual const variant<std::string, Tileset>* getURLOrTileset() const { return nullptr; }
+
+    /**
+     * @brief for tiled sources (SourceTypeInfo::TileSet == Yes) this will return the TileSet when
+     * loaded, nullptr in any other case.
+     *
+     * @return Tileset* or nullptr
+     */
+    virtual const Tileset* getTileset() const { return nullptr; }
+
     /**
      * @brief Returns the GeoJSON data, if this source supports GeoJSON format; returns `nullptr` otherwise.
      *
@@ -165,7 +173,7 @@ public:
 protected:
     explicit Source(Immutable<Impl>);
 
-    void serializeUrlOrTileSet(Value&, const variant<std::string, Tileset>*) const;
+    void serializeUrlOrTileSet(Value&, const variant<std::string, Tileset>&) const;
     void serializeTileSet(Value&, const mbgl::Tileset&) const;
 
     virtual Mutable<Impl> createMutable() const noexcept = 0;
diff --git a/include/mbgl/style/sources/raster_source.hpp b/include/mbgl/style/sources/raster_source.hpp
index e49436b971..013c15a74c 100644
--- a/include/mbgl/style/sources/raster_source.hpp
+++ b/include/mbgl/style/sources/raster_source.hpp
@@ -15,7 +15,8 @@ public:
     RasterSource(std::string id, variant<std::string, Tileset> urlOrTileset, uint16_t tileSize);
     ~RasterSource() override;
 
-    const variant<std::string, Tileset>* getURLOrTileset() const final;
+    const Tileset* getTileset() const final;
+    optional<Resource> getResource() const final;
     optional<std::string> getURL() const;
 
     class Impl;
diff --git a/include/mbgl/style/sources/vector_source.hpp b/include/mbgl/style/sources/vector_source.hpp
index d284062e1f..ff87084e19 100644
--- a/include/mbgl/style/sources/vector_source.hpp
+++ b/include/mbgl/style/sources/vector_source.hpp
@@ -16,7 +16,8 @@ public:
                  optional<float> minZoom = nullopt);
     ~VectorSource() final;
 
-    const variant<std::string, Tileset>* getURLOrTileset() const final;
+    const Tileset* getTileset() const final;
+    optional<Resource> getResource() const final;
     optional<std::string> getURL() const;
 
     class Impl;
diff --git a/platform/default/src/mbgl/storage/offline_download.cpp b/platform/default/src/mbgl/storage/offline_download.cpp
index 59ffd87292..ef33d2f3ee 100644
--- a/platform/default/src/mbgl/storage/offline_download.cpp
+++ b/platform/default/src/mbgl/storage/offline_download.cpp
@@ -143,32 +143,35 @@ OfflineRegionStatus OfflineDownload::getStatus() const {
     result->requiredResourceCountIsPrecise = true;
 
     for (const auto& source : parser.sources) {
-        auto handleTiledSource = [&](const variant<std::string, Tileset>& urlOrTileset) {
-            if (urlOrTileset.is<Tileset>()) {
-                uint64_t tileSourceCount = tileCount(definition, *source, urlOrTileset.get<Tileset>().zoomRange);
-                result->requiredTileCount += tileSourceCount;
-                result->requiredResourceCount += tileSourceCount;
-            } else {
-                result->requiredResourceCount += 1;
-                const auto& url = urlOrTileset.get<std::string>();
-                optional<Response> sourceResponse = offlineDatabase.get(Resource::source(url));
-                if (sourceResponse) {
-                    style::conversion::Error error;
-                    optional<Tileset> tileset = style::conversion::convertJSON<Tileset>(*sourceResponse->data, error);
-                    if (tileset) {
-                        uint64_t tileSourceCount = tileCount(definition, *source, (*tileset).zoomRange);
-                        result->requiredTileCount += tileSourceCount;
-                        result->requiredResourceCount += tileSourceCount;
-                    }
-                } else {
-                    result->requiredResourceCountIsPrecise = false;
+        auto handleTiledSourceWithResource = [&](const Resource& resource) {
+            result->requiredResourceCount += 1;
+            if (auto sourceResponse = offlineDatabase.get(resource)) {
+                style::conversion::Error error;
+                optional<Tileset> tileset = style::conversion::convertJSON<Tileset>(*sourceResponse->data, error);
+                if (tileset) {
+                    uint64_t tileSourceCount = tileCount(definition, *source, (*tileset).zoomRange);
+                    result->requiredTileCount += tileSourceCount;
+                    result->requiredResourceCount += tileSourceCount;
                 }
+            } else {
+                result->requiredResourceCountIsPrecise = false;
             }
         };
 
-        auto urlOrTileSet = source->getURLOrTileset();
-        if (urlOrTileSet) {
-            handleTiledSource(*urlOrTileSet);
+        auto handleTiledSourceWithTileset = [&](const Tileset& tileset) {
+            uint64_t tileSourceCount = tileCount(definition, *source, tileset.zoomRange);
+            result->requiredTileCount += tileSourceCount;
+            result->requiredResourceCount += tileSourceCount;
+        };
+
+        if (source->getTypeInfo()->tileSet == SourceTypeInfo::TileSet::Yes) {
+            if (auto resource = source->getResource()) {
+                handleTiledSourceWithResource(*resource);
+            } else if (auto* tileset = source->getTileset()) {
+                handleTiledSourceWithTileset(*tileset);
+            } else {
+                assert(false);
+            }
         } else if (source->getResource()) {
             result->requiredResourceCount += 1;
         }
@@ -203,29 +206,28 @@ void OfflineDownload::activateDownload() {
         style::Parser parser;
         parser.parse(*styleResponse.data);
 
-        auto handleTiledSource = [this](std::unique_ptr<Source> source,
-                                        const variant<std::string, Tileset>& urlOrTileset) mutable {
-            if (urlOrTileset.is<Tileset>()) {
-                queueTiles(*source, urlOrTileset.get<Tileset>());
-            } else {
-                const auto& url = urlOrTileset.get<std::string>();
+        auto handleTiledSource = [this](std::unique_ptr<Source> source) mutable {
+            assert(source->getTypeInfo()->tileSet == SourceTypeInfo::TileSet::Yes);
+
+            if (auto* tileset = source->getTileset()) {
+                queueTiles(*source, *tileset);
+            } else if (auto sourceResource = source->getResource()) {
+                auto url = sourceResource->url;
                 status.requiredResourceCountIsPrecise = false;
                 status.requiredResourceCount++;
                 requiredSourceURLs.insert(url);
 
-                auto sourceResource = Resource::source(url);
-                sourceResource.setPriority(Resource::Priority::Low);
-                sourceResource.setUsage(Resource::Usage::Offline);
+                sourceResource->setPriority(Resource::Priority::Low);
+                sourceResource->setUsage(Resource::Usage::Offline);
 
                 ensureResource(
-                    std::move(sourceResource),
-                    [=, source = std::shared_ptr<Source>(std::move(source))](const Response& sourceResponse) {
+                    std::move(*sourceResource),
+                    [this, source = std::shared_ptr<Source>(std::move(source)), url = std::move(url)](
+                        const Response& sourceResponse) {
                         style::conversion::Error error;
-                        optional<Tileset> tileset =
-                            style::conversion::convertJSON<Tileset>(*sourceResponse.data, error);
-                        if (tileset) {
-                            util::mapbox::canonicalizeTileset(*tileset, url, *source);
-                            queueTiles(*source, *tileset);
+                        if (auto tset = style::conversion::convertJSON<Tileset>(*sourceResponse.data, error)) {
+                            util::mapbox::canonicalizeTileset(*tset, url, *source);
+                            queueTiles(*source, *tset);
 
                             requiredSourceURLs.erase(url);
                             if (requiredSourceURLs.empty()) {
@@ -233,13 +235,14 @@ void OfflineDownload::activateDownload() {
                             }
                         }
                     });
+            } else {
+                assert(false);
             }
         };
 
         for (auto& source : parser.sources) {
-            auto urlOrTileSet = source->getURLOrTileset();
-            if (urlOrTileSet) {
-                handleTiledSource(std::move(source), *urlOrTileSet);
+            if (source->getTypeInfo()->tileSet == SourceTypeInfo::TileSet::Yes) {
+                handleTiledSource(std::move(source));
             } else if (auto resource = source->getResource()) {
                 queueResource(std::move(*resource));
             }
diff --git a/src/mbgl/style/source.cpp b/src/mbgl/style/source.cpp
index db195867c0..6ffaae0305 100644
--- a/src/mbgl/style/source.cpp
+++ b/src/mbgl/style/source.cpp
@@ -125,12 +125,10 @@ void Source::serializeTileSet(Value& value, const mbgl::Tileset& tileset) const
     }
 }
 
-void Source::serializeUrlOrTileSet(Value& value, const mbgl::variant<std::string, mbgl::Tileset>* urlOrTileSet) const {
+void Source::serializeUrlOrTileSet(Value& value, const mbgl::variant<std::string, mbgl::Tileset>& urlOrTileSet) const {
     assert(value.getObject());
 
-    if (!urlOrTileSet) return;
-
-    urlOrTileSet->match(
+    urlOrTileSet.match(
         [&](const std::string& url) {
             value.getObject()->insert({"url", url});
         },
diff --git a/src/mbgl/style/sources/raster_dem_source.cpp b/src/mbgl/style/sources/raster_dem_source.cpp
index a7efc2ba69..2535772f85 100644
--- a/src/mbgl/style/sources/raster_dem_source.cpp
+++ b/src/mbgl/style/sources/raster_dem_source.cpp
@@ -29,10 +29,9 @@ static std::string tilesetDEMEncodingToString(mbgl::Tileset::DEMEncoding encodin
 
 Value RasterDEMSource::serialize() const {
     auto value = RasterSource::serialize();
-    getURLOrTileset()->match([](const std::string&) {},
-                             [&](const mbgl::Tileset& tileset) {
-                                 value.getObject()->insert({"encoding", tilesetDEMEncodingToString(tileset.encoding)});
-                             });
+    if (auto* tileset = getTileset()) {
+        value.getObject()->insert({"encoding", tilesetDEMEncodingToString(tileset->encoding)});
+    }
     return value;
 }
 
diff --git a/src/mbgl/style/sources/raster_source.cpp b/src/mbgl/style/sources/raster_source.cpp
index 75c5acf24e..8ee442a200 100644
--- a/src/mbgl/style/sources/raster_source.cpp
+++ b/src/mbgl/style/sources/raster_source.cpp
@@ -25,8 +25,11 @@ const RasterSource::Impl& RasterSource::impl() const {
     return static_cast<const Impl&>(*baseImpl);
 }
 
-const variant<std::string, Tileset>* RasterSource::getURLOrTileset() const {
-    return &urlOrTileset;
+const Tileset* RasterSource::getTileset() const {
+    if (urlOrTileset.is<Tileset>()) {
+        return &urlOrTileset.get<Tileset>();
+    }
+    return nullptr;
 }
 
 optional<std::string> RasterSource::getURL() const {
@@ -37,6 +40,14 @@ optional<std::string> RasterSource::getURL() const {
     return urlOrTileset.get<std::string>();
 }
 
+optional<Resource> RasterSource::getResource() const {
+    if (urlOrTileset.is<std::string>()) {
+        return Resource::source(urlOrTileset.get<std::string>());
+    }
+
+    return nullopt;
+}
+
 void RasterSource::loadDescription(FileSource& fileSource) {
     if (urlOrTileset.is<Tileset>()) {
         baseImpl = createMutable(urlOrTileset.get<Tileset>());
@@ -94,7 +105,7 @@ Mutable<Source::Impl> RasterSource::createMutable(Tileset tileset) const noexcep
 
 Value RasterSource::serialize() const {
     auto value = Source::serialize();
-    serializeUrlOrTileSet(value, getURLOrTileset());
+    serializeUrlOrTileSet(value, urlOrTileset);
     return value;
 }
 
diff --git a/src/mbgl/style/sources/vector_source.cpp b/src/mbgl/style/sources/vector_source.cpp
index c08be53e6f..39f6fde76c 100644
--- a/src/mbgl/style/sources/vector_source.cpp
+++ b/src/mbgl/style/sources/vector_source.cpp
@@ -30,8 +30,11 @@ const VectorSource::Impl& VectorSource::impl() const {
     return static_cast<const Impl&>(*baseImpl);
 }
 
-const variant<std::string, Tileset>* VectorSource::getURLOrTileset() const {
-    return &urlOrTileset;
+const Tileset* VectorSource::getTileset() const {
+    if (urlOrTileset.is<Tileset>()) {
+        return &urlOrTileset.get<Tileset>();
+    }
+    return nullptr;
 }
 
 optional<std::string> VectorSource::getURL() const {
@@ -42,6 +45,14 @@ optional<std::string> VectorSource::getURL() const {
     return urlOrTileset.get<std::string>();
 }
 
+optional<Resource> VectorSource::getResource() const {
+    if (urlOrTileset.is<std::string>()) {
+        return Resource::source(urlOrTileset.get<std::string>());
+    }
+
+    return nullopt;
+}
+
 void VectorSource::loadDescription(FileSource& fileSource) {
     if (urlOrTileset.is<Tileset>()) {
         baseImpl = makeMutable<Impl>(impl(), urlOrTileset.get<Tileset>());
@@ -100,7 +111,7 @@ Mutable<Source::Impl> VectorSource::createMutable() const noexcept {
 
 Value VectorSource::serialize() const {
     auto result = Source::serialize();
-    serializeUrlOrTileSet(result, getURLOrTileset());
+    serializeUrlOrTileSet(result, urlOrTileset);
     return result;
 }