Skip to content

Commit f977c8a

Browse files
MMMarcypongad
MMMarcy
authored and
pongad
committed
bigquery: properly fail when setting TableId's project twice (googleapis#3694)
Fixes googleapis#3283
1 parent c90b5ba commit f977c8a

File tree

3 files changed

+71
-52
lines changed

3 files changed

+71
-52
lines changed

‎google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableId.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020

2121
import com.google.api.services.bigquery.model.TableReference;
2222
import com.google.common.base.Function;
23-
23+
import com.google.common.base.Preconditions;
24+
import com.google.common.base.Strings;
2425
import java.io.Serializable;
2526
import java.util.Objects;
2627

@@ -111,7 +112,9 @@ public String toString() {
111112
}
112113

113114
TableId setProjectId(String projectId) {
114-
return getProject() != null ? this : TableId.of(projectId, getDataset(), getTable());
115+
Preconditions
116+
.checkArgument(!Strings.isNullOrEmpty(projectId), "Provided projectId is null or empty");
117+
return TableId.of(projectId, getDataset(), getTable());
115118
}
116119

117120
TableReference toPb() {

‎google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java‎

Lines changed: 64 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,17 @@ public class BigQueryImplTest {
282282
private BigQueryRpc bigqueryRpcMock;
283283
private BigQuery bigquery;
284284

285-
@Rule public ExpectedException thrown = ExpectedException.none();
285+
@Rule
286+
public ExpectedException thrown = ExpectedException.none();
287+
288+
private BigQueryOptions createBigQueryOptionsForProject(String project,
289+
BigQueryRpcFactory rpcFactory) {
290+
return BigQueryOptions.newBuilder()
291+
.setProjectId(project)
292+
.setServiceRpcFactory(rpcFactory)
293+
.setRetrySettings(ServiceOptions.getNoRetrySettings())
294+
.build();
295+
}
286296

287297
@Before
288298
public void setUp() {
@@ -291,12 +301,7 @@ public void setUp() {
291301
EasyMock.expect(rpcFactoryMock.create(EasyMock.anyObject(BigQueryOptions.class)))
292302
.andReturn(bigqueryRpcMock);
293303
EasyMock.replay(rpcFactoryMock);
294-
options =
295-
BigQueryOptions.newBuilder()
296-
.setProjectId(PROJECT)
297-
.setServiceRpcFactory(rpcFactoryMock)
298-
.setRetrySettings(ServiceOptions.getNoRetrySettings())
299-
.build();
304+
options = createBigQueryOptionsForProject(PROJECT, rpcFactoryMock);
300305
}
301306

302307
@After
@@ -326,7 +331,7 @@ public void testCreateDataset() {
326331
public void testCreateDatasetWithSelectedFields() {
327332
Capture<Map<BigQueryRpc.Option, Object>> capturedOptions = Capture.newInstance();
328333
EasyMock.expect(
329-
bigqueryRpcMock.create(eq(DATASET_INFO_WITH_PROJECT.toPb()), capture(capturedOptions)))
334+
bigqueryRpcMock.create(eq(DATASET_INFO_WITH_PROJECT.toPb()), capture(capturedOptions)))
330335
.andReturn(DATASET_INFO_WITH_PROJECT.toPb());
331336
EasyMock.replay(bigqueryRpcMock);
332337
bigquery = options.getService();
@@ -519,8 +524,8 @@ public void testUpdateDatasetWithSelectedFields() {
519524
DatasetInfo updatedDatasetInfoWithProject =
520525
DATASET_INFO_WITH_PROJECT.toBuilder().setDescription("newDescription").build();
521526
EasyMock.expect(
522-
bigqueryRpcMock.patch(
523-
eq(updatedDatasetInfoWithProject.toPb()), capture(capturedOptions)))
527+
bigqueryRpcMock.patch(
528+
eq(updatedDatasetInfoWithProject.toPb()), capture(capturedOptions)))
524529
.andReturn(updatedDatasetInfoWithProject.toPb());
525530
EasyMock.replay(bigqueryRpcMock);
526531
bigquery = options.getService();
@@ -540,7 +545,9 @@ public void testCreateTable() {
540545
EasyMock.expect(bigqueryRpcMock.create(tableInfo.toPb(), EMPTY_RPC_OPTIONS))
541546
.andReturn(tableInfo.toPb());
542547
EasyMock.replay(bigqueryRpcMock);
543-
bigquery = options.getService();
548+
BigQueryOptions bigQueryOptions =
549+
createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock);
550+
bigquery = bigQueryOptions.getService();
544551
Table table = bigquery.create(tableInfo);
545552
assertEquals(new Table(bigquery, new TableInfo.BuilderImpl(tableInfo)), table);
546553
}
@@ -549,7 +556,7 @@ public void testCreateTable() {
549556
public void testCreateTableWithSelectedFields() {
550557
Capture<Map<BigQueryRpc.Option, Object>> capturedOptions = Capture.newInstance();
551558
EasyMock.expect(
552-
bigqueryRpcMock.create(eq(TABLE_INFO_WITH_PROJECT.toPb()), capture(capturedOptions)))
559+
bigqueryRpcMock.create(eq(TABLE_INFO_WITH_PROJECT.toPb()), capture(capturedOptions)))
553560
.andReturn(TABLE_INFO_WITH_PROJECT.toPb());
554561
EasyMock.replay(bigqueryRpcMock);
555562
bigquery = options.getService();
@@ -589,7 +596,9 @@ public void testGetTableFromTableIdWithProject() {
589596
EasyMock.expect(bigqueryRpcMock.getTable(OTHER_PROJECT, DATASET, TABLE, EMPTY_RPC_OPTIONS))
590597
.andReturn(tableInfo.toPb());
591598
EasyMock.replay(bigqueryRpcMock);
592-
bigquery = options.getService();
599+
BigQueryOptions bigQueryOptions =
600+
createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock);
601+
bigquery = bigQueryOptions.getService();
593602
Table table = bigquery.getTable(tableId);
594603
assertEquals(new Table(bigquery, new TableInfo.BuilderImpl(tableInfo)), table);
595604
}
@@ -598,7 +607,7 @@ public void testGetTableFromTableIdWithProject() {
598607
public void testGetTableWithSelectedFields() {
599608
Capture<Map<BigQueryRpc.Option, Object>> capturedOptions = Capture.newInstance();
600609
EasyMock.expect(
601-
bigqueryRpcMock.getTable(eq(PROJECT), eq(DATASET), eq(TABLE), capture(capturedOptions)))
610+
bigqueryRpcMock.getTable(eq(PROJECT), eq(DATASET), eq(TABLE), capture(capturedOptions)))
602611
.andReturn(TABLE_INFO_WITH_PROJECT.toPb());
603612
EasyMock.replay(bigqueryRpcMock);
604613
bigquery = options.getService();
@@ -700,7 +709,8 @@ public void testDeleteTableFromTableIdWithProject() {
700709
TableId tableId = TABLE_ID.setProjectId(OTHER_PROJECT);
701710
EasyMock.expect(bigqueryRpcMock.deleteTable(OTHER_PROJECT, DATASET, TABLE)).andReturn(true);
702711
EasyMock.replay(bigqueryRpcMock);
703-
bigquery = options.getService();
712+
BigQueryOptions bigQueryOptions = createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock);
713+
bigquery = bigQueryOptions.getService();
704714
assertTrue(bigquery.delete(tableId));
705715
}
706716

@@ -711,7 +721,8 @@ public void testUpdateTable() {
711721
EasyMock.expect(bigqueryRpcMock.patch(updatedTableInfo.toPb(), EMPTY_RPC_OPTIONS))
712722
.andReturn(updatedTableInfo.toPb());
713723
EasyMock.replay(bigqueryRpcMock);
714-
bigquery = options.getService();
724+
BigQueryOptions bigQueryOptions = createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock);
725+
bigquery = bigQueryOptions.getService();
715726
Table table = bigquery.update(updatedTableInfo);
716727
assertEquals(new Table(bigquery, new TableInfo.BuilderImpl(updatedTableInfo)), table);
717728
}
@@ -723,7 +734,7 @@ public void testUpdateTableWithSelectedFields() {
723734
TableInfo updatedTableInfoWithProject =
724735
TABLE_INFO_WITH_PROJECT.toBuilder().setDescription("newDescription").build();
725736
EasyMock.expect(
726-
bigqueryRpcMock.patch(eq(updatedTableInfoWithProject.toPb()), capture(capturedOptions)))
737+
bigqueryRpcMock.patch(eq(updatedTableInfoWithProject.toPb()), capture(capturedOptions)))
727738
.andReturn(updatedTableInfoWithProject.toPb());
728739
EasyMock.replay(bigqueryRpcMock);
729740
bigquery = options.getService();
@@ -824,7 +835,8 @@ public TableDataInsertAllRequest.Rows apply(RowToInsert rowToInsert) {
824835
EasyMock.expect(bigqueryRpcMock.insertAll(OTHER_PROJECT, DATASET, TABLE, requestPb))
825836
.andReturn(responsePb);
826837
EasyMock.replay(bigqueryRpcMock);
827-
bigquery = options.getService();
838+
BigQueryOptions bigQueryOptions = createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock);
839+
bigquery = bigQueryOptions.getService();
828840
InsertAllResponse response = bigquery.insertAll(request);
829841
assertNotNull(response.getErrorsFor(0L));
830842
assertNull(response.getErrorsFor(1L));
@@ -860,7 +872,8 @@ public void testListTableDataFromTableIdWithProject() {
860872
EasyMock.expect(bigqueryRpcMock.listTableData(OTHER_PROJECT, DATASET, TABLE, EMPTY_RPC_OPTIONS))
861873
.andReturn(TABLE_DATA_PB);
862874
EasyMock.replay(bigqueryRpcMock);
863-
bigquery = options.getService();
875+
BigQueryOptions bigQueryOptions = createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock);
876+
bigquery = bigQueryOptions.getService();
864877
Page<FieldValueList> page = bigquery.listTableData(tableId);
865878
assertEquals(CURSOR, page.getNextPageToken());
866879
assertArrayEquals(TABLE_DATA.toArray(), Iterables.toArray(page.getValues(), List.class));
@@ -899,7 +912,7 @@ public void testCreateJobSuccess() {
899912

900913
Capture<com.google.api.services.bigquery.model.Job> jobCapture = EasyMock.newCapture();
901914
EasyMock.expect(
902-
bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS)))
915+
bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS)))
903916
.andReturn(newJobPb());
904917
EasyMock.replay(bigqueryRpcMock);
905918

@@ -912,9 +925,9 @@ public void testCreateJobSuccess() {
912925
public void testCreateJobWithSelectedFields() {
913926
Capture<Map<BigQueryRpc.Option, Object>> capturedOptions = Capture.newInstance();
914927
EasyMock.expect(
915-
bigqueryRpcMock.create(
916-
EasyMock.anyObject(com.google.api.services.bigquery.model.Job.class),
917-
EasyMock.capture(capturedOptions)))
928+
bigqueryRpcMock.create(
929+
EasyMock.anyObject(com.google.api.services.bigquery.model.Job.class),
930+
EasyMock.capture(capturedOptions)))
918931
.andReturn(newJobPb());
919932
EasyMock.replay(bigqueryRpcMock);
920933

@@ -938,7 +951,7 @@ public void testCreateJobNoGet() {
938951

939952
Capture<com.google.api.services.bigquery.model.Job> jobCapture = EasyMock.newCapture();
940953
EasyMock.expect(
941-
bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS)))
954+
bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS)))
942955
.andThrow(new BigQueryException(409, "already exists, for some reason"));
943956
EasyMock.replay(bigqueryRpcMock);
944957

@@ -965,14 +978,14 @@ public JobId get() {
965978

966979
Capture<com.google.api.services.bigquery.model.Job> jobCapture = EasyMock.newCapture();
967980
EasyMock.expect(
968-
bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS)))
981+
bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS)))
969982
.andThrow(new BigQueryException(409, "already exists, for some reason"));
970983
EasyMock.expect(
971-
bigqueryRpcMock.getJob(
972-
anyString(),
973-
EasyMock.eq(id),
974-
EasyMock.eq((String) null),
975-
EasyMock.eq(EMPTY_RPC_OPTIONS)))
984+
bigqueryRpcMock.getJob(
985+
anyString(),
986+
EasyMock.eq(id),
987+
EasyMock.eq((String) null),
988+
EasyMock.eq(EMPTY_RPC_OPTIONS)))
976989
.andReturn(newJobPb());
977990
EasyMock.replay(bigqueryRpcMock);
978991

@@ -991,7 +1004,9 @@ public void testCreateJobWithProjectId() {
9911004
EasyMock.expect(bigqueryRpcMock.create(eq(jobInfo.toPb()), capture(capturedOptions)))
9921005
.andReturn(jobInfo.toPb());
9931006
EasyMock.replay(bigqueryRpcMock);
994-
bigquery = options.getService();
1007+
BigQueryOptions bigQueryOptions =
1008+
createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock);
1009+
bigquery = bigQueryOptions.getService();
9951010
Job job = bigquery.create(jobInfo, JOB_OPTION_FIELDS);
9961011
assertEquals(new Job(bigquery, new JobInfo.BuilderImpl(jobInfo)), job);
9971012
String selector = (String) capturedOptions.getValue().get(JOB_OPTION_FIELDS.getRpcOption());
@@ -1168,16 +1183,16 @@ public void testQueryRequestCompleted() throws InterruptedException {
11681183
.setSchema(TABLE_SCHEMA.toPb());
11691184

11701185
EasyMock.expect(
1171-
bigqueryRpcMock.create(
1172-
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>emptyMap()))
1186+
bigqueryRpcMock.create(
1187+
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>emptyMap()))
11731188
.andReturn(jobResponsePb);
11741189
EasyMock.expect(
1175-
bigqueryRpcMock.getQueryResults(
1176-
PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS)))
1190+
bigqueryRpcMock.getQueryResults(
1191+
PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS)))
11771192
.andReturn(responsePb);
11781193
EasyMock.expect(
1179-
bigqueryRpcMock.listTableData(
1180-
PROJECT, DATASET, TABLE, Collections.<BigQueryRpc.Option, Object>emptyMap()))
1194+
bigqueryRpcMock.listTableData(
1195+
PROJECT, DATASET, TABLE, Collections.<BigQueryRpc.Option, Object>emptyMap()))
11811196
.andReturn(
11821197
new TableDataList()
11831198
.setPageToken("")
@@ -1225,8 +1240,8 @@ public void testQueryRequestCompletedOptions() throws InterruptedException {
12251240
optionMap.put(pageSizeOption.getRpcOption(), pageSizeOption.getValue());
12261241

12271242
EasyMock.expect(
1228-
bigqueryRpcMock.getQueryResults(
1229-
PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS)))
1243+
bigqueryRpcMock.getQueryResults(
1244+
PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS)))
12301245
.andReturn(responsePb);
12311246
EasyMock.expect(bigqueryRpcMock.listTableData(PROJECT, DATASET, TABLE, optionMap))
12321247
.andReturn(
@@ -1274,24 +1289,24 @@ public void testQueryRequestCompletedOnSecondAttempt() throws InterruptedExcepti
12741289
.setSchema(TABLE_SCHEMA.toPb());
12751290

12761291
EasyMock.expect(
1277-
bigqueryRpcMock.create(
1278-
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>emptyMap()))
1292+
bigqueryRpcMock.create(
1293+
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>emptyMap()))
12791294
.andReturn(jobResponsePb1);
12801295
EasyMock.expect(
1281-
bigqueryRpcMock.getJob(eq(PROJECT), eq(JOB), anyString(), anyObject(Map.class)))
1282-
.andReturn(jobResponsePb1);
1296+
bigqueryRpcMock.getJob(eq(PROJECT), eq(JOB), anyString(), anyObject(Map.class)))
1297+
.andReturn(jobResponsePb1);
12831298

12841299
EasyMock.expect(
1285-
bigqueryRpcMock.getQueryResults(
1286-
PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS)))
1300+
bigqueryRpcMock.getQueryResults(
1301+
PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS)))
12871302
.andReturn(responsePb1);
12881303
EasyMock.expect(
1289-
bigqueryRpcMock.getQueryResults(
1290-
PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS)))
1304+
bigqueryRpcMock.getQueryResults(
1305+
PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS)))
12911306
.andReturn(responsePb2);
12921307
EasyMock.expect(
1293-
bigqueryRpcMock.listTableData(
1294-
PROJECT, DATASET, TABLE, Collections.<BigQueryRpc.Option, Object>emptyMap()))
1308+
bigqueryRpcMock.listTableData(
1309+
PROJECT, DATASET, TABLE, Collections.<BigQueryRpc.Option, Object>emptyMap()))
12951310
.andReturn(
12961311
new TableDataList()
12971312
.setPageToken("")

‎google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/TableIdTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ public void testToPbAndFromPb() {
5050

5151
@Test
5252
public void testSetProjectId() {
53-
assertEquals(TABLE_COMPLETE, TABLE.setProjectId("project"));
53+
TableId differentProjectTable = TableId.of("differentProject", "dataset", "table");
54+
assertEquals(differentProjectTable, TABLE.setProjectId("differentProject"));
5455
}
5556

5657
private void compareTableIds(TableId expected, TableId value) {

0 commit comments

Comments
 (0)