19980: System properties only require arv: prefix

The decision to require a following alphabetic character was
well-intentioned but misplaced: it doesn't stop these properties from
existing, just changes the layer they would be defined at.

It might make sense to add some name rules to properties, but that would
be better done across the board as a separate story, not as a specific
corner case of system properties.

Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith@curii.com>

diff --git a/sdk/go/arvados/vocabulary.go b/sdk/go/arvados/vocabulary.go
index caab6e8fbee0a5b45690735da0505bea105886d7..1df43b5fb8f1ebc92fa8ffa8f8317badeb2513a1 100644 (file)
--- a/sdk/go/arvados/vocabulary.go
+++ b/sdk/go/arvados/vocabulary.go
@@ -10,13 +10,10 @@ import (
        "errors"
        "fmt"
        "reflect"
-       "regexp"
        "strconv"
        "strings"
 )
 
-const systemKeyPattern = `^arv:[a-zA-Z]`
-
 type Vocabulary struct {
        reservedTagKeys map[string]bool          `json:"-"`
        StrictTags      bool                     `json:"strict_tags"`
@@ -30,8 +27,8 @@ type VocabularyTag struct {
 }
 
 // Cannot have a constant map in Go, so we have to use a function.
-// If you are adding a new system property, it SHOULD match `systemKeyPattern`
-// above, and Check will allow it. This map is for historical exceptions that
+// If you are adding a new system property, it SHOULD start with `arv:`,
+// and Check will allow it. This map is for historical exceptions that
 // predate standardizing on this prefix.
 func (v *Vocabulary) systemTagKeys() map[string]bool {
        return map[string]bool{
@@ -271,13 +268,9 @@ func (v *Vocabulary) Check(data map[string]interface{}) error {
        if v == nil {
                return nil
        }
-       systemKeyRegexp, err := regexp.Compile(systemKeyPattern)
-       if err != nil {
-               return err
-       }
        for key, val := range data {
                // Checks for key validity
-               if systemKeyRegexp.MatchString(key) || v.reservedTagKeys[key] {
+               if strings.HasPrefix(key, "arv:") || v.reservedTagKeys[key] {
                        // Allow reserved keys to be used even if they are not defined in
                        // the vocabulary no matter its strictness.
                        continue

diff --git a/sdk/go/arvados/vocabulary_test.go b/sdk/go/arvados/vocabulary_test.go
index c06efda3e0a3de0fa91b38bf9831935b1ffb8bba..af62833a312cab08c6d63d46e695c8ce96c2e5a3 100644 (file)
--- a/sdk/go/arvados/vocabulary_test.go
+++ b/sdk/go/arvados/vocabulary_test.go
@@ -6,7 +6,6 @@ package arvados
 
 import (
        "encoding/json"
-       "fmt"
        "regexp"
        "strings"
 
@@ -312,28 +311,21 @@ func (s *VocabularySuite) TestValidSystemProperties(c *check.C) {
        c.Check(s.testVoc.Check(properties), check.IsNil)
 }
 
-func (s *VocabularySuite) TestSystemPropertiesFirstCharacterAlphabetic(c *check.C) {
-       s.testVoc.StrictTags = true
-       properties := map[string]interface{}{"arv:": "value"}
-       c.Check(s.testVoc.Check(properties), check.NotNil)
-       // If we expand the list of allowed characters in the future, these lists
-       // may need adjustment to match.
-       for _, prefix := range []string{" ", ".", "_", "-", "1"} {
-               for _, suffix := range []string{"", "invalid"} {
-                       key := fmt.Sprintf("arv:%s%s", prefix, suffix)
-                       properties := map[string]interface{}{key: "value"}
-                       c.Check(s.testVoc.Check(properties), check.NotNil)
-               }
-       }
-}
-
 func (s *VocabularySuite) TestSystemPropertiesPrefixTypo(c *check.C) {
        s.testVoc.StrictTags = true
        for _, key := range []string{
+               // Extra characters in prefix
                "arv :foo",
+               " arv:foo",
+               // Wrong punctuation
+               "arv.foo",
+               "arv-foo",
+               "arv_foo",
+               // Wrong case
+               "Arv:foo",
+               // Wrong word
                "arvados",
                "arvados:foo",
-               "Arv:foo",
        } {
                properties := map[string]interface{}{key: "value"}
                c.Check(s.testVoc.Check(properties), check.NotNil)