Skip to content

Commit

Permalink
fix: addresses PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
thiagotnunes committed Mar 17, 2021
1 parent 3ff8a89 commit acecbfe
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 77 deletions.
Expand Up @@ -62,10 +62,6 @@ public Backup build() {

/** Creates a backup on the server based on the source of this {@link Backup} instance. */
public OperationFuture<Backup, CreateBackupMetadata> create() {
Preconditions.checkState(
getExpireTime() != null, "Cannot create a backup without an expire time");
Preconditions.checkState(
getDatabase() != null, "Cannot create a backup without a source database");
return dbClient.createBackup(this);
}

Expand Down
Expand Up @@ -219,8 +219,7 @@ public long getSize() {

/**
* Returns the {@link BackupEncryptionConfig} to encrypt the backup during its creation. Returns
* <code>
* null</code> if no customer-managed encryption key should be used.
* <code>null</code> if no customer-managed encryption key should be used.
*/
public BackupEncryptionConfig getEncryptionConfig() {
return encryptionConfig;
Expand Down
Expand Up @@ -143,6 +143,11 @@ public OperationFuture<Backup, CreateBackupMetadata> createBackup(
@Override
public OperationFuture<Backup, CreateBackupMetadata> createBackup(Backup backupInfo)
throws SpannerException {
Preconditions.checkArgument(
backupInfo.getExpireTime() != null, "Cannot create a backup without an expire time");
Preconditions.checkArgument(
backupInfo.getDatabase() != null, "Cannot create a backup without a source database");

final OperationFuture<com.google.spanner.admin.database.v1.Backup, CreateBackupMetadata>
rawOperationFuture = rpc.createBackup(backupInfo);

Expand Down
Expand Up @@ -20,4 +20,4 @@

/** Marker interface for encryption configurations that can be applied on backups. */
@InternalApi
public interface BackupEncryptionConfig extends EncryptionConfig {}
public interface BackupEncryptionConfig {}

This file was deleted.

Expand Up @@ -113,37 +113,6 @@ public void create() {
verify(dbClient).createBackup(backup);
}

@Test
public void createWithoutSource() {
Timestamp expireTime = Timestamp.now();
Backup backup =
dbClient
.newBackupBuilder(BackupId.of("test-project", "dest-instance", "backup-id"))
.setExpireTime(expireTime)
.build();
try {
backup.create();
fail("Expected exception");
} catch (IllegalStateException e) {
assertNotNull(e.getMessage());
}
}

@Test
public void createWithoutExpireTime() {
Backup backup =
dbClient
.newBackupBuilder(BackupId.of("test-project", "instance-id", "backup-id"))
.setDatabase(DatabaseId.of("test-project", "instance-id", "src-database"))
.build();
try {
backup.create();
fail("Expected exception");
} catch (IllegalStateException e) {
assertNotNull(e.getMessage());
}
}

@Test
public void createWithoutVersionTimeShouldSucceed() {
final Timestamp expireTime = Timestamp.now();
Expand Down
Expand Up @@ -17,6 +17,7 @@
package com.google.cloud.spanner;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.MockitoAnnotations.initMocks;
Expand Down Expand Up @@ -261,7 +262,7 @@ public void listDatabasesError() {
SpannerExceptionFactory.newSpannerException(ErrorCode.INVALID_ARGUMENT, "Test error"));
try {
client.listDatabases(INSTANCE_ID, Options.pageSize(1));
Assert.fail("Missing expected exception");
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getMessage()).contains(INSTANCE_NAME);
// Assert that the call was done without a page token.
Expand All @@ -279,7 +280,7 @@ public void listDatabaseErrorWithToken() {
SpannerExceptionFactory.newSpannerException(ErrorCode.INVALID_ARGUMENT, "Test error"));
try {
Lists.newArrayList(client.listDatabases(INSTANCE_ID, Options.pageSize(1)).iterateAll());
Assert.fail("Missing expected exception");
fail("Missing expected exception");
} catch (SpannerException e) {
assertThat(e.getMessage()).contains(INSTANCE_NAME);
// Assert that the call was done without a page token.
Expand Down Expand Up @@ -388,12 +389,6 @@ public void createBackupWithBackupObject() throws ExecutionException, Interrupte
final Timestamp versionTime =
Timestamp.ofTimeMicroseconds(
TimeUnit.MILLISECONDS.toMicros(System.currentTimeMillis()) - TimeUnit.DAYS.toMicros(2));
final Backup expectedCallBackup =
Backup.newBuilder()
.setDatabase(DB_NAME)
.setExpireTime(expireTime.toProto())
.setVersionTime(versionTime.toProto())
.build();
final com.google.cloud.spanner.Backup requestBackup =
client
.newBackupBuilder(BackupId.of(PROJECT_ID, INSTANCE_ID, BK_ID))
Expand All @@ -410,6 +405,28 @@ public void createBackupWithBackupObject() throws ExecutionException, Interrupte
assertThat(op.get().getId().getName()).isEqualTo(BK_NAME);
}

@Test(expected = IllegalArgumentException.class)
public void testCreateBackupNoExpireTime() {
final com.google.cloud.spanner.Backup requestBackup =
client
.newBackupBuilder(BackupId.of(PROJECT_ID, INSTANCE_ID, BK_ID))
.setDatabase(DatabaseId.of(PROJECT_ID, INSTANCE_ID, DB_ID))
.build();

client.createBackup(requestBackup);
}

@Test(expected = IllegalArgumentException.class)
public void testCreateBackupNoDatabase() {
final com.google.cloud.spanner.Backup requestBackup =
client
.newBackupBuilder(BackupId.of(PROJECT_ID, INSTANCE_ID, BK_ID))
.setExpireTime(Timestamp.now())
.build();

client.createBackup(requestBackup);
}

@Test
public void createEncryptedBackup() throws ExecutionException, InterruptedException {
final OperationFuture<Backup, CreateBackupMetadata> rawOperationFuture =
Expand Down
Expand Up @@ -16,7 +16,6 @@
package com.google.cloud.spanner.encryption;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

import com.google.spanner.admin.database.v1.CreateBackupEncryptionConfig;
import com.google.spanner.admin.database.v1.EncryptionConfig;
Expand Down Expand Up @@ -85,7 +84,6 @@ public void testCreateBackupConfigUseDatabaseEncryption() {
@Test(expected = IllegalArgumentException.class)
public void testCreateBackupInvalidEncryption() {
EncryptionConfigProtoMapper.createBackupEncryptionConfig(null);
fail("Create backup encryption config with invalid encryption should fail");
}

@Test
Expand Down Expand Up @@ -137,6 +135,5 @@ public void testRestoreDatabaseConfigUseBackupEncryption() {
@Test(expected = IllegalArgumentException.class)
public void testRestoreDatabaseConfigInvalidEncryption() {
EncryptionConfigProtoMapper.restoreDatabaseEncryptionConfig(null);
fail("Restore database encryption config with invalid encryption should fail");
}
}
Expand Up @@ -17,7 +17,6 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.fail;

import org.junit.Test;

Expand All @@ -37,7 +36,6 @@ public void testCustomerManagedEncryption() {
@Test(expected = IllegalArgumentException.class)
public void testCustomerManagedEncryptionNullKeyName() {
EncryptionConfigs.customerManagedEncryption(null);
fail("Customer managed encryption with null key should fail");
}

@Test
Expand Down

0 comments on commit acecbfe

Please sign in to comment.