Skip to content

Commit

Permalink
[apache#5593] improvement(iceberg): Add final to Iceberg table event …
Browse files Browse the repository at this point in the history
…members, fix return codes for rename and exists operations (apache#5594)

### What changes were proposed in this pull request?

1. Add final to Iceberg table-related events and their members.

2. Modify the return codes in `IcebergTableOperations` and
`IcebergTableRenameOperations` to align with the specification, and
update the relevant unit tests.

### Why are the changes needed?

Close: apache#5593

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Unit test
  • Loading branch information
orenccl authored Nov 17, 2024
1 parent 977758d commit 1821345
Show file tree
Hide file tree
Showing 17 changed files with 20 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ public Response tableExists(
TableIdentifier tableIdentifier = TableIdentifier.of(icebergNS, table);
boolean exists = tableOperationDispatcher.tableExists(context, tableIdentifier);
if (exists) {
return IcebergRestUtils.okWithoutContent();
return IcebergRestUtils.noContent();
} else {
return IcebergRestUtils.notExists();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public Response renameTable(
renameTableRequest.destination());
IcebergRequestContext context = new IcebergRequestContext(httpServletRequest(), catalogName);
tableOperationDispatcher.renameTable(context, renameTableRequest);
return IcebergRestUtils.okWithoutContent();
return IcebergRestUtils.noContent();
}

// HTTP request is null in Jersey test, override with a mock request when testing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
@DeveloperApi
public class IcebergCreateTableEvent extends IcebergTableEvent {

private CreateTableRequest createTableRequest;
private LoadTableResponse loadTableResponse;
private final CreateTableRequest createTableRequest;
private final LoadTableResponse loadTableResponse;

public IcebergCreateTableEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/** Represent a failure event when creating Iceberg table failed. */
@DeveloperApi
public class IcebergCreateTableFailureEvent extends IcebergTableFailureEvent {
private CreateTableRequest createTableRequest;
private final CreateTableRequest createTableRequest;

public IcebergCreateTableFailureEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
/** Represent a pre event before creating Iceberg table. */
@DeveloperApi
public class IcebergCreateTablePreEvent extends IcebergTablePreEvent {
private CreateTableRequest createTableRequest;
private final CreateTableRequest createTableRequest;

public IcebergCreateTablePreEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
/** Represent an event after dropping Iceberg table successfully. */
@DeveloperApi
public class IcebergDropTableEvent extends IcebergTableEvent {
private boolean purgeRequested;
private final boolean purgeRequested;

public IcebergDropTableEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
/** Represent a failure event when dropping Iceberg table failed. */
@DeveloperApi
public class IcebergDropTableFailureEvent extends IcebergTableFailureEvent {
private boolean purgeRequested;
private final boolean purgeRequested;

public IcebergDropTableFailureEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
/** Represent a pre event before dropping Iceberg table. */
@DeveloperApi
public class IcebergDropTablePreEvent extends IcebergTablePreEvent {
private boolean purgeRequested;
private final boolean purgeRequested;

public IcebergDropTablePreEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/** Represent an event after loading Iceberg table successfully. */
@DeveloperApi
public class IcebergLoadTableEvent extends IcebergTableEvent {
private LoadTableResponse loadTableResponse;
private final LoadTableResponse loadTableResponse;

public IcebergLoadTableEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/** Represent an event after rename Iceberg table successfully. */
@DeveloperApi
public class IcebergRenameTableEvent extends IcebergTableEvent {
private RenameTableRequest renameTableRequest;
private final RenameTableRequest renameTableRequest;

public IcebergRenameTableEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
/** Represent an event when rename Iceberg table failed. */
@DeveloperApi
public class IcebergRenameTableFailureEvent extends IcebergTableFailureEvent {
private RenameTableRequest renameTableRequest;
private final RenameTableRequest renameTableRequest;

public IcebergRenameTableFailureEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
/** Represent an pre event before rename an Iceberg table. */
@DeveloperApi
public class IcebergRenameTablePreEvent extends IcebergTablePreEvent {
private RenameTableRequest renameTableRequest;
private final RenameTableRequest renameTableRequest;

public IcebergRenameTablePreEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
/** Represent an event after check Iceberg table exists successfully. */
@DeveloperApi
public class IcebergTableExistsEvent extends IcebergTableEvent {
private boolean isExists;
private final boolean isExists;

public IcebergTableExistsEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
@DeveloperApi
public class IcebergUpdateTableEvent extends IcebergTableEvent {

private UpdateTableRequest updateTableRequest;
private LoadTableResponse loadTableResponse;
private final UpdateTableRequest updateTableRequest;
private final LoadTableResponse loadTableResponse;

public IcebergUpdateTableEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/** Represent a failure event when updating Iceberg table failed. */
@DeveloperApi
public class IcebergUpdateTableFailureEvent extends IcebergTableFailureEvent {
private UpdateTableRequest updateTableRequest;
private final UpdateTableRequest updateTableRequest;

public IcebergUpdateTableFailureEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
/** Represent a pre event before updating Iceberg table. */
@DeveloperApi
public class IcebergUpdateTablePreEvent extends IcebergTablePreEvent {
private UpdateTableRequest updateTableRequest;
private final UpdateTableRequest updateTableRequest;

public IcebergUpdateTablePreEvent(
IcebergRequestContext icebergRequestContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ void testTableExits() {

verifyCreateTableSucc("exists_foo1");
dummyEventListener.clearEvent();
verifyTableExistsStatusCode("exists_foo1", 200);
verifyTableExistsStatusCode("exists_foo1", 204);
Assertions.assertTrue(dummyEventListener.popPreEvent() instanceof IcebergTableExistsPreEvent);
postEvent = dummyEventListener.popPostEvent();
Assertions.assertTrue(postEvent instanceof IcebergTableExistsEvent);
Expand Down Expand Up @@ -430,7 +430,7 @@ private void verifyRenameTableSucc(String source, String dest) {
Response response = doRenameTable(source, dest);
System.out.println(response);
System.out.flush();
Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
Assertions.assertEquals(Status.NO_CONTENT.getStatusCode(), response.getStatus());
}

private void verifyRenameTableFail(String source, String dest, int status) {
Expand Down

0 comments on commit 1821345

Please sign in to comment.