Skip to content

Commit

Permalink
bigquery: properly fail when setting TableId's project twice (googlea…
Browse files Browse the repository at this point in the history 
…pis#3694)

Fixes googleapis#3283
  • Loading branch information
@MMMarcy @pongad
MMMarcy authored and pongad committed Sep 18, 2018
1 parent c90b5ba commit f977c8a
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 52 deletions.
7 changes: 5 additions & 2 deletions ...-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableId.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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() {
Expand Down
113 changes: 64 additions & 49 deletions ...ients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/BigQueryImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -326,7 +331,7 @@ public void testCreateDataset() {
public void testCreateDatasetWithSelectedFields() {
Capture<Map<BigQueryRpc.Option, Object>> 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();
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}
Expand All @@ -549,7 +556,7 @@ public void testCreateTable() {
public void testCreateTableWithSelectedFields() {
Capture<Map<BigQueryRpc.Option, Object>> 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();
Expand Down Expand Up @@ -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);
}
Expand All @@ -598,7 +607,7 @@ public void testGetTableFromTableIdWithProject() {
public void testGetTableWithSelectedFields() {
Capture<Map<BigQueryRpc.Option, Object>> 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();
Expand Down Expand Up @@ -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));
}

Expand All @@ -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);
}
Expand All @@ -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();
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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<FieldValueList> page = bigquery.listTableData(tableId);
assertEquals(CURSOR, page.getNextPageToken());
assertArrayEquals(TABLE_DATA.toArray(), Iterables.toArray(page.getValues(), List.class));
Expand Down Expand Up @@ -899,7 +912,7 @@ public void testCreateJobSuccess() {

Capture<com.google.api.services.bigquery.model.Job> 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);

Expand All @@ -912,9 +925,9 @@ public void testCreateJobSuccess() {
public void testCreateJobWithSelectedFields() {
Capture<Map<BigQueryRpc.Option, Object>> 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);

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

Capture<com.google.api.services.bigquery.model.Job> 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);

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

Capture<com.google.api.services.bigquery.model.Job> 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);

Expand All @@ -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());
Expand Down Expand Up @@ -1168,16 +1183,16 @@ public void testQueryRequestCompleted() throws InterruptedException {
.setSchema(TABLE_SCHEMA.toPb());

EasyMock.expect(
bigqueryRpcMock.create(
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>emptyMap()))
bigqueryRpcMock.create(
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>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.<BigQueryRpc.Option, Object>emptyMap()))
bigqueryRpcMock.listTableData(
PROJECT, DATASET, TABLE, Collections.<BigQueryRpc.Option, Object>emptyMap()))
.andReturn(
new TableDataList()
.setPageToken("")
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -1274,24 +1289,24 @@ public void testQueryRequestCompletedOnSecondAttempt() throws InterruptedExcepti
.setSchema(TABLE_SCHEMA.toPb());

EasyMock.expect(
bigqueryRpcMock.create(
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>emptyMap()))
bigqueryRpcMock.create(
JOB_INFO.toPb(), Collections.<BigQueryRpc.Option, Object>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.<BigQueryRpc.Option, Object>emptyMap()))
bigqueryRpcMock.listTableData(
PROJECT, DATASET, TABLE, Collections.<BigQueryRpc.Option, Object>emptyMap()))
.andReturn(
new TableDataList()
.setPageToken("")
Expand Down
3 changes: 2 additions & 1 deletion ...ud-clients/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/TableIdTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit f977c8a

Please sign in to comment.