Improve identifier generation (#304)

* Test identifier generation

* Document magic numbers

Author: 0x2b3bfa0

task/common/identifier.go

@@ -13,17 +13,22 @@ import (
 
 type Identifier string
 
+const (
+	maximumLongLength = 50
+	shortLength       = 16
+)
+
 func (i Identifier) Long() string {
 	re := regexp.MustCompile(`(?s)^tpi-([a-z0-9]+(?:[a-z0-9-]*[a-z0-9])?)-([a-z0-9]+)-([a-z0-9]+)$`)
 
-	if match := re.FindStringSubmatch(string(i)); len(match) > 0 && hash(match[1]+match[2], 4) == match[3] {
+	if match := re.FindStringSubmatch(string(i)); len(match) > 0 && hash(match[1]+match[2], shortLength/2) == match[3] {
 		return match[0]
 	}
 
-	name := normalize(string(i), 50)
-	digest := hash(string(i), 4)
+	name := normalize(string(i), maximumLongLength-shortLength-uint32(len("tpi---")))
+	digest := hash(string(i), shortLength/2)
 
-	return fmt.Sprintf("tpi-%s-%s-%s", name, digest, hash(name+digest, 4))
+	return fmt.Sprintf("tpi-%s-%s-%s", name, digest, hash(name+digest, shortLength/2))
 }
 
 func (i Identifier) Short() string {
@@ -33,13 +38,18 @@ func (i Identifier) Short() string {
 
 // hash deterministically generates a Base36 digest of `size`
 // characters using `identifier` as the seed.
-func hash(identifier string, size uint32) string {
+func hash(identifier string, size uint8) string {
 	digest := sha256.Sum256([]byte(identifier))
 	random := uid.NewRandCustom(bytes.NewReader(digest[:]))
 	encoder := uid.NewEncoderBase36()
-	provider := uid.NewProviderCustom(size, random, encoder)
+	provider := uid.NewProviderCustom(sha256.Size, random, encoder)
+	result := provider.MustGenerate().String()
 
-	return provider.MustGenerate().String()
+	if len(result) < int(size) {
+		panic("not enough bytes to satisfy requested size")
+	}
+
+	return result[:size]
 }
 
 // normalize normalizes user-provided identifiers by adapting them to
@@ -48,13 +58,10 @@ func normalize(identifier string, truncate uint32) string {
 	lowercase := strings.ToLower(identifier)
 
 	normalized := regexp.MustCompile("[^a-z0-9]+").ReplaceAllString(lowercase, "-")
-	normalized = regexp.MustCompile("(^-)|(-$)").ReplaceAllString(normalized, "")
 
 	if len(normalized) > int(truncate) {
 		normalized = normalized[:truncate]
 	}
 
-	return normalized
+	return regexp.MustCompile("(^-)|(-$)").ReplaceAllString(normalized, "")
 }
-
-// func NormalizeIdentifier(identifier common.Identifier, long bool) string {

task/common/identifier_test.go

@@ -0,0 +1,47 @@
+package common
+
+import (
+	"testing"
+
+	"github.com/brianvoe/gofakeit/v6"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIdentifier(t *testing.T) {
+	name := gofakeit.NewCrypto().Sentence(512)
+
+	t.Run("idempotence", func(t *testing.T) {
+		identifier := Identifier(name)
+
+		once := identifier.Long()
+		twice := Identifier(once).Long()
+		require.Equal(t, once, twice)
+	})
+
+	t.Run("stability", func(t *testing.T) {
+		identifier := Identifier(name)
+
+		require.Equal(t, identifier.Long(), identifier.Long())
+		require.Equal(t, identifier.Short(), identifier.Short())
+	})
+
+	t.Run("homogeneity", func(t *testing.T) {
+		identifier := Identifier(name)
+
+		long := identifier.Long()
+		short := identifier.Short()
+
+		require.Regexp(t, "^tpi-[a-z0-9-]+$", long)
+		require.Regexp(t, "^[a-z0-9]+$", short)
+
+		require.LessOrEqual(t, len(long), maximumLongLength)
+		require.Equal(t, len(short), shortLength)
+	})
+
+	t.Run("compatibility", func(t *testing.T) {
+		identifier := Identifier("test")
+
+		require.Equal(t, "tpi-test-3z4xlzwq-3u0vweb4", identifier.Long())
+		require.Equal(t, "3z4xlzwq3u0vweb4", identifier.Short())
+	})
+}