Repository: fsouza/fake-gcs-server
Pull request #140 (2019-11-22 15:17:28)

Support to queries against all versions (in versioned and not versioned a buckets)
_note: incremental PR: the review and potential merge should go after https://github.com/fsouza/fake-gcs-server/pull/60_ 

Support for querying all versions, that means multiple generations of each file are expected in versioned buckets where there has been overwrites or deletions

Done:
* some queries with "versioning equal to true" at fakestorage level, testing a couple of scenarios
* Incorporating two simple calls to list query (one with versioning equal to false, as we had, and another with true) in the backend tests (`TestObjectCRUD`)
* implementation (easiest part actually)


dcaba: rebased & conflicts solved, @fsouza . Thanks for the patience,

Patch:
diff --git a/fakestorage/object.go b/fakestorage/object.go
index 55ba836..c1cb70a 100644
--- a/fakestorage/object.go
+++ b/fakestorage/object.go
@@ -74,8 +74,8 @@ func (s *Server) createObject(obj Object) error {
 
 // ListObjects returns a sorted list of objects that match the given criteria,
 // or an error if the bucket doesn't exist.
-func (s *Server) ListObjects(bucketName, prefix, delimiter string) ([]Object, []string, error) {
-	backendObjects, err := s.backend.ListObjects(bucketName)
+func (s *Server) ListObjects(bucketName, prefix, delimiter string, versions bool) ([]Object, []string, error) {
+	backendObjects, err := s.backend.ListObjects(bucketName, versions)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -186,7 +186,9 @@ func (s *Server) listObjects(w http.ResponseWriter, r *http.Request) {
 	bucketName := mux.Vars(r)["bucketName"]
 	prefix := r.URL.Query().Get("prefix")
 	delimiter := r.URL.Query().Get("delimiter")
-	objs, prefixes, err := s.ListObjects(bucketName, prefix, delimiter)
+	versions := r.URL.Query().Get("versions")
+
+	objs, prefixes, err := s.ListObjects(bucketName, prefix, delimiter, versions == "true")
 	encoder := json.NewEncoder(w)
 	if err != nil {
 		w.WriteHeader(http.StatusNotFound)
diff --git a/internal/backend/fs.go b/internal/backend/fs.go
index 1c3d5f3..fa914d8 100644
--- a/internal/backend/fs.go
+++ b/internal/backend/fs.go
@@ -109,9 +109,10 @@ func (s *StorageFS) CreateObject(obj Object) error {
 }
 
 // ListObjects lists the objects in a given bucket with a given prefix and delimeter
-func (s *StorageFS) ListObjects(bucketName string) ([]Object, error) {
+func (s *StorageFS) ListObjects(bucketName string, versions bool) ([]Object, error) {
 	s.mtx.RLock()
 	defer s.mtx.RUnlock()
+
 	infos, err := ioutil.ReadDir(path.Join(s.rootDir, url.PathEscape(bucketName)))
 	if err != nil {
 		return nil, err
diff --git a/internal/backend/memory.go b/internal/backend/memory.go
index 61f4acf..d7aeda4 100644
--- a/internal/backend/memory.go
+++ b/internal/backend/memory.go
@@ -167,14 +167,17 @@ func (s *StorageMemory) CreateObject(obj Object) error {
 }
 
 // ListObjects lists the objects in a given bucket with a given prefix and delimeter
-func (s *StorageMemory) ListObjects(bucketName string) ([]Object, error) {
+func (s *StorageMemory) ListObjects(bucketName string, versions bool) ([]Object, error) {
 	s.mtx.RLock()
 	defer s.mtx.RUnlock()
 	bucketInMemory, err := s.getBucketInMemory(bucketName)
 	if err != nil {
 		return []Object{}, err
 	}
-	return bucketInMemory.activeObjects, nil
+	if !versions {
+		return bucketInMemory.activeObjects, nil
+	}
+	return append(bucketInMemory.activeObjects, bucketInMemory.archivedObjects...), nil
 }
 
 // GetObject get an object by bucket and name
diff --git a/internal/backend/storage.go b/internal/backend/storage.go
index 6199198..d6c90a3 100644
--- a/internal/backend/storage.go
+++ b/internal/backend/storage.go
@@ -11,7 +11,7 @@ type Storage interface {
 	ListBuckets() ([]Bucket, error)
 	GetBucket(name string) (Bucket, error)
 	CreateObject(obj Object) error
-	ListObjects(bucketName string) ([]Object, error)
+	ListObjects(bucketName string, versions bool) ([]Object, error)
 	GetObject(bucketName, objectName string) (Object, error)
 	GetObjectWithGeneration(bucketName, objectName string, generation int64) (Object, error)
 	DeleteObject(bucketName, objectName string) error


Test patch:
diff --git a/fakestorage/object_test.go b/fakestorage/object_test.go
index 794c657..7e9f16e 100644
--- a/fakestorage/object_test.go
+++ b/fakestorage/object_test.go
@@ -667,6 +667,166 @@ func TestServerClientListAfterCreate(t *testing.T) {
 	}
 }
 
+func contains(s []int64, e int64) bool {
+	for _, a := range s {
+		if a == e {
+			return true
+		}
+	}
+	return false
+}
+
+func unique(in []string) []string {
+	out := []string{}
+	found := make(map[string]bool)
+	for _, entry := range in {
+		if _, value := found[entry]; !value {
+			found[entry] = true
+			out = append(out, entry)
+		}
+	}
+	return out
+}
+
+func TestServerClientListAfterCreateQueryingAllVersions(t *testing.T) {
+	const initialGeneration = 1234
+	const finalGeneration = initialGeneration + 1
+	type listTestForQueryWithVersions struct {
+		listTest
+		expectedGenerations []int64
+		expectedNumObjects  int
+		versioningEnabled   bool
+		withOverwrites      bool
+	}
+	tests := []listTestForQueryWithVersions{
+		{
+			listTest{"no prefix, no delimiter, multiple objects, with versioning and overwrites",
+				"some-bucket",
+				&storage.Query{Versions: true},
+				[]string{
+					"img/brand.jpg",
+					"img/hi-res/party-01.jpg",
+					"img/hi-res/party-02.jpg",
+					"img/hi-res/party-03.jpg",
+					"img/low-res/party-01.jpg",
+					"img/low-res/party-02.jpg",
+					"img/low-res/party-03.jpg",
+					"video/hi-res/some_video_1080p.mp4",
+				},
+				nil},
+			[]int64{initialGeneration, finalGeneration},
+			8 * 2,
+			true,
+			true,
+		},
+		{
+			listTest{"no prefix, no delimiter, single object, with versioning and overwrites",
+				"other-bucket",
+				&storage.Query{Versions: true},
+				[]string{
+					"static/css/style.css",
+				},
+				nil},
+			[]int64{initialGeneration, finalGeneration},
+			2,
+			true,
+			true,
+		},
+		{
+			listTest{"no prefix, no delimiter, single object, without versioning and overwrites",
+				"other-bucket",
+				&storage.Query{Versions: true},
+				[]string{
+					"static/css/style.css",
+				},
+				nil},
+			[]int64{finalGeneration},
+			1,
+			false,
+			true,
+		},
+		{
+			listTest{"no prefix, no delimiter, single object, without versioning neither overwrites",
+				"other-bucket",
+				&storage.Query{Versions: true},
+				[]string{
+					"static/css/style.css",
+				},
+				nil},
+			[]int64{initialGeneration},
+			1,
+			false,
+			false,
+		},
+		{
+			listTest{"no prefix, no delimiter, single object, with versioning but no overwrites",
+				"other-bucket",
+				&storage.Query{Versions: true},
+				[]string{
+					"static/css/style.css",
+				},
+				nil},
+			[]int64{initialGeneration},
+			1,
+			true,
+			false,
+		},
+		{
+			listTest{"no prefix, no delimiter, no objects, versioning and overwrites",
+				"empty-bucket",
+				&storage.Query{Versions: true},
+				[]string{},
+				nil},
+			[]int64{},
+			0,
+			true,
+			true,
+		},
+	}
+	for _, test := range tests {
+		test := test
+		runServersTest(t, nil, func(t *testing.T, server *Server) {
+			for _, bucketName := range []string{"some-bucket", "other-bucket", "empty-bucket"} {
+				server.CreateBucketWithOpts(CreateBucketOpts{
+					Name:              bucketName,
+					VersioningEnabled: test.versioningEnabled,
+				})
+			}
+			for _, obj := range getObjectsForListTests() {
+				obj.Generation = initialGeneration
+				server.CreateObject(obj)
+				if test.withOverwrites {
+					obj.Generation = finalGeneration
+					obj.Content = []byte("final content")
+					server.CreateObject(obj)
+				}
+			}
+			client := server.Client()
+			t.Run(test.testCase, func(t *testing.T) {
+				iter := client.Bucket(test.bucketName).Objects(context.TODO(), test.query)
+				names := []string{}
+				obj, err := iter.Next()
+				for ; err == nil; obj, err = iter.Next() {
+					names = append(names, obj.Name)
+					if !contains(test.expectedGenerations, obj.Generation) {
+						t.Errorf("unexpected generation\nwant in %v\ngot    %d", test.expectedGenerations, obj.Generation)
+					}
+				}
+				if err != iterator.Done {
+					t.Fatal(err)
+				}
+				if len(names) != test.expectedNumObjects {
+					t.Errorf("wrong number objects\nwant %d\ngot  %d", test.expectedNumObjects, len(names))
+				}
+
+				if !reflect.DeepEqual(unique(names), test.expectedNames) {
+					t.Errorf("wrong names returned\nwant %#v\ngot  %#v", test.expectedNames, names)
+				}
+			})
+		})
+	}
+}
+
 func TestServiceClientListObjectsBucketNotFound(t *testing.T) {
 	runServersTest(t, nil, func(t *testing.T, server *Server) {
 		iter := server.Client().Bucket("some-bucket").Objects(context.TODO(), nil)
diff --git a/internal/backend/backend_test.go b/internal/backend/backend_test.go
index 54b9748..432cfd2 100644
--- a/internal/backend/backend_test.go
+++ b/internal/backend/backend_test.go
@@ -132,7 +132,8 @@ func TestObjectCRUD(t *testing.T) {
 				}
 			}
 
-			objs, err := storage.ListObjects(bucketName)
+			t.Logf("checking active object is the expected one when versioning is %t", versioningEnabled)
+			objs, err := storage.ListObjects(bucketName, false)
 			noError(t, err)
 			if len(objs) != 1 {
 				t.Errorf("wrong number of objects returned\nwant 1\ngot  %d", len(objs))
@@ -141,6 +142,16 @@ func TestObjectCRUD(t *testing.T) {
 				t.Errorf("wrong object name\nwant %q\ngot  %q", objectName, objs[0].Name)
 			}
 
+			t.Logf("checking all object listing is the expected one when versioning is %t", versioningEnabled)
+			objs, err = storage.ListObjects(bucketName, true)
+			noError(t, err)
+			if versioningEnabled && len(objs) != 2 {
+				t.Errorf("wrong number of objects returned\nwant 2\ngot  %d", len(objs))
+			}
+			if !versioningEnabled && len(objs) != 1 {
+				t.Errorf("wrong number of objects returned\nwant 1\ngot  %d", len(objs))
+			}
+
 			err = storage.DeleteObject(bucketName, objectName)
 			noError(t, err)