fix nits, add unit tests

Signed-off-by: Bob Callaway <bcallaway@google.com>

pkg/api/error.go

@@ -42,7 +42,7 @@ const (
 	malformedUUID                  = "UUID must be a 64-character hexadecimal string"
 	malformedPublicKey             = "public key provided could not be parsed"
 	failedToGenerateCanonicalKey   = "error generating canonicalized public key"
-	redisUnexpectedResult          = "unexpected result from searching index"
+	indexStorageUnexpectedResult   = "unexpected result from searching index"
 	lastSizeGreaterThanKnown       = "the tree size requested(%d) was greater than what is currently observable(%d)"
 	signingError                   = "error signing"
 	sthGenerateError               = "error generating signed tree head"

pkg/api/index.go

@@ -47,7 +47,7 @@ func SearchIndexHandler(params index.SearchIndexParams) middleware.Responder {
 		sha := util.PrefixSHA(params.Query.Hash)
 		resultUUIDs, err := indexStorageClient.LookupIndices(httpReqCtx, strings.ToLower(sha))
 		if err != nil {
-			return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("redis client: %w", err), redisUnexpectedResult)
+			return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("index storage error: %w", err), indexStorageUnexpectedResult)
 		}
 		result.Add(resultUUIDs)
 	}
@@ -74,14 +74,14 @@ func SearchIndexHandler(params index.SearchIndexParams) middleware.Responder {
 		keyHash := sha256.Sum256(canonicalKey)
 		resultUUIDs, err := indexStorageClient.LookupIndices(httpReqCtx, strings.ToLower(hex.EncodeToString(keyHash[:])))
 		if err != nil {
-			return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("redis client: %w", err), redisUnexpectedResult)
+			return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("index storage error: %w", err), indexStorageUnexpectedResult)
 		}
 		result.Add(resultUUIDs)
 	}
 	if params.Query.Email != "" {
 		resultUUIDs, err := indexStorageClient.LookupIndices(httpReqCtx, strings.ToLower(params.Query.Email.String()))
 		if err != nil {
-			return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("redis client: %w", err), redisUnexpectedResult)
+			return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("index storage error: %w", err), indexStorageUnexpectedResult)
 		}
 		result.Add(resultUUIDs)
 	}

pkg/indexstorage/redis/redis.go

@@ -25,6 +25,7 @@ import (
 
 const ProviderType = "redis"
 
+// IndexStorageProvider implements indexstorage.IndexStorage
 type IndexStorageProvider struct {
 	client *redis.Client
 }
@@ -39,19 +40,22 @@ func NewProvider(address, port string) (*IndexStorageProvider, error) {
 	return provider, nil
 }
 
+// LookupIndices looks up and returns all indices for the specified key. The key value will be canonicalized
+// by converting all characters into a lowercase value before looking up in Redis
 func (isp *IndexStorageProvider) LookupIndices(ctx context.Context, key string) ([]string, error) {
 	if isp.client == nil {
 		return []string{}, errors.New("redis client has not been initialized")
 	}
 	return isp.client.LRange(ctx, strings.ToLower(key), 0, -1).Result()
 }
 
-func (isp *IndexStorageProvider) WriteIndex(ctx context.Context, key, value string) error {
+// WriteIndex adds the index for the specified key. The key value will be canonicalized
+// by converting all characters into a lowercase value before appending the index in Redis
+func (isp *IndexStorageProvider) WriteIndex(ctx context.Context, key, index string) error {
 	if isp.client == nil {
 		return errors.New("redis client has not been initialized")
 	}
-	_, err := isp.client.LPush(ctx, strings.ToLower(key), value).Result()
-	if err != nil {
+	if _, err := isp.client.LPush(ctx, strings.ToLower(key), index).Result(); err != nil {
 		return fmt.Errorf("redis client: %w", err)
 	}
 	return nil

pkg/indexstorage/redis/redis_test.go

@@ -0,0 +1,100 @@
+// Copyright 2023 The Sigstore Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package redis
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/go-redis/redismock/v9"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"go.uber.org/goleak"
+)
+
+func TestLookupIndices(t *testing.T) {
+	key := "87c1b129fbadd7b6e9abc0a9ef7695436d767aece042bec198a97e949fcbe14c"
+	value := []string{"1e1f2c881ae0608ec77ebf88a75c66d3099113a7343238f2f7a0ebb91a4ed335"}
+	redisClient, mock := redismock.NewClientMock()
+	mock.Regexp().ExpectLRange(key, 0, -1).SetVal(value)
+
+	isp := IndexStorageProvider{redisClient}
+
+	indices, err := isp.LookupIndices(context.Background(), key)
+	if err != nil {
+		t.Error(err)
+	}
+
+	less := func(a, b string) bool { return a < b }
+	if cmp.Diff(value, indices, cmpopts.SortSlices(less)) != "" {
+		t.Errorf("expected %s, got %s", value, indices)
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Error(err)
+	}
+
+	mock.ClearExpect()
+	errRedis := errors.New("redis error")
+	mock.Regexp().ExpectLRange(key, 0, -1).SetErr(errRedis)
+	if _, err := isp.LookupIndices(context.Background(), key); err == nil {
+		t.Error("unexpected success")
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Error(err)
+	}
+}
+
+func TestWriteIndex(t *testing.T) {
+	key := "87c1b129fbadd7b6e9abc0a9ef7695436d767aece042bec198a97e949fcbe14c"
+	value := []string{"1e1f2c881ae0608ec77ebf88a75c66d3099113a7343238f2f7a0ebb91a4ed335"}
+	redisClient, mock := redismock.NewClientMock()
+	mock.Regexp().ExpectLPush(key, value).SetVal(1)
+
+	isp := IndexStorageProvider{redisClient}
+	if err := isp.WriteIndex(context.Background(), key, value[0]); err != nil {
+		t.Error(err)
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Error(err)
+	}
+
+	mock.ClearExpect()
+	errRedis := errors.New("redis error")
+	mock.Regexp().ExpectLPush(key, value).SetErr(errRedis)
+	if err := isp.WriteIndex(context.Background(), key, value[0]); err == nil {
+		t.Error("unexpected success")
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Error(err)
+	}
+}
+
+func TestUninitializedClient(t *testing.T) {
+	// this is not initialized with a real Redis client
+	isp := IndexStorageProvider{}
+	if _, err := isp.LookupIndices(context.Background(), "key"); err == nil {
+		t.Error("unexpected success")
+	}
+	if err := isp.WriteIndex(context.Background(), "key", "value"); err == nil {
+		t.Error("unexpected success")
+	}
+}
+
+func TestMain(m *testing.M) {
+	goleak.VerifyTestMain(m)
+}