feat(spanner): add getIsolationLevel and getReadLockMode methods to CommitResponse#13004
feat(spanner): add getIsolationLevel and getReadLockMode methods to CommitResponse#13004shobhitsg wants to merge 2 commits intogoogleapis:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces getIsolationLevel() and getReadLockMode() methods to the CommitResponse class to support internal testing, along with comprehensive unit tests. The review feedback suggests marking these new methods with the @InternalApi annotation to clarify that they are not part of the stable public API. Additionally, it is recommended to use local variables for proto values within these methods to avoid redundant calls and improve code clarity.
| import com.google.spanner.v1.TransactionOptions.IsolationLevel; | ||
| import com.google.spanner.v1.TransactionOptions.ReadWrite.ReadLockMode; |
There was a problem hiding this comment.
Add the imports for InternalApi and Nullable to support marking the new methods as internal and nullable, as they are intended for internal test environments.
| import com.google.spanner.v1.TransactionOptions.IsolationLevel; | |
| import com.google.spanner.v1.TransactionOptions.ReadWrite.ReadLockMode; | |
| import com.google.api.core.InternalApi; | |
| import com.google.spanner.v1.TransactionOptions.IsolationLevel; | |
| import com.google.spanner.v1.TransactionOptions.ReadWrite.ReadLockMode; | |
| import javax.annotation.Nullable; |
| /** | ||
| * Returns the {@link IsolationLevel} used for the read-write transaction if the transaction ran | ||
| * in internal test environments, and otherwise returns null. | ||
| */ | ||
| public @Nullable IsolationLevel getIsolationLevel() { | ||
| if (proto.getIsolationLevel() == IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED | ||
| || proto.getIsolationLevel() == IsolationLevel.UNRECOGNIZED) { | ||
| return null; | ||
| } | ||
| return proto.getIsolationLevel(); | ||
| } |
There was a problem hiding this comment.
Since this method is intended for internal test environments, it should be annotated with @internalapi to indicate it is not part of the stable public API. Additionally, using a local variable for the isolation level avoids multiple calls to the underlying proto getter.
/**
* Returns the {@link IsolationLevel} used for the read-write transaction if the transaction ran
* in internal test environments, and otherwise returns null.
*/
@InternalApi
public @Nullable IsolationLevel getIsolationLevel() {
IsolationLevel isolationLevel = proto.getIsolationLevel();
if (isolationLevel == IsolationLevel.ISOLATION_LEVEL_UNSPECIFIED
|| isolationLevel == IsolationLevel.UNRECOGNIZED) {
return null;
}
return isolationLevel;
}References
- Update stale Javadoc comments to reflect new method behavior for improved clarity, rather than removing them.
- Breaking changes to public APIs are permissible if the API has not yet been released.
| /** | ||
| * Returns the {@link ReadLockMode} used for the read-write transaction if the transaction ran in | ||
| * internal test environments, and otherwise returns null. | ||
| */ | ||
| public @Nullable ReadLockMode getReadLockMode() { | ||
| if (proto.getReadLockMode() == ReadLockMode.READ_LOCK_MODE_UNSPECIFIED | ||
| || proto.getReadLockMode() == ReadLockMode.UNRECOGNIZED) { | ||
| return null; | ||
| } | ||
| return proto.getReadLockMode(); | ||
| } |
There was a problem hiding this comment.
Similar to getIsolationLevel, this method should be annotated with @internalapi and can be optimized by using a local variable for the read lock mode.
/**
* Returns the {@link ReadLockMode} used for the read-write transaction if the transaction ran in
* internal test environments, and otherwise returns null.
*/
@InternalApi
public @Nullable ReadLockMode getReadLockMode() {
ReadLockMode readLockMode = proto.getReadLockMode();
if (readLockMode == ReadLockMode.READ_LOCK_MODE_UNSPECIFIED
|| readLockMode == ReadLockMode.UNRECOGNIZED) {
return null;
}
return readLockMode;
}References
- Update stale Javadoc comments to reflect new method behavior for improved clarity, rather than removing them.
- Breaking changes to public APIs are permissible if the API has not yet been released.
No description provided.