Allow read and write of file rules without cs validation (#1976)

Signed-off-by: Doug Walker <[email protected]>

src/OpenColorIO/OCIOYaml.cpp

@@ -4034,7 +4034,7 @@ inline void load(const YAML::Node & node, ViewingRulesRcPtr & vr)
     catch (Exception & ex)
     {
         std::ostringstream os;
-        os << "File rules: " << ex.what();
+        os << "Viewing rules: " << ex.what();
         throwError(node, os.str().c_str());
     }
 }
@@ -4658,13 +4658,15 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen
         config->setWorkingDir(configrootdir.c_str());
     }
 
-    auto defaultCS = config->getColorSpace(ROLE_DEFAULT);
     if (!fileRulesFound)
     {
         if (config->getMajorVersion() >= 2)
         {
-            if (!defaultCS)
+            if (!config->hasRole(ROLE_DEFAULT))
             {
+                // Note that no validation of the default color space is done (e.g. to check that
+                // it exists in the config) in order to enable loading configs that are only
+                // partially complete. The caller may use config->validate() after, if desired.
                 throwError(node, "The config must contain either a Default file rule or "
                                  "the 'default' role.");
             }
@@ -4683,6 +4685,7 @@ inline void load(const YAML::Node& node, ConfigRcPtr & config, const char* filen
     else
     {
         // If default role is also defined.
+        auto defaultCS = config->getColorSpace(ROLE_DEFAULT);
         if (defaultCS)
         {
             const auto defaultRule = fileRules->getNumEntries() - 1;
@@ -4815,18 +4818,11 @@ inline void save(YAML::Emitter & out, const Config & config)
         const char* role = config.getRoleName(i);
         if(role && *role)
         {
-            ConstColorSpaceRcPtr colorspace = config.getColorSpace(role);
-            if(colorspace)
-            {
-                out << YAML::Key << role;
-                out << YAML::Value << config.getColorSpace(role)->getName();
-            }
-            else
-            {
-                std::ostringstream os;
-                os << "Colorspace associated to the role '" << role << "', does not exist.";
-                throw Exception(os.str().c_str());
-            }
+            // Note that no validation of the name strings is done here (e.g. to check that
+            // they exist in the config) in order to enable serializing configs that are only
+            // partially complete. The caller may use config->validate() first, if desired.
+            out << YAML::Key << role;
+            out << YAML::Value << config.getRoleColorSpace(i);
         }
     }
     out << YAML::EndMap;

tests/cpu/Config_tests.cpp

@@ -1911,16 +1911,6 @@ OCIO_ADD_TEST(Config, context_variable_with_search_path_v2)
     }
 }
 
-OCIO_ADD_TEST(Config, role_without_colorspace)
-{
-    OCIO::ConfigRcPtr config = OCIO::Config::Create()->createEditableCopy();
-    config->setRole("reference", "UnknownColorSpace");
-
-    std::ostringstream os;
-    OCIO_CHECK_THROW_WHAT(config->serialize(os), OCIO::Exception,
-                          "Colorspace associated to the role 'reference', does not exist");
-}
-
 OCIO_ADD_TEST(Config, env_colorspace_name)
 {
     // Unset the env. variable to make sure the test start in the right environment.

tests/cpu/FileRules_tests.cpp

@@ -1831,6 +1831,78 @@ OCIO_ADD_TEST(FileRules, config_v1_to_v2_from_memory)
     }
 }
 
+OCIO_ADD_TEST(FileRules, read_write_incomplete_configs)
+{
+    // It should be possible to read and write configs where that are syntactically valid 
+    // but which are incomplete and hence would not pass validation.
+
+    // The default role references a color space that has not been defined yet.
+    {
+constexpr char CONFIG[] = { R"(ocio_profile_version: 2
+roles:
+  default: cs2
+
+colorspaces:
+  - !<ColorSpace>
+      name: raw
+)" };
+
+        // Test read works.
+        std::istringstream iss;
+        iss.str(CONFIG);
+        OCIO::ConstConfigRcPtr cfg;
+        OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss));
+        OCIO_REQUIRE_ASSERT(cfg);
+
+        // Test write works.
+        std::ostringstream os;
+        OCIO_CHECK_NO_THROW(cfg->serialize(os));
+
+        // Test that validate catches the problem.
+        OCIO_CHECK_THROW_WHAT(
+            cfg->validate(),
+            OCIO::Exception,
+            "Config failed role validation. The role 'default' refers to a color space, 'cs2', "\
+            "which is not defined."
+        );
+    }
+
+    // FileRules Default rule references a color space that has not been defined yet.
+    {
+constexpr char CONFIG[] = { R"(ocio_profile_version: 2
+
+file_rules:
+  - !<Rule> {name: Default, colorspace: cs2}
+
+displays:
+  sRGB:
+  - !<View> {name: Raw, colorspace: raw}
+colorspaces:
+  - !<ColorSpace>
+      name: raw
+)" };
+
+        // Test read works.
+        std::istringstream iss;
+        iss.str(CONFIG);
+        OCIO::ConstConfigRcPtr cfg;
+        OCIO_CHECK_NO_THROW(cfg = OCIO::Config::CreateFromStream(iss));
+        OCIO_REQUIRE_ASSERT(cfg);
+
+        // Test write works.
+        std::ostringstream os;
+        OCIO_CHECK_NO_THROW(cfg->serialize(os));
+
+        // Test that validate catches the problem.
+        OCIO_CHECK_THROW_WHAT(
+            cfg->validate(),
+            OCIO::Exception,
+            "File rules: rule named 'Default' is referencing 'cs2' that is neither "\
+            "a color space nor a named transform."
+        );
+    }
+}
+
 OCIO_ADD_TEST(FileRules, config_v2_wrong_rule)
 {
     // 2 default rules.