Skip to content
8 changes: 7 additions & 1 deletion pkg/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ import (
)

var (
ErrNotFound = errors.New("not found")
ErrNotFound = errors.New("not found")
ErrUnimplemented = errors.New("unimplemented")
)

// Getter wraps a method to read from cache.
type Getter interface {
Get(key interface{}) (interface{}, error)
GetAll() ([]interface{}, error)
}

// Putter wraps a method to write to cache.
Expand Down Expand Up @@ -82,3 +84,7 @@ func (mg *multiGetter) Get(key interface{}) (interface{}, error) {
}
return nil, firstErr
}

func (mg *multiGetter) GetAll() ([]interface{}, error) {
return nil, ErrUnimplemented
}
4 changes: 4 additions & 0 deletions pkg/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ func (f getterFunc) Get(key interface{}) (interface{}, error) {
return f(key)
}

func (f getterFunc) GetAll() ([]interface{}, error) {
return nil, ErrUnimplemented
}

func TestMultiGetter(t *testing.T) {
value := "ok"
err := errors.New("err")
Expand Down
4 changes: 4 additions & 0 deletions pkg/cache/memorycache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,7 @@ func (c *Cache) Delete(key interface{}) error {
c.values.Delete(key)
return nil
}

func (c *Cache) GetAll() ([]interface{}, error) {
return nil, cache.ErrUnimplemented
}
4 changes: 4 additions & 0 deletions pkg/cache/memorycache/lru_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,7 @@ func (c *LRUCache) Delete(key interface{}) error {
c.cache.Remove(key)
return nil
}

func (c *LRUCache) GetAll() ([]interface{}, error) {
return nil, cache.ErrUnimplemented
}
4 changes: 4 additions & 0 deletions pkg/cache/memorycache/ttl_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,7 @@ func (c *TTLCache) Delete(key interface{}) error {
c.entries.Delete(key)
return nil
}

func (c *TTLCache) GetAll() ([]interface{}, error) {
return nil, cache.ErrUnimplemented
}
5 changes: 4 additions & 1 deletion pkg/cache/rediscache/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
name = "go_default_library",
srcs = ["cache.go"],
srcs = [
"cache.go",
"hashcache.go",
],
importpath = "github.com/pipe-cd/pipe/pkg/cache/rediscache",
visibility = ["//visibility:public"],
deps = [
Expand Down
4 changes: 4 additions & 0 deletions pkg/cache/rediscache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,7 @@ func (c *RedisCache) Delete(k interface{}) error {
_, err := conn.Do("DEL", k)
return err
}

func (c *RedisCache) GetAll() ([]interface{}, error) {
return nil, cache.ErrUnimplemented
}
79 changes: 79 additions & 0 deletions pkg/cache/rediscache/hashcache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright 2021 The PipeCD 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 rediscache

import (
"time"

redigo "github.com/gomodule/redigo/redis"

"github.com/pipe-cd/pipe/pkg/cache"
"github.com/pipe-cd/pipe/pkg/redis"
)

type RedisHashCache struct {
RedisCache
key string
}

func NewHashCache(redis redis.Redis, ttl time.Duration, key string) *RedisHashCache {
Comment thread
khanhtc1202 marked this conversation as resolved.
Outdated
return &RedisHashCache{
RedisCache: RedisCache{
redis: redis,
},
key: key,
}
}

func NewTTLHashCache(redis redis.Redis, ttl time.Duration, key string) *RedisHashCache {
return &RedisHashCache{
RedisCache: RedisCache{
redis: redis,
ttl: uint(ttl.Seconds()),
},
key: key,
}
}

func (r *RedisHashCache) Put(k interface{}, v interface{}) error {
conn := r.redis.Get()
defer conn.Close()
_, err := conn.Do("HSET", r.key, k, v)
if r.ttl != 0 {
_, err = conn.Do("EXPIRE", r.key, r.ttl)
}
return err
}

func (r *RedisHashCache) GetAll() ([]interface{}, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who uses GetAll() currently? Any plan to use it later?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this PR is focusing on storeing

conn := r.redis.Get()
defer conn.Close()
reply, err := redigo.StringMap(conn.Do("HGETALL", r.key))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can not use StringMap in this case.
Because HGETALL is returning an array that containing both field and value.
So we have to use Array function to parse the reply. https://pkg.go.dev/github.com/gomodule/redigo/redis#Values

One more point is that instead of returning []interface{} maybe we should return map[interface{}]interface{}.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we adopt map[interface{}]interface{} as return value, then the current redigo.StringMap is good enough since its convert reply to map[string]string, how do you think about that 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got it. Agree about that. 👍

@khanhtc1202 khanhtc1202 Jul 7, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopted returning map[interface{}]interface{} idea on d7e89e0 🙆‍♀️
It looks much better now, thx 👍

if err != nil {
if err == redigo.ErrNil {
return nil, cache.ErrNotFound
}
return nil, err
}
if len(reply) == 0 {
return nil, cache.ErrNotFound
}

out := make([]interface{}, 0, len(reply))
for _, v := range reply {
out = append(out, v)
}
return out, nil
}