Make grpc ListMedia limit optional

This commit is contained in:
Ravi Khadiwala
2026-08-05 13:20:09 -05:00
committed by ravi-signal
parent cbc89f541c
commit e4b09112f1
7 changed files with 33 additions and 11 deletions
@@ -59,6 +59,8 @@ public class BackupManager {
static final String MESSAGE_BACKUP_NAME = "messageBackup";
private static final int DEFAULT_LIST_MEDIA_LIMIT = 1000;
private static final String ZK_AUTHN_COUNTER_NAME = MetricsUtil.name(BackupManager.class, "authentication");
private static final String ZK_AUTHZ_FAILURE_COUNTER_NAME = MetricsUtil.name(BackupManager.class,
"authorizationFailure");
@@ -458,17 +460,19 @@ public class BackupManager {
*
* @param backupUser An already ZK authenticated backup user
* @param cursor A cursor returned by a previous call that can be used to resume listing
* @param limit The maximum number of list results to return
* @param limit The maximum number of list results to return, or empty to use
* {@link #DEFAULT_LIST_MEDIA_LIMIT}
* @return A {@link ListMediaResult}
* @throws BackupPermissionException if the credential does not have the correct level
*/
public ListMediaResult list(
final AuthenticatedBackupUser backupUser,
final Optional<String> cursor,
final int limit) throws BackupPermissionException {
final Optional<Integer> limit) throws BackupPermissionException {
checkBackupLevel(backupUser, BackupLevel.FREE);
final RemoteStorageManager.ListResult result =
remoteStorageManager.list(cdnMediaDirectory(backupUser), cursor, limit).toCompletableFuture().join();
final RemoteStorageManager.ListResult result = remoteStorageManager
.list(cdnMediaDirectory(backupUser), cursor, limit.orElse(DEFAULT_LIST_MEDIA_LIMIT))
.toCompletableFuture().join();
return new ListMediaResult(result
.objects()
.stream()
@@ -975,7 +975,7 @@ public class ArchiveController {
final AuthenticatedBackupUser backupUser =
backupManager.authenticateBackupUser(presentation.presentation, signature.signature, userAgent);
final BackupManager.ListMediaResult listResult =
backupManager.list(backupUser, cursor, limit.orElse(1000));
backupManager.list(backupUser, cursor, limit);
return new ListResponse(listResult.media()
.stream().map(entry -> new StoredMediaObject(entry.cdn(), entry.key(), entry.length()))
.toList(),
@@ -265,7 +265,7 @@ public class BackupsAnonymousGrpcService extends SimpleBackupsAnonymousGrpc.Back
final BackupManager.ListMediaResult listResult = backupManager.list(
backupUser,
request.hasCursor() ? Optional.of(request.getCursor()) : Optional.empty(),
request.getLimit());
request.hasLimit() ? Optional.of(request.getLimit()) : Optional.empty());
final ListMediaResponse.ListResult.Builder builder = ListMediaResponse.ListResult.newBuilder();
for (BackupManager.StorageDescriptorWithLength sd : listResult.media()) {
builder.addPage(ListMediaResponse.ListEntry.newBuilder()
@@ -473,9 +473,11 @@ message ListMediaRequest {
// A cursor returned by a previous call to ListMedia, absent on the first call
optional string cursor = 2;
// If provided, the maximum number of entries to return in a page
uint32 limit = 3 [(require.range) = {min: 1, max: 10000}];
// If provided, the maximum number of entries to return in a page. If absent,
// a server-chosen default is used.
optional uint32 limit = 3 [(require.range) = {min: 1, max: 10000}];
}
message ListMediaResponse {
message ListEntry {
// The backup cdn where this media object is stored
@@ -667,7 +667,7 @@ public class BackupManagerTest {
Optional.of("newCursor")
)));
final BackupManager.ListMediaResult result = backupManager.list(backupUser, cursor, 17);
final BackupManager.ListMediaResult result = backupManager.list(backupUser, cursor, Optional.of(17));
assertThat(result.media()).hasSize(1);
assertThat(result.media().getFirst().cdn()).isEqualTo(13);
assertThat(result.media().getFirst().key()).isEqualTo(
@@ -562,7 +562,7 @@ public class ArchiveControllerTest {
final Optional<String> expectedCursor = cursorProvided ? Optional.of("myCursor") : Optional.empty();
final Optional<String> returnedCursor = cursorReturned ? Optional.of("newCursor") : Optional.empty();
when(backupManager.list(any(), eq(expectedCursor), eq(17)))
when(backupManager.list(any(), eq(expectedCursor), eq(Optional.of(17))))
.thenReturn(new BackupManager.ListMediaResult(
List.of(new BackupManager.StorageDescriptorWithLength(1, mediaId, 100)),
returnedCursor
@@ -11,6 +11,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.google.protobuf.ByteString;
@@ -290,7 +291,7 @@ class BackupsAnonymousGrpcServiceTest extends
final int limit = 17;
when(backupManager.list(any(), eq(expectedCursor), eq(limit)))
when(backupManager.list(any(), eq(expectedCursor), eq(Optional.of(limit))))
.thenReturn(new BackupManager.ListMediaResult(
List.of(new BackupManager.StorageDescriptorWithLength(1, mediaId, 100)),
returnedCursor));
@@ -463,6 +464,21 @@ class BackupsAnonymousGrpcServiceTest extends
.build()));
}
@Test
void listMediaUnsetLimit() throws BackupException {
when(backupManager.list(any(), any(), eq(Optional.empty())))
.thenReturn(new BackupManager.ListMediaResult(List.of(), Optional.empty()));
// should be able to list without setting a limit
final ListMediaResponse response = unauthenticatedServiceStub().listMedia(
ListMediaRequest.newBuilder()
.setSignedPresentation(signedPresentation(presentation))
.build());
assertThat(response.hasListResult()).isTrue();
verify(backupManager).list(any(), any(), eq(Optional.empty()));
}
private static AuthenticatedBackupUser backupUser(final byte[] backupId, final BackupCredentialType credentialType,
final BackupLevel backupLevel) {