From 413de6d35f021b8ebdf17d64aade5760f183a166 Mon Sep 17 00:00:00 2001 From: Rahul Kesharawni Date: Thu, 3 Jan 2019 17:10:00 +0530 Subject: [PATCH 1/5] Adding Query.fromProto --- .../cloud/bigtable/data/v2/models/Query.java | 19 ++++++++ .../bigtable/data/v2/models/QueryTest.java | 44 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java index 0041fee227fb..89193e641ce7 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java @@ -260,6 +260,25 @@ public ReadRowsRequest toProto(RequestContext requestContext) { .build(); } + /** + * Wraps the protobuf {@link ReadRowsRequest}. This method is considered an internal implementation detail + * and not meant to be used by applications. + */ + @InternalApi + public static Query fromProto(ReadRowsRequest request) { + Preconditions.checkArgument(request != null, "ReadRowsRequest must not be null"); + + TableName tableName = TableName.parse(request.getTableName()); + String tableId = ""; + if (tableName != null) { + tableId = tableName.getTable(); + } + + Query query = new Query(tableId); + query.builder = ReadRowsRequest.newBuilder(request); + return query; + } + private static ByteString wrapKey(String key) { if (key == null) { return null; diff --git a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java index 85521bc2dbe9..8460fa66334e 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java +++ b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java @@ -227,4 +227,48 @@ private static ReadRowsRequest.Builder expectedProtoBuilder() { .setTableName(TABLE_NAME.toString()) .setAppProfileId(APP_PROFILE_ID); } + + @Test + public void testFromProto() { + ReadRowsRequest request = ReadRowsRequest.newBuilder() + .setTableName(TABLE_NAME.toString()) + .setAppProfileId(APP_PROFILE_ID) + .setFilter( + RowFilter.newBuilder() + .setRowKeyRegexFilter(ByteString.copyFromUtf8(".*"))) + .setRows(RowSet.newBuilder() + .addRowKeys(ByteString.copyFromUtf8("row-key")) + .addRowRanges( + RowRange.newBuilder() + .setStartKeyClosed(ByteString.copyFromUtf8("j")) + .setEndKeyClosed(ByteString.copyFromUtf8("z")))) + .build(); + Query query = Query.fromProto(request); + + assertThat(query.toProto(requestContext)) + .isEqualTo(request); + } + + @Test + public void testFromProtoWithEmptyTableId() { + TableName EMPTY_TABLE_NAME = TableName.of("", "", ""); + ReadRowsRequest request = ReadRowsRequest.newBuilder() + .setTableName(EMPTY_TABLE_NAME.toString()) + .setFilter( + RowFilter.newBuilder() + .setRowKeyRegexFilter( + ByteString.copyFromUtf8(".*"))) + .setRows( + RowSet.newBuilder() + .addRowKeys(ByteString.copyFromUtf8("row-key")) + .addRowRanges( + RowRange.newBuilder() + .setStartKeyClosed(ByteString.copyFromUtf8("j")) + .setEndKeyClosed(ByteString.copyFromUtf8("z")))) + .build(); + + RequestContext reqContext = RequestContext.create(InstanceName.of("", ""), ""); + assertThat(Query.fromProto(request).toProto(reqContext)) + .isEqualTo(request); + } } From f6c00215b01ea1e10a9a1c23242f94cbd2efdd08 Mon Sep 17 00:00:00 2001 From: Rahul Kesharawni Date: Thu, 3 Jan 2019 17:10:00 +0530 Subject: [PATCH 2/5] Adding Query.fromProto --- .../cloud/bigtable/data/v2/models/Query.java | 19 ++++++++ .../bigtable/data/v2/models/QueryTest.java | 44 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java index c01174bbe07d..f87cc1da79ec 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java @@ -258,6 +258,25 @@ public ReadRowsRequest toProto(RequestContext requestContext) { .build(); } + /** + * Wraps the protobuf {@link ReadRowsRequest}. This method is considered an internal implementation detail + * and not meant to be used by applications. + */ + @InternalApi + public static Query fromProto(ReadRowsRequest request) { + Preconditions.checkArgument(request != null, "ReadRowsRequest must not be null"); + + TableName tableName = TableName.parse(request.getTableName()); + String tableId = ""; + if (tableName != null) { + tableId = tableName.getTable(); + } + + Query query = new Query(tableId); + query.builder = ReadRowsRequest.newBuilder(request); + return query; + } + private static ByteString wrapKey(String key) { if (key == null) { return null; diff --git a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java index 780720fceea4..bf84fa045719 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java +++ b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java @@ -226,4 +226,48 @@ private static ReadRowsRequest.Builder expectedProtoBuilder() { .setTableName(NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID)) .setAppProfileId(APP_PROFILE_ID); } + + @Test + public void testFromProto() { + ReadRowsRequest request = ReadRowsRequest.newBuilder() + .setTableName(TABLE_NAME.toString()) + .setAppProfileId(APP_PROFILE_ID) + .setFilter( + RowFilter.newBuilder() + .setRowKeyRegexFilter(ByteString.copyFromUtf8(".*"))) + .setRows(RowSet.newBuilder() + .addRowKeys(ByteString.copyFromUtf8("row-key")) + .addRowRanges( + RowRange.newBuilder() + .setStartKeyClosed(ByteString.copyFromUtf8("j")) + .setEndKeyClosed(ByteString.copyFromUtf8("z")))) + .build(); + Query query = Query.fromProto(request); + + assertThat(query.toProto(requestContext)) + .isEqualTo(request); + } + + @Test + public void testFromProtoWithEmptyTableId() { + TableName EMPTY_TABLE_NAME = TableName.of("", "", ""); + ReadRowsRequest request = ReadRowsRequest.newBuilder() + .setTableName(EMPTY_TABLE_NAME.toString()) + .setFilter( + RowFilter.newBuilder() + .setRowKeyRegexFilter( + ByteString.copyFromUtf8(".*"))) + .setRows( + RowSet.newBuilder() + .addRowKeys(ByteString.copyFromUtf8("row-key")) + .addRowRanges( + RowRange.newBuilder() + .setStartKeyClosed(ByteString.copyFromUtf8("j")) + .setEndKeyClosed(ByteString.copyFromUtf8("z")))) + .build(); + + RequestContext reqContext = RequestContext.create(InstanceName.of("", ""), ""); + assertThat(Query.fromProto(request).toProto(reqContext)) + .isEqualTo(request); + } } From 4a9f86981478bd9413a006e6566011d7ed9da774 Mon Sep 17 00:00:00 2001 From: Rahul Kesharawni Date: Tue, 8 Jan 2019 17:25:10 +0530 Subject: [PATCH 3/5] adding changes as per feedback --- .../bigtable/data/v2/internal/NameUtil.java | 13 ++++++++ .../cloud/bigtable/data/v2/models/Query.java | 17 +++++----- .../bigtable/data/v2/models/QueryTest.java | 32 +++++++------------ 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/NameUtil.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/NameUtil.java index c1118ed181c7..4744d3ef1ee9 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/NameUtil.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/NameUtil.java @@ -16,6 +16,8 @@ package com.google.cloud.bigtable.data.v2.internal; import com.google.api.core.InternalApi; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.annotation.Nonnull; /** @@ -26,6 +28,9 @@ */ @InternalApi public class NameUtil { + private static final Pattern TABLE_PATTERN = + Pattern.compile("projects/([^/]+)/instances/([^/]+)/tables/([^/]+)"); + public static String formatInstanceName(@Nonnull String projectId, @Nonnull String instanceId) { return "projects/" + projectId + "/instances/" + instanceId; } @@ -34,4 +39,12 @@ public static String formatTableName( @Nonnull String projectId, @Nonnull String instanceId, @Nonnull String tableId) { return formatInstanceName(projectId, instanceId) + "/tables/" + tableId; } + + public static String extractTableIdFromTableName(@Nonnull String fullTableName) { + Matcher matcher = TABLE_PATTERN.matcher(fullTableName); + if (!matcher.matches()) { + throw new IllegalArgumentException("Invalid table name: " + fullTableName); + } + return matcher.group(3); + } } diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java index f87cc1da79ec..e81ae30d49d6 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java @@ -36,6 +36,7 @@ import java.io.Serializable; import java.util.List; import java.util.SortedSet; +import javax.annotation.Nonnull; /** A simple wrapper to construct a query for the ReadRows RPC. */ public final class Query implements Serializable { @@ -261,19 +262,17 @@ public ReadRowsRequest toProto(RequestContext requestContext) { /** * Wraps the protobuf {@link ReadRowsRequest}. This method is considered an internal implementation detail * and not meant to be used by applications. + * + *

WARNING: ProjectId & InstanceId of {@link ReadRowsRequest} would be ignored. */ - @InternalApi - public static Query fromProto(ReadRowsRequest request) { + public static Query fromProto(@Nonnull ReadRowsRequest request) { Preconditions.checkArgument(request != null, "ReadRowsRequest must not be null"); - TableName tableName = TableName.parse(request.getTableName()); - String tableId = ""; - if (tableName != null) { - tableId = tableName.getTable(); - } + Query query = new Query( + NameUtil.extractTableIdFromTableName( + request.getTableName())); + query.builder = request.toBuilder(); - Query query = new Query(tableId); - query.builder = ReadRowsRequest.newBuilder(request); return query; } diff --git a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java index bf84fa045719..b6b5d637db39 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java +++ b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java @@ -38,7 +38,9 @@ import java.util.List; import java.util.SortedSet; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -50,6 +52,9 @@ public class QueryTest { private static final String APP_PROFILE_ID = "fake-profile-id"; private RequestContext requestContext; + @Rule + public ExpectedException expect = ExpectedException.none(); + @Before public void setUp() { requestContext = RequestContext.create(PROJECT_ID, INSTANCE_ID, APP_PROFILE_ID); @@ -230,7 +235,7 @@ private static ReadRowsRequest.Builder expectedProtoBuilder() { @Test public void testFromProto() { ReadRowsRequest request = ReadRowsRequest.newBuilder() - .setTableName(TABLE_NAME.toString()) + .setTableName(NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID)) .setAppProfileId(APP_PROFILE_ID) .setFilter( RowFilter.newBuilder() @@ -248,26 +253,11 @@ public void testFromProto() { .isEqualTo(request); } - @Test + @Test(expected=IllegalArgumentException.class) public void testFromProtoWithEmptyTableId() { - TableName EMPTY_TABLE_NAME = TableName.of("", "", ""); - ReadRowsRequest request = ReadRowsRequest.newBuilder() - .setTableName(EMPTY_TABLE_NAME.toString()) - .setFilter( - RowFilter.newBuilder() - .setRowKeyRegexFilter( - ByteString.copyFromUtf8(".*"))) - .setRows( - RowSet.newBuilder() - .addRowKeys(ByteString.copyFromUtf8("row-key")) - .addRowRanges( - RowRange.newBuilder() - .setStartKeyClosed(ByteString.copyFromUtf8("j")) - .setEndKeyClosed(ByteString.copyFromUtf8("z")))) - .build(); - - RequestContext reqContext = RequestContext.create(InstanceName.of("", ""), ""); - assertThat(Query.fromProto(request).toProto(reqContext)) - .isEqualTo(request); + Query.fromProto(ReadRowsRequest.getDefaultInstance()); + + expect.expect(IllegalArgumentException.class); + expect.expectMessage("Invalid table name:"); } } From f291f98752dc17d1ae34104da80b50a271e087f0 Mon Sep 17 00:00:00 2001 From: Rahul Kesharawni Date: Wed, 9 Jan 2019 13:20:36 +0530 Subject: [PATCH 4/5] updated javadoc as per feedback --- .../com/google/cloud/bigtable/data/v2/models/Query.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java index e81ae30d49d6..4444eb347818 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java @@ -260,10 +260,10 @@ public ReadRowsRequest toProto(RequestContext requestContext) { } /** - * Wraps the protobuf {@link ReadRowsRequest}. This method is considered an internal implementation detail - * and not meant to be used by applications. + * Wraps the protobuf {@link ReadRowsRequest}. * - *

WARNING: ProjectId & InstanceId of {@link ReadRowsRequest} would be ignored. + *

WARNING: Please note that the project id & instance id in the table name will be + * overwritten by the configuration in the BigtableDataClient. */ public static Query fromProto(@Nonnull ReadRowsRequest request) { Preconditions.checkArgument(request != null, "ReadRowsRequest must not be null"); From b5aa5d7d9a7ea02d525fd0b491ee0c5a1e8f05ad Mon Sep 17 00:00:00 2001 From: Rahul Kesharawni Date: Wed, 9 Jan 2019 13:43:26 +0530 Subject: [PATCH 5/5] reformatted Query & QueryTest.java --- .../cloud/bigtable/data/v2/models/Query.java | 8 ++-- .../bigtable/data/v2/models/QueryTest.java | 40 +++++++++---------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java index 4444eb347818..9ec36d5a7a52 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java @@ -262,15 +262,13 @@ public ReadRowsRequest toProto(RequestContext requestContext) { /** * Wraps the protobuf {@link ReadRowsRequest}. * - *

WARNING: Please note that the project id & instance id in the table name will be - * overwritten by the configuration in the BigtableDataClient. + *

WARNING: Please note that the project id & instance id in the table name will be overwritten + * by the configuration in the BigtableDataClient. */ public static Query fromProto(@Nonnull ReadRowsRequest request) { Preconditions.checkArgument(request != null, "ReadRowsRequest must not be null"); - Query query = new Query( - NameUtil.extractTableIdFromTableName( - request.getTableName())); + Query query = new Query(NameUtil.extractTableIdFromTableName(request.getTableName())); query.builder = request.toBuilder(); return query; diff --git a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java index b6b5d637db39..857a90916d14 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java +++ b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/QueryTest.java @@ -52,8 +52,7 @@ public class QueryTest { private static final String APP_PROFILE_ID = "fake-profile-id"; private RequestContext requestContext; - @Rule - public ExpectedException expect = ExpectedException.none(); + @Rule public ExpectedException expect = ExpectedException.none(); @Before public void setUp() { @@ -234,30 +233,29 @@ private static ReadRowsRequest.Builder expectedProtoBuilder() { @Test public void testFromProto() { - ReadRowsRequest request = ReadRowsRequest.newBuilder() - .setTableName(NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID)) - .setAppProfileId(APP_PROFILE_ID) - .setFilter( - RowFilter.newBuilder() - .setRowKeyRegexFilter(ByteString.copyFromUtf8(".*"))) - .setRows(RowSet.newBuilder() - .addRowKeys(ByteString.copyFromUtf8("row-key")) - .addRowRanges( - RowRange.newBuilder() - .setStartKeyClosed(ByteString.copyFromUtf8("j")) - .setEndKeyClosed(ByteString.copyFromUtf8("z")))) - .build(); + ReadRowsRequest request = + ReadRowsRequest.newBuilder() + .setTableName(NameUtil.formatTableName(PROJECT_ID, INSTANCE_ID, TABLE_ID)) + .setAppProfileId(APP_PROFILE_ID) + .setFilter(RowFilter.newBuilder().setRowKeyRegexFilter(ByteString.copyFromUtf8(".*"))) + .setRows( + RowSet.newBuilder() + .addRowKeys(ByteString.copyFromUtf8("row-key")) + .addRowRanges( + RowRange.newBuilder() + .setStartKeyClosed(ByteString.copyFromUtf8("j")) + .setEndKeyClosed(ByteString.copyFromUtf8("z")))) + .build(); Query query = Query.fromProto(request); - assertThat(query.toProto(requestContext)) - .isEqualTo(request); + assertThat(query.toProto(requestContext)).isEqualTo(request); } - @Test(expected=IllegalArgumentException.class) + @Test(expected = IllegalArgumentException.class) public void testFromProtoWithEmptyTableId() { - Query.fromProto(ReadRowsRequest.getDefaultInstance()); + Query.fromProto(ReadRowsRequest.getDefaultInstance()); - expect.expect(IllegalArgumentException.class); - expect.expectMessage("Invalid table name:"); + expect.expect(IllegalArgumentException.class); + expect.expectMessage("Invalid table name:"); } }