chore: do joins on the query and make error messages match

flipt-io · Mar 29, 2023 · d9500dc · d9500dc
1 parent 75cb62c
commit d9500dc
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 29 deletions.
diff --git a/internal/storage/sql/common/rule.go b/internal/storage/sql/common/rule.go
@@ -436,37 +436,23 @@ func (s *Store) distributionValidationHelper(ctx context.Context, distributionRe
 	GetVariantId() string
 	GetRuleId() string
 }) error {
-	var ruleNamespace, variantNamespace, flagNamespace string
-
-	if err := s.builder.Select("namespace_key").
+	var count int
+	if err := s.builder.Select("COUNT(*)").
 		From("rules").
-		Where(sq.Eq{"id": distributionRequest.GetRuleId()}).
-		QueryRowContext(ctx).
-		Scan(&ruleNamespace); err != nil {
-		return err
-	}
-
-	if err := s.builder.Select("namespace_key").
-		From("variants").
-		Where(sq.Eq{"id": distributionRequest.GetVariantId()}).
+		Join("variants USING (namespace_key)").
+		Join("flags USING (namespace_key)").
+		Where(sq.Eq{
+			"namespace_key": distributionRequest.GetNamespaceKey(),
+			"rules.id":      distributionRequest.GetRuleId(),
+			"variants.id":   distributionRequest.GetVariantId(),
+			"flags.\"key\"": distributionRequest.GetFlagKey(),
+		}).
 		QueryRowContext(ctx).
-		Scan(&variantNamespace); err != nil {
-		return err
-	}
-
-	if err := s.builder.Select("namespace_key").
-		From("flags").
-		Where(sq.And{sq.Eq{"\"key\"": distributionRequest.GetFlagKey()}, sq.Eq{"namespace_key": distributionRequest.GetNamespaceKey()}}).
-		QueryRowContext(ctx).
-		Scan(&flagNamespace); err != nil {
+		Scan(&count); err != nil {
 		return err
 	}
 
-	// We return the sql.ErrNoRows here so that upstream we can return a more appropriate
-	// error to the client.
-	//nolint:gocritic
-	valid := (ruleNamespace == variantNamespace) && (ruleNamespace == flagNamespace)
-	if !valid {
+	if count < 1 {
 		return sql.ErrNoRows
 	}
 

diff --git a/internal/storage/sql/mysql/mysql.go b/internal/storage/sql/mysql/mysql.go
@@ -172,7 +172,7 @@ func (s *Store) CreateDistribution(ctx context.Context, r *flipt.CreateDistribut
 		var merr *mysql.MySQLError
 
 		if errors.As(err, &merr) && merr.Number == constraintForeignKeyErrCode {
-			return nil, errs.ErrNotFoundf("variant %q, rule %q, flag %q/%q in namespace", r.VariantId, r.RuleId, r.NamespaceKey, r.FlagKey)
+			return nil, errs.ErrNotFoundf("variant %q, rule %q, flag %q in namespace %q", r.VariantId, r.RuleId, r.FlagKey, r.NamespaceKey)
 		}
 
 		return nil, err

diff --git a/internal/storage/sql/postgres/postgres.go b/internal/storage/sql/postgres/postgres.go
@@ -172,7 +172,7 @@ func (s *Store) CreateDistribution(ctx context.Context, r *flipt.CreateDistribut
 		var perr *pq.Error
 
 		if errors.As(err, &perr) && perr.Code.Name() == constraintForeignKeyErr {
-			return nil, errs.ErrNotFoundf("variant %q, rule %q, flag %q/%q in namespace", r.VariantId, r.RuleId, r.NamespaceKey, r.FlagKey)
+			return nil, errs.ErrNotFoundf("variant %q, rule %q, flag %q in namespace %q", r.VariantId, r.RuleId, r.FlagKey, r.NamespaceKey)
 		}
 
 		return nil, err

diff --git a/internal/storage/sql/sqlite/sqlite.go b/internal/storage/sql/sqlite/sqlite.go
@@ -169,7 +169,7 @@ func (s *Store) CreateDistribution(ctx context.Context, r *flipt.CreateDistribut
 		var serr sqlite3.Error
 
 		if errors.As(err, &serr) && serr.Code == sqlite3.ErrConstraint {
-			return nil, errs.ErrNotFoundf("variant %q, rule %q, flag %q/%q in namespace", r.VariantId, r.RuleId, r.NamespaceKey, r.FlagKey)
+			return nil, errs.ErrNotFoundf("variant %q, rule %q, flag %q in namespace %q", r.VariantId, r.RuleId, r.FlagKey, r.NamespaceKey)
 		}
 
 		return nil, err