Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for table deletion protection #2430

Merged
merged 7 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ public CreateTableRequest addChangeStreamRetention(Duration retention) {
return this;
}

/** Configures if the table is deletion protected. */
public CreateTableRequest setDeletionProtection(boolean deletionProtection) {
requestBuilder.getTableBuilder().setDeletionProtection(deletionProtection);
return this;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public com.google.bigtable.admin.v2.Table.ClusterState.ReplicationState toProto(
private final List<ColumnFamily> columnFamilies;

private final Duration changeStreamRetention;
private final boolean deletionProtection;

@InternalApi
public static Table fromProto(@Nonnull com.google.bigtable.admin.v2.Table proto) {
Expand Down Expand Up @@ -135,19 +136,22 @@ public static Table fromProto(@Nonnull com.google.bigtable.admin.v2.Table proto)
TableName.parse(proto.getName()),
replicationStates.build(),
columnFamilies.build(),
changeStreamConfig);
changeStreamConfig,
proto.getDeletionProtection());
}

private Table(
TableName tableName,
Map<String, ReplicationState> replicationStatesByClusterId,
List<ColumnFamily> columnFamilies,
Duration changeStreamRetention) {
Duration changeStreamRetention,
boolean deletionProtection) {
this.instanceId = tableName.getInstance();
this.id = tableName.getTable();
this.replicationStatesByClusterId = replicationStatesByClusterId;
this.columnFamilies = columnFamilies;
this.changeStreamRetention = changeStreamRetention;
this.deletionProtection = deletionProtection;
}

/** Gets the table's id. */
Expand All @@ -172,6 +176,10 @@ public Duration getChangeStreamRetention() {
return changeStreamRetention;
}

public boolean isProtected() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment: /** Returns whether this table is deletion protected. */
and rename it to isDeletionProtected to be consistent with authorized view

return deletionProtection;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -185,12 +193,18 @@ public boolean equals(Object o) {
&& Objects.equal(instanceId, table.instanceId)
&& Objects.equal(replicationStatesByClusterId, table.replicationStatesByClusterId)
&& Objects.equal(columnFamilies, table.columnFamilies)
&& Objects.equal(changeStreamRetention, table.changeStreamRetention);
&& Objects.equal(changeStreamRetention, table.changeStreamRetention)
&& Objects.equal(deletionProtection, table.deletionProtection);
}

@Override
public int hashCode() {
return Objects.hashCode(
id, instanceId, replicationStatesByClusterId, columnFamilies, changeStreamRetention);
id,
instanceId,
replicationStatesByClusterId,
columnFamilies,
changeStreamRetention,
deletionProtection);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ public UpdateTableRequest disableChangeStreamRetention() {
return addChangeStreamRetention(Duration.ZERO);
}

/** Changes the deletion protection of an existing table. */
public UpdateTableRequest setDeletionProtection(boolean deletionProtection) {
requestBuilder.getTableBuilder().setDeletionProtection(deletionProtection);
requestBuilder.getUpdateMaskBuilder().addPaths("deletion_protection");
return this;
}

@InternalApi
public com.google.bigtable.admin.v2.UpdateTableRequest toProto(
String projectId, String instanceId) {
Expand All @@ -88,11 +95,11 @@ public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof UpdateTableRequest)) return false;
UpdateTableRequest that = (UpdateTableRequest) o;
return Objects.equals(requestBuilder, that.requestBuilder);
return Objects.equals(requestBuilder.build(), that.requestBuilder.build());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why changing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove it but that's what we do in other like CreateTableRequest

}

@Override
public int hashCode() {
return Objects.hash(requestBuilder);
return Objects.hash(requestBuilder.build());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, why changing this?

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,43 @@ public void testCreateTable() {
assertThat(result).isEqualTo(Table.fromProto(expectedResponse));
}

@Test
public void testCreateTableWithDeletionProtectionSet() {
// Setup
Mockito.when(mockStub.createTableCallable()).thenReturn(mockCreateTableCallable);

com.google.bigtable.admin.v2.CreateTableRequest expectedRequest =
com.google.bigtable.admin.v2.CreateTableRequest.newBuilder()
.setParent(INSTANCE_NAME)
.setTableId(TABLE_ID)
.setTable(
com.google.bigtable.admin.v2.Table.newBuilder()
.setDeletionProtection(true)
.putColumnFamilies(
"cf1",
ColumnFamily.newBuilder()
.setGcRule(GcRule.getDefaultInstance())
.setValueType(TypeProtos.intSumType())
.build()))
.build();

com.google.bigtable.admin.v2.Table expectedResponse =
com.google.bigtable.admin.v2.Table.newBuilder().setName(TABLE_NAME).build();

Mockito.when(mockCreateTableCallable.futureCall(expectedRequest))
.thenReturn(ApiFutures.immediateFuture(expectedResponse));

// Execute
Table result =
adminClient.createTable(
CreateTableRequest.of(TABLE_ID)
.addFamily("cf1", Type.int64Sum())
.setDeletionProtection(true));

// Verify
assertThat(result).isEqualTo(Table.fromProto(expectedResponse));
}

@Test
public void testUpdateTable() {
// Setup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public void testToProto() {
.addFamily("another-family", GCRULES.maxAge(100, TimeUnit.HOURS))
.addSplit(splitKey)
.addSplit(secondSplitKey)
.addChangeStreamRetention(Duration.ofHours(24));
.addChangeStreamRetention(Duration.ofHours(24))
.setDeletionProtection(true);

com.google.bigtable.admin.v2.CreateTableRequest requestProto =
com.google.bigtable.admin.v2.CreateTableRequest.newBuilder()
Expand All @@ -70,7 +71,8 @@ public void testToProto() {
ChangeStreamConfig.newBuilder()
.setRetentionPeriod(
com.google.protobuf.Duration.newBuilder().setSeconds(86400))
.build()))
.build())
.setDeletionProtection(true))
.setParent(NameUtil.formatInstanceName(PROJECT_ID, INSTANCE_ID))
.addInitialSplits(
com.google.bigtable.admin.v2.CreateTableRequest.Split.newBuilder().setKey(splitKey))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public void testFromProto() {
.setSeconds(1)
.setNanos(99)))
.build())
.setDeletionProtection(true)
.build();

Table result = Table.fromProto(proto);
Expand All @@ -78,6 +79,7 @@ public void testFromProto() {
"cluster1", Table.ReplicationState.READY,
"cluster2", Table.ReplicationState.INITIALIZING);
assertThat(result.getColumnFamilies()).hasSize(3);
assertThat(result.isProtected()).isTrue();

for (Entry<String, ColumnFamily> entry : proto.getColumnFamiliesMap().entrySet()) {
assertThat(result.getColumnFamilies())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,46 @@ public void testNoChangeChangeStreamToProto() {
.build();
assertThat(request.toProto(PROJECT_ID, INSTANCE_ID)).isEqualTo(requestProto);
}

@Test
public void testEnableDeletionProtection() {
UpdateTableRequest request = UpdateTableRequest.of(TABLE_ID).setDeletionProtection(true);

com.google.bigtable.admin.v2.UpdateTableRequest requestProto =
com.google.bigtable.admin.v2.UpdateTableRequest.newBuilder()
.setTable(
Table.newBuilder()
.setName(NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID))
.setDeletionProtection(true))
.setUpdateMask(FieldMask.newBuilder().addPaths("deletion_protection").build())
.build();

assertThat(request.toProto(PROJECT_ID, INSTANCE_ID)).isEqualTo(requestProto);
}

@Test
public void testDisableDeletionProtection() {
UpdateTableRequest request = UpdateTableRequest.of(TABLE_ID).setDeletionProtection(false);

com.google.bigtable.admin.v2.UpdateTableRequest requestProto =
com.google.bigtable.admin.v2.UpdateTableRequest.newBuilder()
.setTable(
Table.newBuilder()
.setName(NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID))
.setDeletionProtection(false))
.setUpdateMask(FieldMask.newBuilder().addPaths("deletion_protection").build())
.build();

assertThat(request.toProto(PROJECT_ID, INSTANCE_ID)).isEqualTo(requestProto);
}

@Test
public void testHashCodeWithDeleteProtection() {
UpdateTableRequest request = UpdateTableRequest.of(TABLE_ID).setDeletionProtection(true);

assertThat(request.hashCode())
.isEqualTo(UpdateTableRequest.of(TABLE_ID).setDeletionProtection(true).hashCode());
assertThat(request.hashCode())
.isNotEqualTo(UpdateTableRequest.of(TABLE_ID).setDeletionProtection(false).hashCode());
}
}
Loading