Add new verifier to handle identifiers and update codegen to be able to use them (closes #2522)

include/openspace/documentation/verifier.h

@@ -168,6 +168,21 @@ private:
     bool _mustBeNotEmpty = false;
 };
 
+/**
+ * A Verifier that checks whether a given string is a valid identifier, meaning that is
+ * does not contain any whitespaces or dots
+ */
+struct IdentifierVerifier : public StringVerifier {
+    IdentifierVerifier();
+
+    TestResult operator()(const ghoul::Dictionary& dict,
+        const std::string& key) const override;
+
+    std::string documentation() const override;
+
+    std::string type() const override;
+};
+
 /**
  * A Verifier that checks whether a given key inside a ghoul::Dictionary is a string and
  * refers to an existing file on disk.

modules/base/lightsource/scenegraphlightsource.cpp

@@ -51,7 +51,7 @@ namespace {
         std::optional<float> intensity;
 
         // [[codegen::verbatim(NodeInfo.description)]]
-        std::string node;
+        std::string node [[codegen::identifier()]];
     };
 #include "scenegraphlightsource_codegen.cpp"
 } // namespace

modules/base/rendering/renderablenodeline.cpp

@@ -112,10 +112,10 @@ namespace {
 
     struct [[codegen::Dictionary(RenderableNodeLine)]] Parameters {
         // [[codegen::verbatim(StartNodeInfo.description)]]
-        std::optional<std::string> startNode;
+        std::optional<std::string> startNode [[codegen::identifier()]];
 
         // [[codegen::verbatim(EndNodeInfo.description)]]
-        std::optional<std::string> endNode;
+        std::optional<std::string> endNode [[codegen::identifier()]];
 
         // [[codegen::verbatim(LineColorInfo.description)]]
         std::optional<glm::vec3> color [[codegen::color()]];

modules/globebrowsing/src/layer.cpp

@@ -96,8 +96,8 @@ namespace {
     };
 
     struct [[codegen::Dictionary(Layer), codegen::noexhaustive()]] Parameters {
-        // The unique identifier for this layer. May not contain '.' or spaces
-        std::string identifier;
+        // The unique identifier for this layer.
+        std::string identifier [[codegen::identifier()]];
 
         // A human-readable name for the user interface. If this is omitted, the
         // identifier is used instead

modules/sync/syncs/httpsynchronization.cpp

@@ -39,7 +39,7 @@ namespace {
     struct [[codegen::Dictionary(HttpSynchronization)]] Parameters {
         // The unique identifier for this resource that is used to request a set of files
         // from the synchronization servers
-        std::string identifier;
+        std::string identifier [[codegen::identifier()]];
 
         // The version of this resource that should be requested
         int version;

modules/sync/syncs/urlsynchronization.cpp

@@ -43,7 +43,7 @@ namespace {
 
         // This identifier will be part of the used folder structure and, can be used to
         // manually find the downloaded folder in the synchronization folder
-        std::string identifier;
+        std::string identifier [[codegen::identifier()]];
 
         // If this value is set to 'true' and it is not overwritten by the global
         // settings, the file(s) pointed to by this URLSynchronization will always be

src/documentation/verifier.cpp

@@ -231,6 +231,37 @@ std::string StringVerifier::type() const {
     return "String";
 }
 
+IdentifierVerifier::IdentifierVerifier() : StringVerifier(true) {}
+
+TestResult IdentifierVerifier::operator()(const ghoul::Dictionary& dict,
+                                          const std::string& key) const
+{
+    TestResult res = StringVerifier::operator()(dict, key);
+    if (!res.success) {
+        return res;
+    }
+
+    std::string identifier = dict.value<std::string>(key);
+    size_t pos = identifier.find_first_of(" \t\n\r.");
+    if (pos != std::string::npos) {
+        res.success = false;
+        TestResult::Offense off;
+        off.offender = key;
+        off.reason = TestResult::Offense::Reason::Verification;
+        off.explanation = "Identifier contained illegal character";
+        res.offenses.push_back(off);
+    }
+    return res;
+}
+
+std::string IdentifierVerifier::documentation() const {
+    return "An identifier string. May not contain '.', spaces, newlines, or tabs";
+};
+
+std::string IdentifierVerifier::type() const {
+    return "Identifier";
+}
+
 FileVerifier::FileVerifier() : StringVerifier(true) {}
 
 TestResult FileVerifier::operator()(const ghoul::Dictionary& dict,

src/rendering/dashboarditem.cpp

@@ -43,7 +43,7 @@ namespace {
     struct [[codegen::Dictionary(DashboardItem)]] Parameters {
         std::string type;
 
-        std::string identifier;
+        std::string identifier [[codegen::identifier()]];
 
         std::optional<std::string> guiName;
 

src/rendering/screenspacerenderable.cpp

@@ -175,9 +175,8 @@ namespace {
 
         // This is the unique identifier for this screenspace renderable. It has to be
         // unique amongst all existing screenspace nodes that already have been added to
-        // the scene. The identifier is not allowed to have any whitespace or '.' and must
-        // not be empty
-        std::optional<std::string> identifier;
+        // the scene.
+        std::optional<std::string> identifier [[codegen::identifier()]];
 
         // [[codegen::verbatim(EnabledInfo.description)]]
         std::optional<bool> enabled;

src/scene/assetmanager.cpp

@@ -100,7 +100,7 @@ namespace {
 
         // A list of all identifiers that are exposed by this asset. This list is needed
         // to populate the descriptions in the main user interface
-        std::optional<std::vector<std::string>> identifiers;
+        std::optional<std::vector<std::string>> identifiers [[codegen::identifier()]];
     };
 
 #include "assetmanager_codegen.cpp"

src/scene/lightsource.cpp

@@ -48,7 +48,7 @@ namespace {
         std::string type [[codegen::annotation("Must name a valid LightSource type")]];
 
         // The identifier of the light source
-        std::string identifier;
+        std::string identifier [[codegen::identifier()]];
 
         // [[codegen::verbatim(EnabledInfo.description)]]
         std::optional<bool> enabled;

src/scene/scenegraphnode.cpp

@@ -179,16 +179,13 @@ namespace {
         // The identifier of this scene graph node. This name must be unique among all
         // scene graph nodes that are loaded in a specific scene. If a duplicate is
         // detected the loading of the node will fail, as will all childing that depend on
-        // the node. The identifier must not contain any whitespaces or '.'
-        std::string identifier;
+        // the node.
+        std::string identifier [[codegen::identifier()]];
 
         // This names the parent of the currently specified scene graph node. The parent
         // must already exist in the scene graph. If not specified, the node will be
         // attached to the root of the scene graph
-        std::optional<std::string> parent
-            [[codegen::annotation(
-                "If specified, this must be a name for another scene graph node"
-            )]];
+        std::optional<std::string> parent [[codegen::identifier()]];
 
         // The renderable that is to be created for this scene graph node. A renderable is
         // a component of a scene graph node that will lead to some visual result on the

src/util/resourcesynchronization.cpp

@@ -41,7 +41,7 @@ namespace {
 
         // A unique identifier that is used to reference this specific
         // ResourceSynchronization object
-        std::string identifier;
+        std::string identifier [[codegen::identifier()]];
 
         // A user readable name of this synchronization
         std::string name;

tests/test_documentation.cpp

@@ -57,6 +57,11 @@ TEST_CASE("Documentation: Constructor", "[documentation]") {
         new StringVerifier,
         Optional::No
     );
+    doc.entries.emplace_back(
+        "IdentifierVerifier",
+        new IdentifierVerifier,
+        Optional::No
+    );
     doc.entries.emplace_back(
         "FileVerifier",
         new FileVerifier,
@@ -279,6 +284,7 @@ TEST_CASE("Documentation: Initializer Constructor", "[documentation]") {
             {"DoubleVerifier", new DoubleVerifier, Optional::No },
             {"IntVerifier", new IntVerifier, Optional::No },
             {"StringVerifier", new StringVerifier, Optional::No },
+            {"IdentifierVerifier", new IdentifierVerifier, Optional::No },
             {"FileVerifier", new FileVerifier, Optional::No },
             {"DirectoryVerifier", new DirectoryVerifier, Optional::No },
             {"DateTimeVerifier", new DateTimeVerifier, Optional::No },
@@ -465,6 +471,69 @@ TEST_CASE("Documentation: StringVerifier", "[documentation]") {
     CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::MissingKey);
 }
 
+TEST_CASE("Documentation: IdentifierVerifier", "[documentation]") {
+    using namespace openspace::documentation;
+    using namespace std::string_literals;
+
+    Documentation doc{
+        {{ "Identifier", new IdentifierVerifier, Optional::No }}
+    };
+
+    ghoul::Dictionary positive;
+    positive.setValue("Identifier", "abcdef"s);
+    TestResult positiveRes = testSpecification(doc, positive);
+    CHECK(positiveRes.success);
+    CHECK(positiveRes.offenses.empty());
+
+    ghoul::Dictionary negativeSpace;
+    negativeSpace.setValue("Identifier", "abc def"s);
+    TestResult negativeRes = testSpecification(doc, negativeSpace);
+    CHECK(!negativeRes.success);
+    REQUIRE(negativeRes.offenses.size() == 1);
+    CHECK(negativeRes.offenses[0].offender == "Identifier");
+    CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification);
+
+    ghoul::Dictionary negativeTab;
+    negativeTab.setValue("Identifier", "abc\tdef"s);
+    negativeRes = testSpecification(doc, negativeTab);
+    CHECK(!negativeRes.success);
+    REQUIRE(negativeRes.offenses.size() == 1);
+    CHECK(negativeRes.offenses[0].offender == "Identifier");
+    CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification);
+
+    ghoul::Dictionary negativeNewline;
+    negativeNewline.setValue("Identifier", "abc\ndef"s);
+    negativeRes = testSpecification(doc, negativeNewline);
+    CHECK(!negativeRes.success);
+    REQUIRE(negativeRes.offenses.size() == 1);
+    CHECK(negativeRes.offenses[0].offender == "Identifier");
+    CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification);
+
+    ghoul::Dictionary negativeCarriageReturn;
+    negativeCarriageReturn.setValue("Identifier", "abc\rdef"s);
+    negativeRes = testSpecification(doc, negativeCarriageReturn);
+    CHECK(!negativeRes.success);
+    REQUIRE(negativeRes.offenses.size() == 1);
+    CHECK(negativeRes.offenses[0].offender == "Identifier");
+    CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification);
+
+    ghoul::Dictionary negativeDot;
+    negativeDot.setValue("Identifier", "abc.def"s);
+    negativeRes = testSpecification(doc, negativeDot);
+    CHECK(!negativeRes.success);
+    REQUIRE(negativeRes.offenses.size() == 1);
+    CHECK(negativeRes.offenses[0].offender == "Identifier");
+    CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification);
+
+    ghoul::Dictionary negativeType;
+    negativeType.setValue("Identifier", 0);
+    negativeRes = testSpecification(doc, negativeType);
+    CHECK_FALSE(negativeRes.success);
+    REQUIRE(negativeRes.offenses.size() == 1);
+    CHECK(negativeRes.offenses[0].offender == "Identifier");
+    CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::WrongType);
+}
+
 TEST_CASE("Documentation: FileVerifier", "[documentation]") {
     using namespace openspace::documentation;
     using namespace std::string_literals;