Skip to content

Commit 51bce3f

Browse files
trollyxiatrollyxia
committed
misc: Address review comments
- Remove SchemalessProto/Enum SqlType APIs - Add @BetaApi - Suppress clirr failures - Change docstring etc - Change several internal function names in SchemalessProto/Enum Type Change-Id: Ifbeb710ac77245a741500b305c2f96374a4717e9
1 parent 7927687 commit 51bce3f

File tree

9 files changed

+69
-106
lines changed

9 files changed

+69
-106
lines changed

‎google-cloud-bigtable/clirr-ignored-differences.xml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,30 @@
331331
<className>com/google/cloud/bigtable/data/v2/models/sql/Statement$Builder</className>
332332
<method>*</method>
333333
</difference>
334+
<difference>
335+
<!-- BetaApi was added -->
336+
<differenceType>7012</differenceType>
337+
<className>com/google/cloud/bigtable/data/v2/models/sql/StructReader</className>
338+
<method>*getProtoMessage(*)</method>
339+
</difference>
340+
<difference>
341+
<!-- BetaApi was added -->
342+
<differenceType>7012</differenceType>
343+
<className>com/google/cloud/bigtable/data/v2/models/sql/StructReader</className>
344+
<method>*getProtoEnum(*)</method>
345+
</difference>
346+
<difference>
347+
<!-- BetaApi was added -->
348+
<differenceType>7012</differenceType>
349+
<className>com/google/cloud/bigtable/data/v2/models/sql/SqlType</className>
350+
<method>*protoOf(*)</method>
351+
</difference>
352+
<difference>
353+
<!-- BetaApi was added -->
354+
<differenceType>7012</differenceType>
355+
<className>com/google/cloud/bigtable/data/v2/models/sql/SqlType</className>
356+
<method>*enumOf(*)</method>
357+
</difference>
334358
<difference>
335359
<!-- InternalApi was updated -->
336360
<differenceType>7004</differenceType>

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/common/Type.java‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ public static <T extends AbstractMessage> SqlType.Proto<T> create(T message) {
402402
}
403403

404404
@Nonnull
405-
abstract T message();
405+
abstract T getMessage();
406406

407407
@Override
408408
public Code getCode() {
@@ -412,12 +412,12 @@ public Code getCode() {
412412
@Nonnull
413413
@Override
414414
public Parser<T> getParserForType() {
415-
return (Parser<T>) message().getParserForType();
415+
return (Parser<T>) getMessage().getParserForType();
416416
}
417417

418418
@Override
419419
public java.lang.String getMessageName() {
420-
return message().getDescriptorForType().getFullName();
420+
return getMessage().getDescriptorForType().getFullName();
421421
}
422422

423423
@Override
@@ -489,10 +489,9 @@ public int hashCode() {
489489
}
490490

491491
/**
492-
* This is a special version of proto that is intended to only be used in the {@link
493-
* com.google.cloud.bigtable.data.v2.models.sql.StructReader} getters that require types. We don't
494-
* want users to need to specify the proto schema when the schema will be validated on calls to
495-
* {@link com.google.cloud.bigtable.data.v2.models.sql.StructReader} methods on the struct.
492+
* This is a special version of proto that is intended to only be used internally, to facilitate
493+
* proto schema parsing, which does not have the full information required to parse the protobuf
494+
* messages.
496495
*
497496
* <p>Any attempts to call getParserForType() will throw an exception.
498497
*/
@@ -507,35 +506,30 @@ public static SchemalessProto create(java.lang.String messageName) {
507506
return new AutoValue_Type_SchemalessProto(messageName);
508507
}
509508

510-
abstract java.lang.String messageName();
509+
@Override
510+
public abstract java.lang.String getMessageName();
511511

512512
@Override
513513
public Parser<AbstractMessage> getParserForType() {
514514
throw new UnsupportedOperationException(
515515
"Cannot get parser for unresolved proto type. Please use the getProtoMessage overload that takes a message instance.");
516516
}
517517

518-
@Override
519-
public java.lang.String getMessageName() {
520-
return messageName();
521-
}
522-
523518
@Override
524519
public Code getCode() {
525520
return Code.PROTO;
526521
}
527522

528523
@Override
529524
public java.lang.String toString() {
530-
return getCode().name() + "{messageName=" + messageName() + "}";
525+
return getCode().name() + "{messageName=" + getMessageName() + "}";
531526
}
532527
}
533528

534529
/**
535-
* This is a special version of enum that is intended to only be used in the {@link
536-
* com.google.cloud.bigtable.data.v2.models.sql.StructReader} getters that require types. We don't
537-
* want users to need to specify the enum schema when the schema will be validated on calls to
538-
* {@link com.google.cloud.bigtable.data.v2.models.sql.StructReader} methods on the struct.
530+
* This is a special version of enum that is intended to only be used internally, to facilitate
531+
* enum schema parsing, which does not have the full information required to parse the protobuf
532+
* enum messages.
539533
*
540534
* <p>Any attempts to call getForNumber() will throw an exception.
541535
*/
@@ -550,7 +544,7 @@ public static SchemalessEnum create(java.lang.String enumName) {
550544
return new AutoValue_Type_SchemalessEnum(enumName);
551545
}
552546

553-
abstract java.lang.String enumName();
547+
abstract java.lang.String getEnumName();
554548

555549
@Override
556550
public Function<Integer, ProtocolMessageEnum> getForNumber() {
@@ -565,14 +559,15 @@ public Code getCode() {
565559

566560
@Override
567561
public java.lang.String toString() {
568-
return getCode().name() + "{enumName=" + enumName() + "}";
562+
return getCode().name() + "{enumName=" + getEnumName() + "}";
569563
}
570564
}
571565

572566
// Implementation detail to make singleton instances private without referencing the concrete
573567
// autovalue generated class from the abstract base classes.
574568
@InternalApi
575569
class DefaultInstances {
570+
576571
private static final Bytes BYTES = new AutoValue_Type_Bytes();
577572
private static final String STRING = new AutoValue_Type_String();
578573
private static final Int64 INT64 = new AutoValue_Type_Int64();

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/sql/SqlType.java‎

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.google.cloud.bigtable.data.v2.models.sql;
1717

18+
import com.google.api.core.BetaApi;
1819
import com.google.api.core.InternalApi;
1920
import com.google.cloud.Date;
2021
import com.google.cloud.bigtable.common.Type;
@@ -244,64 +245,13 @@ static <K, V> SqlType.Map<K, V> mapOf(SqlType<K> keyType, SqlType<V> valType) {
244245
return mapOf(bytes(), arrayOf(struct()));
245246
}
246247

247-
/**
248-
* Creates a fake {@link SqlType.Proto} for use on in {@link StructReader} methods that require a
249-
* {@link SqlType} to validate against. This does not specify a schema because the proto schema
250-
* will be validated on calls to the structs data accessors.
251-
*
252-
* <p>This is considered an internal implementation detail and not meant to be used by
253-
* applications.
254-
*/
255-
@InternalApi
256-
static SqlType.Proto protoOf(com.google.bigtable.v2.Type.Proto protoType) {
257-
return Type.SchemalessProto.fromProto(protoType);
258-
}
259-
260-
/**
261-
* Creates a fake {@link SqlType.Enum} for use on in {@link StructReader} methods that require a
262-
* {@link SqlType} to validate against. This does not specify a schema because the enum schema
263-
* will be validated on calls to the structs data accessors.
264-
*
265-
* <p>This is considered an internal implementation detail and not meant to be used by
266-
* applications.
267-
*/
268-
@InternalApi
269-
static SqlType.Enum enumOf(com.google.bigtable.v2.Type.Enum enumType) {
270-
return Type.SchemalessEnum.fromProto(enumType);
271-
}
272-
273-
/**
274-
* Creates a fake {@link SqlType.Proto} for use on in {@link StructReader} methods that require a
275-
* {@link SqlType} to validate against. This does not specify a schema because the proto schema
276-
* will be validated on calls to the structs data accessors.
277-
*
278-
* <p>This is considered an internal implementation detail and not meant to be used by
279-
* applications.
280-
*/
281-
@InternalApi
282-
static SqlType.Proto protoOf(String messageName) {
283-
return Type.SchemalessProto.create(messageName);
284-
}
285-
286-
/**
287-
* Creates a fake {@link SqlType.Enum} for use on in {@link StructReader} methods that require a
288-
* {@link SqlType} to validate against. This does not specify a schema because the enum schema
289-
* will be validated on calls to the structs data accessors.
290-
*
291-
* <p>This is considered an internal implementation detail and not meant to be used by
292-
* applications.
293-
*/
294-
@InternalApi
295-
static SqlType.Enum enumOf(String enumName) {
296-
return Type.SchemalessEnum.create(enumName);
297-
}
298-
299248
/**
300249
* Returns a {@link SqlType} for a protobuf message.
301250
*
302251
* @param message an instance of the message. {@code MyMessage.getDefaultInstance()} can be used.
303252
* @param <T> the message type
304253
*/
254+
@BetaApi("This feature is currently experimental and can change in the future")
305255
static <T extends AbstractMessage> SqlType.Proto<T> protoOf(T message) {
306256
return Type.Proto.create(message);
307257
}
@@ -313,6 +263,7 @@ static <T extends AbstractMessage> SqlType.Proto<T> protoOf(T message) {
313263
* MyEnum::forNumber}
314264
* @param <T> the enum type
315265
*/
266+
@BetaApi("This feature is currently experimental and can change in the future")
316267
static <T extends ProtocolMessageEnum> SqlType.Enum<T> enumOf(Function<Integer, T> method) {
317268
return Type.Enum.create(method);
318269
}
@@ -350,9 +301,9 @@ static SqlType<?> fromProto(com.google.bigtable.v2.Type proto) {
350301
com.google.bigtable.v2.Type.Map mapType = proto.getMapType();
351302
return mapOf(fromProto(mapType.getKeyType()), fromProto(mapType.getValueType()));
352303
case PROTO_TYPE:
353-
return protoOf(proto.getProtoType());
304+
return Type.SchemalessProto.fromProto(proto.getProtoType());
354305
case ENUM_TYPE:
355-
return enumOf(proto.getEnumType());
306+
return Type.SchemalessEnum.fromProto(proto.getEnumType());
356307
case KIND_NOT_SET:
357308
throw new IllegalStateException("Unrecognized Type. You may need to update your client.");
358309
default:

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/sql/StructReader.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.google.cloud.bigtable.data.v2.models.sql;
1717

18+
import com.google.api.core.BetaApi;
1819
import com.google.cloud.Date;
1920
import com.google.protobuf.AbstractMessage;
2021
import com.google.protobuf.ByteString;
@@ -210,6 +211,7 @@ public interface StructReader {
210211
* @see <a
211212
* href="https://developers.google.com/protocol-buffers/docs/reference/java-generated#message">getDefaultInstance()</a>
212213
*/
214+
@BetaApi("This feature is currently experimental and can change in the future")
213215
<MsgType extends AbstractMessage> MsgType getProtoMessage(int columnIndex, MsgType message);
214216

215217
/**
@@ -222,6 +224,7 @@ public interface StructReader {
222224
* @see <a
223225
* href="https://developers.google.com/protocol-buffers/docs/reference/java-generated#message">getDefaultInstance()</a>
224226
*/
227+
@BetaApi("This feature is currently experimental and can change in the future")
225228
<MsgType extends AbstractMessage> MsgType getProtoMessage(String columnName, MsgType message);
226229

227230
/**
@@ -234,6 +237,7 @@ public interface StructReader {
234237
* @see <a
235238
* href="https://developers.google.com/protocol-buffers/docs/reference/java-generated#enum">forNumber()</a>
236239
*/
240+
@BetaApi("This feature is currently experimental and can change in the future")
237241
<EnumType extends ProtocolMessageEnum> EnumType getProtoEnum(
238242
int columnIndex, Function<Integer, EnumType> forNumber);
239243

@@ -247,6 +251,7 @@ <EnumType extends ProtocolMessageEnum> EnumType getProtoEnum(
247251
* @see <a
248252
* href="https://developers.google.com/protocol-buffers/docs/reference/java-generated#enum">forNumber()</a>
249253
*/
254+
@BetaApi("This feature is currently experimental and can change in the future")
250255
<EnumType extends ProtocolMessageEnum> EnumType getProtoEnum(
251256
String columnName, Function<Integer, EnumType> forNumber);
252257
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/sql/ProtoRowsMergingStateMachine.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.google.bigtable.v2.PartialResultSet;
2020
import com.google.bigtable.v2.ProtoRows;
2121
import com.google.bigtable.v2.Value;
22-
import com.google.bigtable.v2.Value.KindCase;
2322
import com.google.cloud.bigtable.data.v2.internal.ProtoSqlRow;
2423
import com.google.cloud.bigtable.data.v2.internal.SqlRow;
2524
import com.google.cloud.bigtable.data.v2.models.sql.ColumnMetadata;
@@ -257,7 +256,7 @@ static void validateValueAndType(SqlType<?> type, Value value) {
257256
case PROTO:
258257
checkExpectedKind(value, Value.KindCase.BYTES_VALUE, type);
259258
case ENUM:
260-
checkExpectedKind(value, KindCase.INT_VALUE, type);
259+
checkExpectedKind(value, Value.KindCase.INT_VALUE, type);
261260
default:
262261
// This should be caught already at ResultSetMetadata creation
263262
throw new IllegalStateException("Unrecognized type: " + type);

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/AbstractProtoStructReaderTest.java‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -289,22 +289,6 @@ public void mapField_accessingProto() {
289289
SqlType.mapOf(
290290
SqlType.bytes(),
291291
SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Singer"))));
292-
assertThrows(
293-
UnsupportedOperationException.class,
294-
() ->
295-
structWithMap.getMap(
296-
"testField",
297-
SqlType.mapOf(
298-
SqlType.bytes(),
299-
SqlType.protoOf("com.google.cloud.bigtable.data.v2.test.Singer"))));
300-
assertThrows(
301-
UnsupportedOperationException.class,
302-
() ->
303-
structWithMap.getMap(
304-
0,
305-
SqlType.mapOf(
306-
SqlType.bytes(),
307-
SqlType.protoOf("com.google.cloud.bigtable.data.v2.test.Singer"))));
308292
assertThrows(
309293
IllegalStateException.class,
310294
() ->
@@ -372,15 +356,15 @@ public void mapField_accessingEnum() {
372356
"testField",
373357
SqlType.mapOf(
374358
SqlType.bytes(),
375-
SqlType.enumOf("com.google.cloud.bigtable.data.v2.test.Genre"))));
359+
SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre"))));
376360
assertThrows(
377361
UnsupportedOperationException.class,
378362
() ->
379363
structWithMap.getMap(
380364
0,
381365
SqlType.mapOf(
382366
SqlType.bytes(),
383-
SqlType.enumOf("com.google.cloud.bigtable.data.v2.test.Genre"))));
367+
SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre"))));
384368
assertThrows(
385369
IllegalStateException.class,
386370
() -> structWithMap.getMap("testField", SqlType.mapOf(SqlType.bytes(), SqlType.bytes())));

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/ProtoStructTest.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
4747
import com.google.bigtable.v2.Type.Struct;
4848
import com.google.bigtable.v2.Value;
4949
import com.google.cloud.Date;
50+
import com.google.cloud.bigtable.common.Type.SchemalessEnum;
51+
import com.google.cloud.bigtable.common.Type.SchemalessProto;
5052
import com.google.cloud.bigtable.data.v2.models.sql.SqlType;
5153
import com.google.cloud.bigtable.data.v2.test.SingerProto.Genre;
5254
import com.google.cloud.bigtable.data.v2.test.SingerProto.Singer;
@@ -182,9 +184,9 @@ public void getColumnType_byName() {
182184
assertThat(struct.getColumnType("mapField"))
183185
.isEqualTo(SqlType.mapOf(SqlType.string(), SqlType.string()));
184186
assertThat(struct.getColumnType("protoField"))
185-
.isEqualTo(SqlType.protoOf("com.google.cloud.bigtable.data.v2.test.Singer"));
187+
.isEqualTo(SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Singer"));
186188
assertThat(struct.getColumnType("enumField"))
187-
.isEqualTo(SqlType.enumOf("com.google.cloud.bigtable.data.v2.test.Genre"));
189+
.isEqualTo(SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre"));
188190
}
189191

190192
@Test
@@ -203,9 +205,9 @@ public void getColumnType_byIndex() {
203205
assertThat(struct.getColumnType(10))
204206
.isEqualTo(SqlType.mapOf(SqlType.string(), SqlType.string()));
205207
assertThat(struct.getColumnType(11))
206-
.isEqualTo(SqlType.protoOf("com.google.cloud.bigtable.data.v2.test.Singer"));
208+
.isEqualTo(SchemalessProto.create("com.google.cloud.bigtable.data.v2.test.Singer"));
207209
assertThat(struct.getColumnType(12))
208-
.isEqualTo(SqlType.enumOf("com.google.cloud.bigtable.data.v2.test.Genre"));
210+
.isEqualTo(SchemalessEnum.create("com.google.cloud.bigtable.data.v2.test.Genre"));
209211
}
210212

211213
@Test

0 commit comments

Comments
 (0)