Merge pull request #54 from TGSAI/44_datetime

Fixed date-time validation
TGSAI · Aug 1, 2024 · 27bec41 · 27bec41
2 parents 7ad427c + e551c59
commit 27bec41
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 44 deletions.
diff --git a/mdio/dataset_schema.h b/mdio/dataset_schema.h
@@ -350,6 +350,12 @@
                "title": "Apiversion",
                "type": "string"
             },
+            "createdOn": {
+               "description": "The timestamp indicating when the dataset was first created, including timezone information. Expressed in ISO 8601 format.",
+               "format": "date-time",
+               "title": "Createdon",
+               "type": "string"
+            },
             "attributes": {
                "anyOf": [
                   {
@@ -366,7 +372,8 @@
          },
          "required": [
             "name",
-            "apiVersion"
+            "apiVersion",
+            "createdOn"
          ],
          "title": "DatasetMetadata",
          "type": "object"

diff --git a/mdio/dataset_validator.h b/mdio/dataset_validator.h
@@ -16,7 +16,6 @@
 #define MDIO_DATASET_VALIDATOR_H_
 
 #include <fstream>
-#include <regex>  // NOLINT
 #include <string>
 #include <unordered_set>
 
@@ -27,17 +26,6 @@
 #include <nlohmann/json-schema.hpp>  // NOLINT
 // clang-format on
 
-/**
- * @brief Checks if a string is a valid ISO8601 datetime
- * @param dateTimeStr A string representing a datetime
- * @return True if valid, false otherwise
- */
-bool isISO8601DateTime(const std::string& dateTimeStr) {
-  std::regex iso8601Regex(
-      R"(^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{6}(-\d{2}:\d{2}|Z)$)");
-  return std::regex_match(dateTimeStr, iso8601Regex);
-}
-
 /**
  * @brief Checks if a key exists in a map
  * Specific for our case of {coordinate: index} mapping
@@ -55,26 +43,10 @@ bool contains(const std::unordered_set<std::string>& set,
  * InvalidArgumentError if validation fails for any reason
  */
 absl::Status validate_schema(nlohmann::json& spec /*NOLINT*/) {
-  // This is a hack to fix the date-time format not working as intended with the
-  // json-schema-validator
-
-  nlohmann::json createdOn = nlohmann::json::object();
-  if (spec.contains("metadata")) {
-    if (!spec["metadata"].contains("createdOn")) {
-      return absl::Status(absl::StatusCode::kInvalidArgument,
-                          "CreatedOn field not found.");
-    }
-    createdOn = spec["metadata"]["createdOn"];
-    spec["metadata"].erase("createdOn");
-    if (!isISO8601DateTime(createdOn.get<std::string>())) {
-      return absl::Status(absl::StatusCode::kInvalidArgument,
-                          "CreatedOn field is not a valid ISO8601 datetime.");
-    }
-  }
-
   nlohmann::json targetSchema = nlohmann::json::parse(kDatasetSchema);
 
-  nlohmann::json_schema::json_validator validator;
+  nlohmann::json_schema::json_validator validator(
+      nullptr, nlohmann::json_schema::default_string_format_check);
   validator.set_root_schema(targetSchema);
 
   try {
@@ -85,9 +57,6 @@ absl::Status validate_schema(nlohmann::json& spec /*NOLINT*/) {
         "Validation failed, here is why: " + std::string(e.what()));
   }
 
-  if (!createdOn.empty()) {
-    spec["metadata"]["createdOn"] = createdOn;
-  }
   return absl::OkStatus();
 }
 

diff --git a/mdio/dataset_validator_test.cc b/mdio/dataset_validator_test.cc
@@ -897,16 +897,6 @@ TEST(Validate, valid) {
   EXPECT_TRUE(status.ok()) << status;
 }
 
-TEST(Validate, invalid) { EXPECT_TRUE(true); }
-
-TEST(Datetime, valid) {
-  EXPECT_TRUE(isISO8601DateTime("2023-12-12T15:02:06.413469-06:00"));
-}
-
-TEST(Datetime, invalid) {
-  EXPECT_FALSE(isISO8601DateTime("2023-12-12T15:02:06.413469-06:"));
-}
-
 // TODO(BrianMichell): Validate that the shapes are all valid
 // TODO(BrianMichell): Return a list of Variable specs + metadata