gcs: Use Pager to iterate results in ListObjectsV1/V2 (#6162)

Fixes #6052

cmd/gateway/gcs/gateway-gcs.go

@@ -554,16 +554,16 @@ func isGCSMarker(marker string) bool {
 
 // ListObjects - lists all blobs in GCS bucket filtered by prefix
 func (l *gcsGateway) ListObjects(ctx context.Context, bucket string, prefix string, marker string, delimiter string, maxKeys int) (minio.ListObjectsInfo, error) {
+	if maxKeys == 0 {
+		return minio.ListObjectsInfo{}, nil
+	}
+
 	it := l.client.Bucket(bucket).Objects(l.ctx, &storage.Query{
 		Delimiter: delimiter,
 		Prefix:    prefix,
 		Versions:  false,
 	})
 
-	isTruncated := false
-	nextMarker := ""
-	prefixes := []string{}
-
 	// To accommodate S3-compatible applications using
 	// ListObjectsV1 to use object keys as markers to control the
 	// listing of objects, we use the following encoding scheme to
@@ -574,83 +574,86 @@ func (l *gcsGateway) ListObjects(ctx context.Context, bucket string, prefix stri
 	//   prefixing "{minio}" to the GCS continuation token,
 	//   e.g, "{minio}CgRvYmoz"
 	//
-	// - Application supplied markers are used as-is to list
+	// - Application supplied markers are transformed to a
-	//   object keys that appear after it in the lexicographical order.
+	//   GCS continuation token.
 
 	// If application is using GCS continuation token we should
 	// strip the gcsTokenPrefix we added.
-	gcsMarker := isGCSMarker(marker)
+	token := ""
-	if gcsMarker {
+	if marker != "" {
-		it.PageInfo().Token = strings.TrimPrefix(marker, gcsTokenPrefix)
+		if isGCSMarker(marker) {
+			token = strings.TrimPrefix(marker, gcsTokenPrefix)
+		} else {
+			token = toGCSPageToken(marker)
+		}
 	}
+	nextMarker := ""
 
-	it.PageInfo().MaxSize = maxKeys
+	var prefixes []string
+	var objects []minio.ObjectInfo
+	var nextPageToken string
+	var err error
 
-	objects := []minio.ObjectInfo{}
+	pager := iterator.NewPager(it, maxKeys, token)
 	for {
-		if len(objects) >= maxKeys {
+		gcsObjects := make([]*storage.ObjectAttrs, 0)
-			// check if there is one next object and
+		nextPageToken, err = pager.NextPage(&gcsObjects)
-			// if that one next object is our hidden
-			// metadata folder, then just break
-			// otherwise we've truncated the output
-			attrs, _ := it.Next()
-			if attrs != nil && attrs.Prefix == minio.GatewayMinioSysTmp {
-				break
-			}
-
-			isTruncated = true
-			break
-		}
-
-		attrs, err := it.Next()
-		if err == iterator.Done {
-			break
-		}
 		if err != nil {
 			logger.LogIf(ctx, err)
 			return minio.ListObjectsInfo{}, gcsToObjectError(err, bucket, prefix)
 		}
 
-		nextMarker = toGCSPageToken(attrs.Name)
+		for _, attrs := range gcsObjects {
 
-		if attrs.Prefix == minio.GatewayMinioSysTmp {
+			// Due to minio.GatewayMinioSysTmp keys being skipped, the number of objects + prefixes
-			// We don't return our metadata prefix.
+			// returned may not total maxKeys. This behavior is compatible with the S3 spec which
-			continue
+			// allows the response to include less keys than maxKeys.
-		}
+			if attrs.Prefix == minio.GatewayMinioSysTmp {
-		if !strings.HasPrefix(prefix, minio.GatewayMinioSysTmp) {
+				// We don't return our metadata prefix.
-			// If client lists outside gcsMinioPath then we filter out gcsMinioPath/* entries.
-			// But if the client lists inside gcsMinioPath then we return the entries in gcsMinioPath/
-			// which will be helpful to observe the "directory structure" for debugging purposes.
-			if strings.HasPrefix(attrs.Prefix, minio.GatewayMinioSysTmp) ||
-				strings.HasPrefix(attrs.Name, minio.GatewayMinioSysTmp) {
 				continue
 			}
-		}
+			if !strings.HasPrefix(prefix, minio.GatewayMinioSysTmp) {
-		if attrs.Prefix != "" {
+				// If client lists outside gcsMinioPath then we filter out gcsMinioPath/* entries.
-			prefixes = append(prefixes, attrs.Prefix)
+				// But if the client lists inside gcsMinioPath then we return the entries in gcsMinioPath/
-			continue
+				// which will be helpful to observe the "directory structure" for debugging purposes.
-		}
+				if strings.HasPrefix(attrs.Prefix, minio.GatewayMinioSysTmp) ||
-		if !gcsMarker && attrs.Name <= marker {
+					strings.HasPrefix(attrs.Name, minio.GatewayMinioSysTmp) {
-			// if user supplied a marker don't append
+					continue
-			// objects until we reach marker (and skip it).
+				}
-			continue
+			}
+
+			if attrs.Prefix != "" {
+				prefixes = append(prefixes, attrs.Prefix)
+			} else {
+				objects = append(objects, fromGCSAttrsToObjectInfo(attrs))
+			}
+
+			// The NextMarker property should only be set in the response if a delimiter is used
+			if delimiter != "" {
+				if attrs.Prefix > nextMarker {
+					nextMarker = attrs.Prefix
+				} else if attrs.Name > nextMarker {
+					nextMarker = attrs.Name
+				}
+			}
 		}
 
-		objects = append(objects, minio.ObjectInfo{
+		// Exit the loop if at least one item can be returned from
-			Name:            attrs.Name,
+		// the current page or there are no more pages available
-			Bucket:          attrs.Bucket,
+		if nextPageToken == "" || len(prefixes)+len(objects) > 0 {
-			ModTime:         attrs.Updated,
+			break
-			Size:            attrs.Size,
+		}
-			ETag:            minio.ToS3ETag(fmt.Sprintf("%d", attrs.CRC32C)),
+	}
-			UserDefined:     attrs.Metadata,
+
-			ContentType:     attrs.ContentType,
+	if nextPageToken == "" {
-			ContentEncoding: attrs.ContentEncoding,
+		nextMarker = ""
-		})
+	} else if nextMarker != "" {
+		nextMarker = gcsTokenPrefix + toGCSPageToken(nextMarker)
 	}
 
 	return minio.ListObjectsInfo{
-		IsTruncated: isTruncated,
+		IsTruncated: nextPageToken != "",
-		NextMarker:  gcsTokenPrefix + nextMarker,
+		NextMarker:  nextMarker,
 		Prefixes:    prefixes,
 		Objects:     objects,
 	}, nil
@@ -658,8 +661,8 @@ func (l *gcsGateway) ListObjects(ctx context.Context, bucket string, prefix stri
 
 // ListObjectsV2 - lists all blobs in GCS bucket filtered by prefix
 func (l *gcsGateway) ListObjectsV2(ctx context.Context, bucket, prefix, continuationToken, delimiter string, maxKeys int, fetchOwner bool, startAfter string) (minio.ListObjectsV2Info, error) {
-	if continuationToken == "" && startAfter != "" {
+	if maxKeys == 0 {
-		continuationToken = toGCSPageToken(startAfter)
+		return minio.ListObjectsV2Info{ContinuationToken: continuationToken}, nil
 	}
 
 	it := l.client.Bucket(bucket).Objects(l.ctx, &storage.Query{
@@ -668,61 +671,62 @@ func (l *gcsGateway) ListObjectsV2(ctx context.Context, bucket, prefix, continua
 		Versions:  false,
 	})
 
-	isTruncated := false
+	token := continuationToken
-
+	if token == "" && startAfter != "" {
-	if continuationToken != "" {
+		token = toGCSPageToken(startAfter)
-		// If client sends continuationToken, set it
-		it.PageInfo().Token = continuationToken
-	} else {
-		// else set the continuationToken to return
-		continuationToken = it.PageInfo().Token
-		if continuationToken != "" {
-			// If GCS SDK sets continuationToken, it means there are more than maxKeys in the current page
-			// and the response will be truncated
-			isTruncated = true
-		}
 	}
 
 	var prefixes []string
 	var objects []minio.ObjectInfo
+	var nextPageToken string
+	var err error
 
-	for keyCount := 0; keyCount < maxKeys; keyCount++ {
+	pager := iterator.NewPager(it, maxKeys, token)
-		attrs, err := it.Next()
+	for {
-		if err == iterator.Done {
+		gcsObjects := make([]*storage.ObjectAttrs, 0)
-			break
+		nextPageToken, err = pager.NextPage(&gcsObjects)
-		}
-
 		if err != nil {
 			logger.LogIf(ctx, err)
 			return minio.ListObjectsV2Info{}, gcsToObjectError(err, bucket, prefix)
 		}
 
-		if attrs.Prefix == minio.GatewayMinioSysTmp {
+		for _, attrs := range gcsObjects {
-			// We don't return our metadata prefix.
+
-			continue
+			// Due to minio.GatewayMinioSysTmp keys being skipped, the number of objects + prefixes
-		}
+			// returned may not total maxKeys. This behavior is compatible with the S3 spec which
-		if !strings.HasPrefix(prefix, minio.GatewayMinioSysTmp) {
+			// allows the response to include less keys than maxKeys.
-			// If client lists outside gcsMinioPath then we filter out gcsMinioPath/* entries.
+			if attrs.Prefix == minio.GatewayMinioSysTmp {
-			// But if the client lists inside gcsMinioPath then we return the entries in gcsMinioPath/
+				// We don't return our metadata prefix.
-			// which will be helpful to observe the "directory structure" for debugging purposes.
-			if strings.HasPrefix(attrs.Prefix, minio.GatewayMinioSysTmp) ||
-				strings.HasPrefix(attrs.Name, minio.GatewayMinioSysTmp) {
 				continue
 			}
+			if !strings.HasPrefix(prefix, minio.GatewayMinioSysTmp) {
+				// If client lists outside gcsMinioPath then we filter out gcsMinioPath/* entries.
+				// But if the client lists inside gcsMinioPath then we return the entries in gcsMinioPath/
+				// which will be helpful to observe the "directory structure" for debugging purposes.
+				if strings.HasPrefix(attrs.Prefix, minio.GatewayMinioSysTmp) ||
+					strings.HasPrefix(attrs.Name, minio.GatewayMinioSysTmp) {
+					continue
+				}
+			}
+
+			if attrs.Prefix != "" {
+				prefixes = append(prefixes, attrs.Prefix)
+			} else {
+				objects = append(objects, fromGCSAttrsToObjectInfo(attrs))
+			}
 		}
 
-		if attrs.Prefix != "" {
+		// Exit the loop if at least one item can be returned from
-			prefixes = append(prefixes, attrs.Prefix)
+		// the current page or there are no more pages available
-			continue
+		if nextPageToken == "" || len(prefixes)+len(objects) > 0 {
+			break
 		}
-
-		objects = append(objects, fromGCSAttrsToObjectInfo(attrs))
 	}
 
 	return minio.ListObjectsV2Info{
-		IsTruncated:           isTruncated,
+		IsTruncated:           nextPageToken != "",
 		ContinuationToken:     continuationToken,
-		NextContinuationToken: continuationToken,
+		NextContinuationToken: nextPageToken,
 		Prefixes:              prefixes,
 		Objects:               objects,
 	}, nil