From f977c8a1c8aa189d1db8fa4e1ef44411fb32fe49 Mon Sep 17 00:00:00 2001 From: Marcello Steiner Date: Tue, 18 Sep 2018 21:28:28 +0200 Subject: [PATCH] bigquery: properly fail when setting TableId's project twice (#3694) Fixes #3283 --- .../com/google/cloud/bigquery/TableId.java | 7 +- .../cloud/bigquery/BigQueryImplTest.java | 113 ++++++++++-------- .../google/cloud/bigquery/TableIdTest.java | 3 +- 3 files changed, 71 insertions(+), 52 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableId.java b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableId.java index 18047c625a38..fc65d3f3743e 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableId.java +++ b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableId.java @@ -20,7 +20,8 @@ import com.google.api.services.bigquery.model.TableReference; import com.google.common.base.Function; - +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import java.io.Serializable; import java.util.Objects; @@ -111,7 +112,9 @@ public String toString() { } TableId setProjectId(String projectId) { - return getProject() != null ? this : TableId.of(projectId, getDataset(), getTable()); + Preconditions + .checkArgument(!Strings.isNullOrEmpty(projectId), "Provided projectId is null or empty"); + return TableId.of(projectId, getDataset(), getTable()); } TableReference toPb() { diff --git a/google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java b/google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java index 4c695b22995a..4bbfe4805131 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java +++ b/google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java @@ -282,7 +282,17 @@ public class BigQueryImplTest { private BigQueryRpc bigqueryRpcMock; private BigQuery bigquery; - @Rule public ExpectedException thrown = ExpectedException.none(); + @Rule + public ExpectedException thrown = ExpectedException.none(); + + private BigQueryOptions createBigQueryOptionsForProject(String project, + BigQueryRpcFactory rpcFactory) { + return BigQueryOptions.newBuilder() + .setProjectId(project) + .setServiceRpcFactory(rpcFactory) + .setRetrySettings(ServiceOptions.getNoRetrySettings()) + .build(); + } @Before public void setUp() { @@ -291,12 +301,7 @@ public void setUp() { EasyMock.expect(rpcFactoryMock.create(EasyMock.anyObject(BigQueryOptions.class))) .andReturn(bigqueryRpcMock); EasyMock.replay(rpcFactoryMock); - options = - BigQueryOptions.newBuilder() - .setProjectId(PROJECT) - .setServiceRpcFactory(rpcFactoryMock) - .setRetrySettings(ServiceOptions.getNoRetrySettings()) - .build(); + options = createBigQueryOptionsForProject(PROJECT, rpcFactoryMock); } @After @@ -326,7 +331,7 @@ public void testCreateDataset() { public void testCreateDatasetWithSelectedFields() { Capture> capturedOptions = Capture.newInstance(); EasyMock.expect( - bigqueryRpcMock.create(eq(DATASET_INFO_WITH_PROJECT.toPb()), capture(capturedOptions))) + bigqueryRpcMock.create(eq(DATASET_INFO_WITH_PROJECT.toPb()), capture(capturedOptions))) .andReturn(DATASET_INFO_WITH_PROJECT.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -519,8 +524,8 @@ public void testUpdateDatasetWithSelectedFields() { DatasetInfo updatedDatasetInfoWithProject = DATASET_INFO_WITH_PROJECT.toBuilder().setDescription("newDescription").build(); EasyMock.expect( - bigqueryRpcMock.patch( - eq(updatedDatasetInfoWithProject.toPb()), capture(capturedOptions))) + bigqueryRpcMock.patch( + eq(updatedDatasetInfoWithProject.toPb()), capture(capturedOptions))) .andReturn(updatedDatasetInfoWithProject.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -540,7 +545,9 @@ public void testCreateTable() { EasyMock.expect(bigqueryRpcMock.create(tableInfo.toPb(), EMPTY_RPC_OPTIONS)) .andReturn(tableInfo.toPb()); EasyMock.replay(bigqueryRpcMock); - bigquery = options.getService(); + BigQueryOptions bigQueryOptions = + createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock); + bigquery = bigQueryOptions.getService(); Table table = bigquery.create(tableInfo); assertEquals(new Table(bigquery, new TableInfo.BuilderImpl(tableInfo)), table); } @@ -549,7 +556,7 @@ public void testCreateTable() { public void testCreateTableWithSelectedFields() { Capture> capturedOptions = Capture.newInstance(); EasyMock.expect( - bigqueryRpcMock.create(eq(TABLE_INFO_WITH_PROJECT.toPb()), capture(capturedOptions))) + bigqueryRpcMock.create(eq(TABLE_INFO_WITH_PROJECT.toPb()), capture(capturedOptions))) .andReturn(TABLE_INFO_WITH_PROJECT.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -589,7 +596,9 @@ public void testGetTableFromTableIdWithProject() { EasyMock.expect(bigqueryRpcMock.getTable(OTHER_PROJECT, DATASET, TABLE, EMPTY_RPC_OPTIONS)) .andReturn(tableInfo.toPb()); EasyMock.replay(bigqueryRpcMock); - bigquery = options.getService(); + BigQueryOptions bigQueryOptions = + createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock); + bigquery = bigQueryOptions.getService(); Table table = bigquery.getTable(tableId); assertEquals(new Table(bigquery, new TableInfo.BuilderImpl(tableInfo)), table); } @@ -598,7 +607,7 @@ public void testGetTableFromTableIdWithProject() { public void testGetTableWithSelectedFields() { Capture> capturedOptions = Capture.newInstance(); EasyMock.expect( - bigqueryRpcMock.getTable(eq(PROJECT), eq(DATASET), eq(TABLE), capture(capturedOptions))) + bigqueryRpcMock.getTable(eq(PROJECT), eq(DATASET), eq(TABLE), capture(capturedOptions))) .andReturn(TABLE_INFO_WITH_PROJECT.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -700,7 +709,8 @@ public void testDeleteTableFromTableIdWithProject() { TableId tableId = TABLE_ID.setProjectId(OTHER_PROJECT); EasyMock.expect(bigqueryRpcMock.deleteTable(OTHER_PROJECT, DATASET, TABLE)).andReturn(true); EasyMock.replay(bigqueryRpcMock); - bigquery = options.getService(); + BigQueryOptions bigQueryOptions = createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock); + bigquery = bigQueryOptions.getService(); assertTrue(bigquery.delete(tableId)); } @@ -711,7 +721,8 @@ public void testUpdateTable() { EasyMock.expect(bigqueryRpcMock.patch(updatedTableInfo.toPb(), EMPTY_RPC_OPTIONS)) .andReturn(updatedTableInfo.toPb()); EasyMock.replay(bigqueryRpcMock); - bigquery = options.getService(); + BigQueryOptions bigQueryOptions = createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock); + bigquery = bigQueryOptions.getService(); Table table = bigquery.update(updatedTableInfo); assertEquals(new Table(bigquery, new TableInfo.BuilderImpl(updatedTableInfo)), table); } @@ -723,7 +734,7 @@ public void testUpdateTableWithSelectedFields() { TableInfo updatedTableInfoWithProject = TABLE_INFO_WITH_PROJECT.toBuilder().setDescription("newDescription").build(); EasyMock.expect( - bigqueryRpcMock.patch(eq(updatedTableInfoWithProject.toPb()), capture(capturedOptions))) + bigqueryRpcMock.patch(eq(updatedTableInfoWithProject.toPb()), capture(capturedOptions))) .andReturn(updatedTableInfoWithProject.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -824,7 +835,8 @@ public TableDataInsertAllRequest.Rows apply(RowToInsert rowToInsert) { EasyMock.expect(bigqueryRpcMock.insertAll(OTHER_PROJECT, DATASET, TABLE, requestPb)) .andReturn(responsePb); EasyMock.replay(bigqueryRpcMock); - bigquery = options.getService(); + BigQueryOptions bigQueryOptions = createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock); + bigquery = bigQueryOptions.getService(); InsertAllResponse response = bigquery.insertAll(request); assertNotNull(response.getErrorsFor(0L)); assertNull(response.getErrorsFor(1L)); @@ -860,7 +872,8 @@ public void testListTableDataFromTableIdWithProject() { EasyMock.expect(bigqueryRpcMock.listTableData(OTHER_PROJECT, DATASET, TABLE, EMPTY_RPC_OPTIONS)) .andReturn(TABLE_DATA_PB); EasyMock.replay(bigqueryRpcMock); - bigquery = options.getService(); + BigQueryOptions bigQueryOptions = createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock); + bigquery = bigQueryOptions.getService(); Page page = bigquery.listTableData(tableId); assertEquals(CURSOR, page.getNextPageToken()); assertArrayEquals(TABLE_DATA.toArray(), Iterables.toArray(page.getValues(), List.class)); @@ -899,7 +912,7 @@ public void testCreateJobSuccess() { Capture jobCapture = EasyMock.newCapture(); EasyMock.expect( - bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS))) + bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS))) .andReturn(newJobPb()); EasyMock.replay(bigqueryRpcMock); @@ -912,9 +925,9 @@ public void testCreateJobSuccess() { public void testCreateJobWithSelectedFields() { Capture> capturedOptions = Capture.newInstance(); EasyMock.expect( - bigqueryRpcMock.create( - EasyMock.anyObject(com.google.api.services.bigquery.model.Job.class), - EasyMock.capture(capturedOptions))) + bigqueryRpcMock.create( + EasyMock.anyObject(com.google.api.services.bigquery.model.Job.class), + EasyMock.capture(capturedOptions))) .andReturn(newJobPb()); EasyMock.replay(bigqueryRpcMock); @@ -938,7 +951,7 @@ public void testCreateJobNoGet() { Capture jobCapture = EasyMock.newCapture(); EasyMock.expect( - bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS))) + bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS))) .andThrow(new BigQueryException(409, "already exists, for some reason")); EasyMock.replay(bigqueryRpcMock); @@ -965,14 +978,14 @@ public JobId get() { Capture jobCapture = EasyMock.newCapture(); EasyMock.expect( - bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS))) + bigqueryRpcMock.create(EasyMock.capture(jobCapture), EasyMock.eq(EMPTY_RPC_OPTIONS))) .andThrow(new BigQueryException(409, "already exists, for some reason")); EasyMock.expect( - bigqueryRpcMock.getJob( - anyString(), - EasyMock.eq(id), - EasyMock.eq((String) null), - EasyMock.eq(EMPTY_RPC_OPTIONS))) + bigqueryRpcMock.getJob( + anyString(), + EasyMock.eq(id), + EasyMock.eq((String) null), + EasyMock.eq(EMPTY_RPC_OPTIONS))) .andReturn(newJobPb()); EasyMock.replay(bigqueryRpcMock); @@ -991,7 +1004,9 @@ public void testCreateJobWithProjectId() { EasyMock.expect(bigqueryRpcMock.create(eq(jobInfo.toPb()), capture(capturedOptions))) .andReturn(jobInfo.toPb()); EasyMock.replay(bigqueryRpcMock); - bigquery = options.getService(); + BigQueryOptions bigQueryOptions = + createBigQueryOptionsForProject(OTHER_PROJECT, rpcFactoryMock); + bigquery = bigQueryOptions.getService(); Job job = bigquery.create(jobInfo, JOB_OPTION_FIELDS); assertEquals(new Job(bigquery, new JobInfo.BuilderImpl(jobInfo)), job); String selector = (String) capturedOptions.getValue().get(JOB_OPTION_FIELDS.getRpcOption()); @@ -1168,16 +1183,16 @@ public void testQueryRequestCompleted() throws InterruptedException { .setSchema(TABLE_SCHEMA.toPb()); EasyMock.expect( - bigqueryRpcMock.create( - JOB_INFO.toPb(), Collections.emptyMap())) + bigqueryRpcMock.create( + JOB_INFO.toPb(), Collections.emptyMap())) .andReturn(jobResponsePb); EasyMock.expect( - bigqueryRpcMock.getQueryResults( - PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + bigqueryRpcMock.getQueryResults( + PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb); EasyMock.expect( - bigqueryRpcMock.listTableData( - PROJECT, DATASET, TABLE, Collections.emptyMap())) + bigqueryRpcMock.listTableData( + PROJECT, DATASET, TABLE, Collections.emptyMap())) .andReturn( new TableDataList() .setPageToken("") @@ -1225,8 +1240,8 @@ public void testQueryRequestCompletedOptions() throws InterruptedException { optionMap.put(pageSizeOption.getRpcOption(), pageSizeOption.getValue()); EasyMock.expect( - bigqueryRpcMock.getQueryResults( - PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + bigqueryRpcMock.getQueryResults( + PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb); EasyMock.expect(bigqueryRpcMock.listTableData(PROJECT, DATASET, TABLE, optionMap)) .andReturn( @@ -1274,24 +1289,24 @@ public void testQueryRequestCompletedOnSecondAttempt() throws InterruptedExcepti .setSchema(TABLE_SCHEMA.toPb()); EasyMock.expect( - bigqueryRpcMock.create( - JOB_INFO.toPb(), Collections.emptyMap())) + bigqueryRpcMock.create( + JOB_INFO.toPb(), Collections.emptyMap())) .andReturn(jobResponsePb1); EasyMock.expect( - bigqueryRpcMock.getJob(eq(PROJECT), eq(JOB), anyString(), anyObject(Map.class))) - .andReturn(jobResponsePb1); + bigqueryRpcMock.getJob(eq(PROJECT), eq(JOB), anyString(), anyObject(Map.class))) + .andReturn(jobResponsePb1); EasyMock.expect( - bigqueryRpcMock.getQueryResults( - PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + bigqueryRpcMock.getQueryResults( + PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb1); EasyMock.expect( - bigqueryRpcMock.getQueryResults( - PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + bigqueryRpcMock.getQueryResults( + PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb2); EasyMock.expect( - bigqueryRpcMock.listTableData( - PROJECT, DATASET, TABLE, Collections.emptyMap())) + bigqueryRpcMock.listTableData( + PROJECT, DATASET, TABLE, Collections.emptyMap())) .andReturn( new TableDataList() .setPageToken("") diff --git a/google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/TableIdTest.java b/google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/TableIdTest.java index cc1be2545ba0..c0114cda51e1 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/TableIdTest.java +++ b/google-cloud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/TableIdTest.java @@ -50,7 +50,8 @@ public void testToPbAndFromPb() { @Test public void testSetProjectId() { - assertEquals(TABLE_COMPLETE, TABLE.setProjectId("project")); + TableId differentProjectTable = TableId.of("differentProject", "dataset", "table"); + assertEquals(differentProjectTable, TABLE.setProjectId("differentProject")); } private void compareTableIds(TableId expected, TableId value) {