Use HashFunction::Type to deserialize ArtifactDescription

5 files changed, 91 insertions, 48 deletions

diff --git a/test/buildtool/common/TARGETS b/test/buildtool/common/TARGETS
index dc6b3056..ef1fff25 100644
--- a/test/buildtool/common/TARGETS
+++ b/test/buildtool/common/TARGETS
@@ -11,6 +11,7 @@
     , ["@", "src", "src/buildtool/common", "artifact_digest_factory"]
     , ["@", "src", "src/buildtool/crypto", "hash_function"]
     , ["@", "src", "src/buildtool/file_system", "object_type"]
+    , ["@", "src", "src/buildtool/compatibility", "compatibility"]
     ]
   , "stage": ["test", "buildtool", "common"]
   }
@@ -24,6 +25,8 @@
     , ["@", "json", "", "json"]
     , ["@", "src", "src/buildtool/common", "action_description"]
     , ["@", "src", "src/buildtool/common", "common"]
+    , ["@", "src", "src/buildtool/crypto", "hash_function"]
+    , ["@", "src", "src/buildtool/compatibility", "compatibility"]
     ]
   , "stage": ["test", "buildtool", "common"]
   }
diff --git a/test/buildtool/common/action_description.test.cpp b/test/buildtool/common/action_description.test.cpp
index eab2cc48..7d5a72d7 100644
--- a/test/buildtool/common/action_description.test.cpp
+++ b/test/buildtool/common/action_description.test.cpp
@@ -20,6 +20,14 @@
 #include "nlohmann/json.hpp"
 #include "src/buildtool/common/action.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
+#include "src/buildtool/compatibility/compatibility.hpp"
+#include "src/buildtool/crypto/hash_function.hpp"
+
+[[nodiscard]] static inline auto GetHashType(bool compatible) noexcept
+    -> HashFunction::Type {
+    return compatible ? HashFunction::Type::PlainSHA256
+                      : HashFunction::Type::GitSHA1;
+}
 
 TEST_CASE("From JSON", "[action_description]") {
     using path = std::filesystem::path;
@@ -37,56 +45,57 @@ TEST_CASE("From JSON", "[action_description]") {
                           desc.Inputs()}
             .ToJson();
 
+    auto const hash_type = GetHashType(Compatibility::IsCompatible());
     SECTION("Parse full action") {
-        auto description = ActionDescription::FromJson("id", json);
+        auto description = ActionDescription::FromJson(hash_type, "id", json);
         REQUIRE(description);
         CHECK((*description)->ToJson() == json);
     }
 
     SECTION("Parse action without optional input") {
         json["input"] = nlohmann::json::object();
-        CHECK(ActionDescription::FromJson("id", json));
+        CHECK(ActionDescription::FromJson(hash_type, "id", json));
 
         json["input"] = nlohmann::json::array();
-        CHECK_FALSE(ActionDescription::FromJson("id", json));
+        CHECK_FALSE(ActionDescription::FromJson(hash_type, "id", json));
 
         json.erase("input");
-        CHECK(ActionDescription::FromJson("id", json));
+        CHECK(ActionDescription::FromJson(hash_type, "id", json));
     }
 
     SECTION("Parse action without optional env") {
         json["env"] = nlohmann::json::object();
-        CHECK(ActionDescription::FromJson("id", json));
+        CHECK(ActionDescription::FromJson(hash_type, "id", json));
 
         json["env"] = nlohmann::json::array();
-        CHECK_FALSE(ActionDescription::FromJson("id", json));
+        CHECK_FALSE(ActionDescription::FromJson(hash_type, "id", json));
 
         json.erase("env");
-        CHECK(ActionDescription::FromJson("id", json));
+        CHECK(ActionDescription::FromJson(hash_type, "id", json));
     }
 
     SECTION("Parse action without mandatory outputs") {
         json["output"] = nlohmann::json::array();
         json["output_dirs"] = nlohmann::json::array();
-        CHECK_FALSE(ActionDescription::FromJson("id", json));
+        CHECK_FALSE(ActionDescription::FromJson(hash_type, "id", json));
 
         json["output"] = nlohmann::json::object();
         json["output_dirs"] = nlohmann::json::object();
-        CHECK_FALSE(ActionDescription::FromJson("id", json));
+        CHECK_FALSE(ActionDescription::FromJson(hash_type, "id", json));
 
         json.erase("output");
         json.erase("output_dirs");
-        CHECK_FALSE(ActionDescription::FromJson("id", json));
+        CHECK_FALSE(ActionDescription::FromJson(hash_type, "id", json));
     }
 
     SECTION("Parse action without mandatory command") {
         json["command"] = nlohmann::json::array();
-        CHECK_FALSE(ActionDescription::FromJson("id", json));
+        CHECK_FALSE(ActionDescription::FromJson(hash_type, "id", json));
 
         json["command"] = nlohmann::json::object();
-        CHECK_FALSE(ActionDescription::FromJson("id", json));
+        CHECK_FALSE(ActionDescription::FromJson(hash_type, "id", json));
 
         json.erase("command");
-        CHECK_FALSE(ActionDescription::FromJson("id", json));
+        CHECK_FALSE(ActionDescription::FromJson(hash_type, "id", json));
     }
 }
diff --git a/test/buildtool/common/artifact_description.test.cpp b/test/buildtool/common/artifact_description.test.cpp
index f7af5159..a05baa66 100644
--- a/test/buildtool/common/artifact_description.test.cpp
+++ b/test/buildtool/common/artifact_description.test.cpp
@@ -22,6 +22,7 @@
 #include "src/buildtool/common/artifact.hpp"
 #include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/common/artifact_digest_factory.hpp"
+#include "src/buildtool/compatibility/compatibility.hpp"
 #include "src/buildtool/crypto/hash_function.hpp"
 #include "src/buildtool/file_system/object_type.hpp"
 
@@ -39,10 +40,17 @@ static auto NamedDigest(std::string const& str) -> ArtifactDigest {
            lhs.Info() == rhs.Info();
 }
 
+[[nodiscard]] static inline auto GetHashType(bool compatible) noexcept
+    -> HashFunction::Type {
+    return compatible ? HashFunction::Type::PlainSHA256
+                      : HashFunction::Type::GitSHA1;
+}
+
 TEST_CASE("Local artifact", "[artifact_description]") {
     auto local_desc = ArtifactDescription::CreateLocal(
         std::filesystem::path{"local_path"}, "repo");
-    auto from_json = ArtifactDescription::FromJson(local_desc.ToJson());
+    auto from_json = ArtifactDescription::FromJson(
+        GetHashType(Compatibility::IsCompatible()), local_desc.ToJson());
     CHECK(local_desc == *from_json);
 }
 
@@ -50,19 +58,22 @@ TEST_CASE("Known artifact", "[artifact_description]") {
     SECTION("File object") {
         auto known_desc = ArtifactDescription::CreateKnown(
             NamedDigest("f_fake_hash"), ObjectType::File);
-        auto from_json = ArtifactDescription::FromJson(known_desc.ToJson());
+        auto from_json = ArtifactDescription::FromJson(
+            GetHashType(Compatibility::IsCompatible()), known_desc.ToJson());
         CHECK(known_desc == *from_json);
     }
     SECTION("Executable object") {
         auto known_desc = ArtifactDescription::CreateKnown(
             NamedDigest("x_fake_hash"), ObjectType::Executable);
-        auto from_json = ArtifactDescription::FromJson(known_desc.ToJson());
+        auto from_json = ArtifactDescription::FromJson(
+            GetHashType(Compatibility::IsCompatible()), known_desc.ToJson());
         CHECK(known_desc == *from_json);
     }
     SECTION("Symlink object") {
         auto known_desc = ArtifactDescription::CreateKnown(
             NamedDigest("l_fake_hash"), ObjectType::Symlink);
-        auto from_json = ArtifactDescription::FromJson(known_desc.ToJson());
+        auto from_json = ArtifactDescription::FromJson(
+            GetHashType(Compatibility::IsCompatible()), known_desc.ToJson());
         CHECK(known_desc == *from_json);
     }
 }
@@ -70,7 +81,8 @@ TEST_CASE("Known artifact", "[artifact_description]") {
 TEST_CASE("Action artifact", "[artifact_description]") {
     auto action_desc = ArtifactDescription::CreateAction(
         "action_id", std::filesystem::path{"out_path"});
-    auto from_json = ArtifactDescription::FromJson(action_desc.ToJson());
+    auto from_json = ArtifactDescription::FromJson(
+        GetHashType(Compatibility::IsCompatible()), action_desc.ToJson());
     CHECK(action_desc == *from_json);
 }
 
@@ -81,76 +93,77 @@ TEST_CASE("From JSON", "[artifact_description]") {
             .ToJson();
     auto action = ArtifactDescription::CreateAction("id", "output").ToJson();
 
+    auto const hash_type = GetHashType(Compatibility::IsCompatible());
     SECTION("Parse artifacts") {
-        CHECK(ArtifactDescription::FromJson(local));
-        CHECK(ArtifactDescription::FromJson(known));
-        CHECK(ArtifactDescription::FromJson(action));
+        CHECK(ArtifactDescription::FromJson(hash_type, local));
+        CHECK(ArtifactDescription::FromJson(hash_type, known));
+        CHECK(ArtifactDescription::FromJson(hash_type, action));
     }
 
     SECTION("Parse artifact without mandatory type") {
         local.erase("type");
         known.erase("type");
         action.erase("type");
-        CHECK_FALSE(ArtifactDescription::FromJson(local));
-        CHECK_FALSE(ArtifactDescription::FromJson(known));
-        CHECK_FALSE(ArtifactDescription::FromJson(action));
+        CHECK_FALSE(ArtifactDescription::FromJson(hash_type, local));
+        CHECK_FALSE(ArtifactDescription::FromJson(hash_type, known));
+        CHECK_FALSE(ArtifactDescription::FromJson(hash_type, action));
     }
 
     SECTION("Parse artifact without mandatory data") {
         local.erase("data");
         known.erase("data");
         action.erase("data");
-        CHECK_FALSE(ArtifactDescription::FromJson(local));
-        CHECK_FALSE(ArtifactDescription::FromJson(known));
-        CHECK_FALSE(ArtifactDescription::FromJson(action));
+        CHECK_FALSE(ArtifactDescription::FromJson(hash_type, local));
+        CHECK_FALSE(ArtifactDescription::FromJson(hash_type, known));
+        CHECK_FALSE(ArtifactDescription::FromJson(hash_type, action));
     }
 
     SECTION("Parse local artifact without mandatory path") {
         local["data"]["path"] = 0;
-        CHECK_FALSE(ArtifactDescription::FromJson(local));
+        CHECK_FALSE(ArtifactDescription::FromJson(hash_type, local));
 
         local["data"].erase("path");
-        CHECK_FALSE(ArtifactDescription::FromJson(local));
+        CHECK_FALSE(ArtifactDescription::FromJson(hash_type, local));
     }
 
     SECTION("Parse known artifact") {
         SECTION("without mandatory id") {
             known["data"]["id"] = 0;
-            CHECK_FALSE(ArtifactDescription::FromJson(known));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, known));
 
             known["data"].erase("id");
-            CHECK_FALSE(ArtifactDescription::FromJson(known));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, known));
         }
         SECTION("without mandatory size") {
             known["data"]["size"] = "0";
-            CHECK_FALSE(ArtifactDescription::FromJson(known));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, known));
 
             known["data"].erase("size");
-            CHECK_FALSE(ArtifactDescription::FromJson(known));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, known));
         }
         SECTION("without mandatory file_type") {
             known["data"]["file_type"] = "more_than_one_char";
-            CHECK_FALSE(ArtifactDescription::FromJson(known));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, known));
 
             known["data"].erase("file_type");
-            CHECK_FALSE(ArtifactDescription::FromJson(known));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, known));
         }
     }
 
     SECTION("Parse action artifact") {
         SECTION("without mandatory id") {
             action["data"]["id"] = 0;
-            CHECK_FALSE(ArtifactDescription::FromJson(action));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, action));
 
             action["data"].erase("id");
-            CHECK_FALSE(ArtifactDescription::FromJson(action));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, action));
         }
         SECTION("without mandatory path") {
             action["data"]["path"] = 0;
-            CHECK_FALSE(ArtifactDescription::FromJson(action));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, action));
 
             action["data"].erase("path");
-            CHECK_FALSE(ArtifactDescription::FromJson(action));
+            CHECK_FALSE(ArtifactDescription::FromJson(hash_type, action));
         }
     }
 }
@@ -158,16 +171,19 @@ TEST_CASE("From JSON", "[artifact_description]") {
 TEST_CASE("Description missing mandatory key/value pair",
           "[artifact_description]") {
     nlohmann::json const missing_type = {{"data", {{"path", "some/path"}}}};
-    CHECK(not ArtifactDescription::FromJson(missing_type));
+    CHECK(not ArtifactDescription::FromJson(
+        GetHashType(Compatibility::IsCompatible()), missing_type));
     nlohmann::json const missing_data = {{"type", "LOCAL"}};
-    CHECK(not ArtifactDescription::FromJson(missing_data));
+    CHECK(not ArtifactDescription::FromJson(
+        GetHashType(Compatibility::IsCompatible()), missing_data));
 }
 
 TEST_CASE("Local artifact description contains incorrect value for \"data\"",
           "[artifact_description]") {
     nlohmann::json const local_art_missing_path = {
         {"type", "LOCAL"}, {"data", nlohmann::json::object()}};
-    CHECK(not ArtifactDescription::FromJson(local_art_missing_path));
+    CHECK(not ArtifactDescription::FromJson(
+        GetHashType(Compatibility::IsCompatible()), local_art_missing_path));
 }
 
 TEST_CASE("Known artifact description contains incorrect value for \"data\"",
@@ -178,19 +194,24 @@ TEST_CASE("Known artifact description contains incorrect value for \"data\"",
         nlohmann::json const known_art_missing_id = {
             {"type", "KNOWN"},
             {"data", {{"size", 15}, {"file_type", file_type}}}};
-        CHECK(not ArtifactDescription::FromJson(known_art_missing_id));
+        CHECK(not ArtifactDescription::FromJson(
+            GetHashType(Compatibility::IsCompatible()), known_art_missing_id));
     }
     SECTION("missing \"size\"") {
         nlohmann::json const known_art_missing_size = {
             {"type", "KNOWN"},
             {"data", {{"id", "known_input"}, {"file_type", file_type}}}};
-        CHECK(not ArtifactDescription::FromJson(known_art_missing_size));
+        CHECK(not ArtifactDescription::FromJson(
+            GetHashType(Compatibility::IsCompatible()),
+            known_art_missing_size));
     }
     SECTION("missing \"file_type\"") {
         nlohmann::json const known_art_missing_file_type = {
             {"type", "KNOWN"}, {"data", {{"id", "known_input"}, {"size", 15}}}};
 
-        CHECK(not ArtifactDescription::FromJson(known_art_missing_file_type));
+        CHECK(not ArtifactDescription::FromJson(
+            GetHashType(Compatibility::IsCompatible()),
+            known_art_missing_file_type));
     }
 }
 
@@ -198,9 +219,11 @@ TEST_CASE("Action artifact description contains incorrect value for \"data\"",
           "[artifact_description]") {
     nlohmann::json const action_art_missing_id = {
         {"type", "ACTION"}, {"data", {{"path", "output/path"}}}};
-    CHECK(not ArtifactDescription::FromJson(action_art_missing_id));
+    CHECK(not ArtifactDescription::FromJson(
+        GetHashType(Compatibility::IsCompatible()), action_art_missing_id));
 
     nlohmann::json const action_art_missing_path = {
         {"type", "ACTION"}, {"data", {{"id", "action_id"}}}};
-    CHECK(not ArtifactDescription::FromJson(action_art_missing_path));
+    CHECK(not ArtifactDescription::FromJson(
+        GetHashType(Compatibility::IsCompatible()), action_art_missing_path));
 }
diff --git a/test/buildtool/execution_engine/traverser/TARGETS b/test/buildtool/execution_engine/traverser/TARGETS
index 821c4bc8..bdd7c4cc 100644
--- a/test/buildtool/execution_engine/traverser/TARGETS
+++ b/test/buildtool/execution_engine/traverser/TARGETS
@@ -10,6 +10,8 @@
     , ["@", "src", "src/buildtool/common", "artifact_description"]
     , ["@", "src", "src/buildtool/execution_engine/dag", "dag"]
     , ["@", "src", "src/buildtool/execution_engine/traverser", "traverser"]
+    , ["@", "src", "src/buildtool/crypto", "hash_function"]
+    , ["@", "src", "src/buildtool/compatibility", "compatibility"]
     ]
   , "stage": ["test", "buildtool", "execution_engine", "traverser"]
   }
diff --git a/test/buildtool/execution_engine/traverser/traverser.test.cpp b/test/buildtool/execution_engine/traverser/traverser.test.cpp
index 97e2095e..70ab2465 100644
--- a/test/buildtool/execution_engine/traverser/traverser.test.cpp
+++ b/test/buildtool/execution_engine/traverser/traverser.test.cpp
@@ -24,6 +24,8 @@
 
 #include "catch2/catch_test_macros.hpp"
 #include "src/buildtool/common/artifact_description.hpp"
+#include "src/buildtool/compatibility/compatibility.hpp"
+#include "src/buildtool/crypto/hash_function.hpp"
 #include "src/buildtool/execution_engine/dag/dag.hpp"
 #include "test/utils/container_matchers.hpp"
 
@@ -182,8 +184,12 @@ class TestProject {
         auto inputs_desc = ActionDescription::inputs_t{};
         if (not inputs.empty()) {
             command.emplace_back("FROM");
+            auto const hash_type = Compatibility::IsCompatible()
+                                       ? HashFunction::Type::PlainSHA256
+                                       : HashFunction::Type::GitSHA1;
             for (auto const& input_desc : inputs) {
-                auto artifact = ArtifactDescription::FromJson(input_desc);
+                auto artifact =
+                    ArtifactDescription::FromJson(hash_type, input_desc);
                 REQUIRE(artifact);
                 auto const input_id = artifact->Id();
                 command.push_back(input_id);