[gold] Extract out Mapper.ImagePaths as a function defined alongside the ImageLoader struct.
Bug: skia:9350
Change-Id: Ibc6717b9a406ce629ed573659122752fa8339268
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/236301
Commit-Queue: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Auto-Submit: Leandro Lovisolo <lovisolo@google.com>
diff --git a/golden/go/diffstore/imgloader.go b/golden/go/diffstore/imgloader.go
index dca03f1..5ca8ce3 100644
--- a/golden/go/diffstore/imgloader.go
+++ b/golden/go/diffstore/imgloader.go
@@ -61,6 +61,15 @@
mapper mapper.Mapper
}
+// ImagePaths returns the storage paths for a given image ID. The first return
+// value is the local file path used to store the image on disk and serve it
+// over HTTP. The second return value is the GCS path (not including the bucket).
+func ImagePaths(imageID types.Digest) (string, string) {
+ gsPath := fmt.Sprintf("%s.%s", imageID, common.IMG_EXTENSION)
+ localPath := fileutil.TwoLevelRadixPath(gsPath)
+ return localPath, gsPath
+}
+
// Creates a new instance of ImageLoader.
func NewImgLoader(client *http.Client, baseDir, imgDir string, gsBucketNames []string, gsImageBaseDir string, maxCacheSize int, m mapper.Mapper) (*ImageLoader, error) {
storageClient, err := storage.NewClient(context.Background(), option.WithHTTPClient(client))
@@ -175,14 +184,14 @@
// IsOnDisk returns true if the image that corresponds to the given imageID is in the disk cache.
func (il *ImageLoader) IsOnDisk(imageID types.Digest) bool {
- localRelPath, _ := il.mapper.ImagePaths(imageID)
+ localRelPath, _ := ImagePaths(imageID)
return fileutil.FileExists(filepath.Join(il.localImgDir, localRelPath))
}
// PurgeImages removes the images that correspond to the given images.
func (il *ImageLoader) PurgeImages(images types.DigestSlice, purgeGCS bool) error {
for _, id := range images {
- localRelPath, gsRelPath := il.mapper.ImagePaths(id)
+ localRelPath, gsRelPath := ImagePaths(id)
localPath := filepath.Join(il.localImgDir, localRelPath)
if fileutil.FileExists(localPath) {
if err := os.Remove(localPath); err != nil {
@@ -201,7 +210,7 @@
// It loads an image file either from disk or from Google storage.
func (il *ImageLoader) imageLoadWorker(priority int64, imageID types.Digest) (interface{}, error) {
// Check if the image is in the disk cache.
- localRelPath, gsRelPath := il.mapper.ImagePaths(imageID)
+ localRelPath, gsRelPath := ImagePaths(imageID)
localPath := filepath.Join(il.localImgDir, localRelPath)
sklog.Debugf("Looking for image with id %s", imageID)
if fileutil.FileExists(localPath) {
@@ -238,7 +247,7 @@
writeDoneCh := make(chan bool)
go func() {
defer close(writeDoneCh)
- localRelPath, _ := il.mapper.ImagePaths(imageID)
+ localRelPath, _ := ImagePaths(imageID)
p := filepath.Join(il.localImgDir, localRelPath)
if err := common.SaveFilePath(p, bytes.NewBuffer(imgBytes)); err != nil {
sklog.Errorf("Could not write file to disk at %s: %s", p, err)
diff --git a/golden/go/diffstore/imgloader_test.go b/golden/go/diffstore/imgloader_test.go
index d425745..ed9fca6 100644
--- a/golden/go/diffstore/imgloader_test.go
+++ b/golden/go/diffstore/imgloader_test.go
@@ -73,3 +73,14 @@
assert.NoError(t, err)
return workingDir, tile, imgLoader, cleanup
}
+
+func TestImagePaths(t *testing.T) {
+ unittest.SmallTest(t)
+
+ digest := types.Digest("098f6bcd4621d373cade4e832627b4f6")
+ expectedLocalPath := filepath.Join("09", "8f", string(digest)+".png")
+ expectedGSPath := string(digest + ".png")
+ localPath, gsPath := ImagePaths(digest)
+ assert.Equal(t, expectedLocalPath, localPath)
+ assert.Equal(t, expectedGSPath, gsPath)
+}
diff --git a/golden/go/diffstore/mapper/disk_mapper/disk_mapper.go b/golden/go/diffstore/mapper/disk_mapper/disk_mapper.go
index bf1d954..c23e359 100644
--- a/golden/go/diffstore/mapper/disk_mapper/disk_mapper.go
+++ b/golden/go/diffstore/mapper/disk_mapper/disk_mapper.go
@@ -1,18 +1,11 @@
package disk_mapper
import (
- "fmt"
"image"
- "go.skia.org/infra/go/fileutil"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/diff"
"go.skia.org/infra/golden/go/diffstore/mapper"
- "go.skia.org/infra/golden/go/types"
-)
-
-const (
- imgExtension = "png"
)
// DiskMapper implements the Mapper interface.
@@ -37,12 +30,5 @@
return diff.DefaultDiffFn(left, right)
}
-// ImagePaths implements the DiffStoreMapper interface.
-func (g *DiskMapper) ImagePaths(imageID types.Digest) (string, string) {
- gsPath := fmt.Sprintf("%s.%s", imageID, imgExtension)
- localPath := fileutil.TwoLevelRadixPath(gsPath)
- return localPath, gsPath
-}
-
// Make sure DiskMapper fulfills the Mapper interface
var _ mapper.Mapper = (*DiskMapper)(nil)
diff --git a/golden/go/diffstore/mapper/disk_mapper/disk_mapper_test.go b/golden/go/diffstore/mapper/disk_mapper/disk_mapper_test.go
deleted file mode 100644
index 94466be..0000000
--- a/golden/go/diffstore/mapper/disk_mapper/disk_mapper_test.go
+++ /dev/null
@@ -1,31 +0,0 @@
-package disk_mapper
-
-import (
- "path/filepath"
- "testing"
-
- "github.com/stretchr/testify/assert"
- "go.skia.org/infra/go/testutils/unittest"
- "go.skia.org/infra/golden/go/diffstore/testutils"
- "go.skia.org/infra/golden/go/types"
-)
-
-const (
- // Arbitrary MD5 digest.
- imgOne = types.Digest("098f6bcd4621d373cade4e832627b4f6")
-
- // PNG extension.
- png = ".png"
-)
-
-func TestImagePaths(t *testing.T) {
- unittest.SmallTest(t)
-
- dm := New(&testutils.DummyDiffMetrics{})
-
- expectedLocalPath := filepath.Join("09", "8f", string(imgOne)+png)
- expectedGSPath := string(imgOne + png)
- localPath, gsPath := dm.ImagePaths(imgOne)
- assert.Equal(t, expectedLocalPath, localPath)
- assert.Equal(t, expectedGSPath, gsPath)
-}
diff --git a/golden/go/diffstore/mapper/mapper.go b/golden/go/diffstore/mapper/mapper.go
index 99eadb3..edb9726 100644
--- a/golden/go/diffstore/mapper/mapper.go
+++ b/golden/go/diffstore/mapper/mapper.go
@@ -4,7 +4,6 @@
"image"
"go.skia.org/infra/go/util"
- "go.skia.org/infra/golden/go/types"
)
// Mapper is the interface to define how the diff metric between two images
@@ -19,11 +18,4 @@
// above. It is also what is returned by the Get(...) function of the
// DiffStore interface.
DiffFn(*image.NRGBA, *image.NRGBA) interface{}
-
- // ImagePaths returns the storage paths for a given image ID. The first return
- // value is the local file path used to store the image on disk and serve it
- // over HTTP. The second return value is the GCS path (not including the bucket).
- // TODO(kjlubick): It might be nice to have Mapper just focus on the
- // diff metric and have a different interface for the disk storing.
- ImagePaths(id types.Digest) (string, string)
}
diff --git a/golden/go/diffstore/mem_diffstore.go b/golden/go/diffstore/mem_diffstore.go
index 3566109..2532ff1 100644
--- a/golden/go/diffstore/mem_diffstore.go
+++ b/golden/go/diffstore/mem_diffstore.go
@@ -286,7 +286,7 @@
noCacheNotFound(w, r)
return
}
- localRelPath, _ = m.mapper.ImagePaths(imgDigest)
+ localRelPath, _ = ImagePaths(imgDigest)
// Rewrite the path to include the mapper's custom local path construction format.
r.URL.Path = path.Join(DEFAULT_IMG_DIR_NAME, localRelPath)
@@ -312,8 +312,8 @@
}
// Get their absolute paths.
- leftImgLocalRelPath, _ := m.mapper.ImagePaths(leftImgDigest)
- rightImgLocalRelPath, _ := m.mapper.ImagePaths(rightImgDigest)
+ leftImgLocalRelPath, _ := ImagePaths(leftImgDigest)
+ rightImgLocalRelPath, _ := ImagePaths(rightImgDigest)
leftImgPath := filepath.Join(absPath, DEFAULT_IMG_DIR_NAME, leftImgLocalRelPath)
rightImgPath := filepath.Join(absPath, DEFAULT_IMG_DIR_NAME, rightImgLocalRelPath)
