From e6c4f2b85f6e03c1177076f4749349fdd8ab0076 Mon Sep 17 00:00:00 2001 From: Praful Makani Date: Wed, 9 Jan 2019 19:21:51 +0530 Subject: [PATCH 1/4] Fix Location configurable from BigQueryOptions --- .../google/cloud/bigquery/BigQueryImpl.java | 11 ++++--- .../java/com/google/cloud/bigquery/JobId.java | 4 +++ .../cloud/bigquery/BigQueryImplTest.java | 31 ++++++++++--------- .../java/com/google/cloud/ServiceOptions.java | 26 ++++++++++++++++ 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java index e30b0ffd2156..572a7e39abbd 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java +++ b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java @@ -679,7 +679,8 @@ public Job getJob(String jobId, JobOption... options) { @Override public Job getJob(JobId jobId, JobOption... options) { final Map optionsMap = optionMap(options); - final JobId completeJobId = jobId.setProjectId(getOptions().getProjectId()); + final JobId completeJobId = + jobId.setProjectId(getOptions().getProjectId()).setLocation(getOptions().getLocation()); try { com.google.api.services.bigquery.model.Job answer = runWithRetries( @@ -742,7 +743,8 @@ public boolean cancel(String jobId) { @Override public boolean cancel(JobId jobId) { - final JobId completeJobId = jobId.setProjectId(getOptions().getProjectId()); + final JobId completeJobId = + jobId.setProjectId(getOptions().getProjectId()).setLocation(getOptions().getLocation()); try { return runWithRetries( new Callable() { @@ -784,7 +786,8 @@ private static QueryResponse getQueryResults( JobId jobId, final BigQueryOptions serviceOptions, final Map optionsMap) { - final JobId completeJobId = jobId.setProjectId(serviceOptions.getProjectId()); + final JobId completeJobId = + jobId.setProjectId(serviceOptions.getProjectId()).setLocation(serviceOptions.getLocation()); try { GetQueryResultsResponse results = runWithRetries( @@ -833,7 +836,7 @@ public TableDataWriteChannel writer( JobId jobId, WriteChannelConfiguration writeChannelConfiguration) { return new TableDataWriteChannel( getOptions(), - jobId.setProjectId(getOptions().getProjectId()), + jobId.setProjectId(getOptions().getProjectId()).setLocation(getOptions().getLocation()), writeChannelConfiguration.setProjectId(getOptions().getProjectId())); } diff --git a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/JobId.java b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/JobId.java index 254e69647e1a..05f80bbf376f 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/JobId.java +++ b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/JobId.java @@ -101,6 +101,10 @@ JobId setProjectId(String projectId) { return getProject() != null ? this : toBuilder().setProject(projectId).build(); } + JobId setLocation(String location) { + return getLocation() != null ? this : toBuilder().setLocation(location).build(); + } + JobReference toPb() { return new JobReference() .setProjectId(getProject()) 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 9b67445acd13..f4d01fd5a5cd 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 @@ -66,6 +66,7 @@ public class BigQueryImplTest { private static final String PROJECT = "project"; + private static final String LOCATION = "US"; private static final String OTHER_PROJECT = "otherProject"; private static final String DATASET = "dataset"; private static final String TABLE = "table"; @@ -288,6 +289,7 @@ private BigQueryOptions createBigQueryOptionsForProject( String project, BigQueryRpcFactory rpcFactory) { return BigQueryOptions.newBuilder() .setProjectId(project) + .setLocation(LOCATION) .setServiceRpcFactory(rpcFactory) .setRetrySettings(ServiceOptions.getNoRetrySettings()) .build(); @@ -1137,7 +1139,7 @@ public JobId get() { bigqueryRpcMock.getJob( anyString(), EasyMock.eq(id), - EasyMock.eq((String) null), + EasyMock.eq((String) LOCATION), EasyMock.eq(EMPTY_RPC_OPTIONS))) .andReturn(newJobPb()); EasyMock.replay(bigqueryRpcMock); @@ -1171,7 +1173,7 @@ public void testCreateJobWithProjectId() { @Test public void testGetJob() { - EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) .andReturn(COMPLETE_COPY_JOB.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1181,7 +1183,7 @@ public void testGetJob() { @Test public void testGetJobFromJobId() { - EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) .andReturn(COMPLETE_COPY_JOB.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1193,7 +1195,7 @@ public void testGetJobFromJobId() { public void testGetJobFromJobIdWithProject() { JobId jobId = JobId.of(OTHER_PROJECT, JOB); JobInfo jobInfo = COPY_JOB.setProjectId(OTHER_PROJECT); - EasyMock.expect(bigqueryRpcMock.getJob(OTHER_PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getJob(OTHER_PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) .andReturn(jobInfo.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1291,7 +1293,7 @@ public com.google.api.services.bigquery.model.Job apply(Job job) { @Test public void testCancelJob() { - EasyMock.expect(bigqueryRpcMock.cancel(PROJECT, JOB, null)).andReturn(true); + EasyMock.expect(bigqueryRpcMock.cancel(PROJECT, JOB, LOCATION)).andReturn(true); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); assertTrue(bigquery.cancel(JOB)); @@ -1299,7 +1301,7 @@ public void testCancelJob() { @Test public void testCancelJobFromJobId() { - EasyMock.expect(bigqueryRpcMock.cancel(PROJECT, JOB, null)).andReturn(true); + EasyMock.expect(bigqueryRpcMock.cancel(PROJECT, JOB, LOCATION)).andReturn(true); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); assertTrue(bigquery.cancel(JobId.of(PROJECT, JOB))); @@ -1308,7 +1310,7 @@ public void testCancelJobFromJobId() { @Test public void testCancelJobFromJobIdWithProject() { JobId jobId = JobId.of(OTHER_PROJECT, JOB); - EasyMock.expect(bigqueryRpcMock.cancel(OTHER_PROJECT, JOB, null)).andReturn(true); + EasyMock.expect(bigqueryRpcMock.cancel(OTHER_PROJECT, JOB, LOCATION)).andReturn(true); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); assertTrue(bigquery.cancel(jobId)); @@ -1341,7 +1343,7 @@ public void testQueryRequestCompleted() throws InterruptedException { .andReturn(jobResponsePb); EasyMock.expect( bigqueryRpcMock.getQueryResults( - PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + PROJECT, JOB, LOCATION, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb); EasyMock.expect( bigqueryRpcMock.listTableData( @@ -1395,7 +1397,7 @@ public void testQueryRequestCompletedOptions() throws InterruptedException { EasyMock.expect( bigqueryRpcMock.getQueryResults( - PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + PROJECT, JOB, LOCATION, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb); EasyMock.expect(bigqueryRpcMock.listTableData(PROJECT, DATASET, TABLE, optionMap)) .andReturn( @@ -1449,11 +1451,11 @@ public void testQueryRequestCompletedOnSecondAttempt() throws InterruptedExcepti EasyMock.expect( bigqueryRpcMock.getQueryResults( - PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + PROJECT, JOB, LOCATION, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb1); EasyMock.expect( bigqueryRpcMock.getQueryResults( - PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + PROJECT, JOB, LOCATION, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb2); EasyMock.expect( bigqueryRpcMock.listTableData( @@ -1488,7 +1490,7 @@ public void testGetQueryResults() { .setPageToken(CURSOR) .setTotalBytesProcessed(42L) .setTotalRows(BigInteger.valueOf(1L)); - EasyMock.expect(bigqueryRpcMock.getQueryResults(PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getQueryResults(PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) .andReturn(responsePb); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1510,7 +1512,8 @@ public void testGetQueryResultsWithProject() { .setPageToken(CURSOR) .setTotalBytesProcessed(42L) .setTotalRows(BigInteger.valueOf(1L)); - EasyMock.expect(bigqueryRpcMock.getQueryResults(OTHER_PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) + EasyMock.expect( + bigqueryRpcMock.getQueryResults(OTHER_PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) .andReturn(responsePb); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1531,7 +1534,7 @@ public void testGetQueryResultsWithOptions() { .setPageToken(CURSOR) .setTotalBytesProcessed(42L) .setTotalRows(BigInteger.valueOf(1L)); - EasyMock.expect(bigqueryRpcMock.getQueryResults(PROJECT, JOB, null, QUERY_RESULTS_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getQueryResults(PROJECT, JOB, LOCATION, QUERY_RESULTS_OPTIONS)) .andReturn(responsePb); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java index 141c99857da4..dee6d6388bf1 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java @@ -82,6 +82,7 @@ public abstract class ServiceOptions< private static final String DEFAULT_HOST = "https://www.googleapis.com"; private static final String LEGACY_PROJECT_ENV_NAME = "GCLOUD_PROJECT"; private static final String PROJECT_ENV_NAME = "GOOGLE_CLOUD_PROJECT"; + private static final String DEFAULT_LOCATION = "US"; private static final RetrySettings DEFAULT_RETRY_SETTINGS = getDefaultRetrySettingsBuilder().build(); @@ -91,6 +92,7 @@ public abstract class ServiceOptions< private static final long serialVersionUID = 9198896031667942014L; private final String projectId; + private final String location; private final String host; private final RetrySettings retrySettings; private final String serviceRpcFactoryClassName; @@ -118,6 +120,7 @@ public abstract static class Builder< B extends Builder> { private String projectId; + private String location; private String host; protected Credentials credentials; private RetrySettings retrySettings; @@ -133,6 +136,7 @@ protected Builder() {} @InternalApi("This class should only be extended within google-cloud-java") protected Builder(ServiceOptions options) { projectId = options.projectId; + location = options.location; host = options.host; credentials = options.credentials; retrySettings = options.retrySettings; @@ -178,6 +182,16 @@ public B setProjectId(String projectId) { return self(); } + /** + * Sets service location + * + * @return the builder + */ + public B setLocation(String location) { + this.location = location; + return self(); + } + /** * Sets service host. * @@ -262,6 +276,7 @@ protected ServiceOptions( Builder builder, ServiceDefaults serviceDefaults) { projectId = builder.projectId != null ? builder.projectId : getDefaultProject(); + location = builder.location != null ? builder.location : getDefaultLocation(); if (projectIdRequired()) { checkArgument( projectId != null, @@ -313,6 +328,10 @@ protected String getDefaultProject() { return getDefaultProjectId(); } + protected String getDefaultLocation() { + return DEFAULT_LOCATION; + } + /** * Returns the default project ID, or {@code null} if no default project ID could be found. This * method returns the first available project ID among the following sources: @@ -519,6 +538,11 @@ public String getProjectId() { return projectId; } + /** Returns the service location */ + public String getLocation() { + return location; + } + /** Returns the service host. */ public String getHost() { return host; @@ -622,6 +646,7 @@ public final String getUserAgent() { protected int baseHashCode() { return Objects.hash( projectId, + location, host, credentials, retrySettings, @@ -632,6 +657,7 @@ protected int baseHashCode() { protected boolean baseEquals(ServiceOptions other) { return Objects.equals(projectId, other.projectId) + && Objects.equals(location, other.location) && Objects.equals(host, other.host) && Objects.equals(credentials, other.credentials) && Objects.equals(retrySettings, other.retrySettings) From 4dd861bcb972affa5c8cb8f31d31f1bc6c95a39a Mon Sep 17 00:00:00 2001 From: Praful Makani Date: Fri, 11 Jan 2019 18:44:45 +0530 Subject: [PATCH 2/4] modified code --- .../google/cloud/bigquery/BigQueryImpl.java | 17 ++++++++--- .../cloud/bigquery/BigQueryOptions.java | 16 +++++++++++ .../cloud/bigquery/BigQueryImplTest.java | 28 +++++++++---------- .../java/com/google/cloud/ServiceOptions.java | 25 ----------------- 4 files changed, 43 insertions(+), 43 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java index 572a7e39abbd..6e52896a3757 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java +++ b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java @@ -680,7 +680,10 @@ public Job getJob(String jobId, JobOption... options) { public Job getJob(JobId jobId, JobOption... options) { final Map optionsMap = optionMap(options); final JobId completeJobId = - jobId.setProjectId(getOptions().getProjectId()).setLocation(getOptions().getLocation()); + jobId.setProjectId(getOptions().getProjectId()); + if (jobId.getLocation() == null && getOptions().getLocation() != null) { + completeJobId.setLocation(getOptions().getLocation()); + } try { com.google.api.services.bigquery.model.Job answer = runWithRetries( @@ -744,7 +747,10 @@ public boolean cancel(String jobId) { @Override public boolean cancel(JobId jobId) { final JobId completeJobId = - jobId.setProjectId(getOptions().getProjectId()).setLocation(getOptions().getLocation()); + jobId.setProjectId(getOptions().getProjectId()); + if (jobId.getLocation() == null && getOptions().getLocation() != null) { + completeJobId.setLocation(getOptions().getLocation()); + } try { return runWithRetries( new Callable() { @@ -787,7 +793,10 @@ private static QueryResponse getQueryResults( final BigQueryOptions serviceOptions, final Map optionsMap) { final JobId completeJobId = - jobId.setProjectId(serviceOptions.getProjectId()).setLocation(serviceOptions.getLocation()); + jobId.setProjectId(serviceOptions.getProjectId()); + if (jobId.getLocation() == null && serviceOptions.getLocation() != null) { + completeJobId.setLocation(serviceOptions.getLocation()); + } try { GetQueryResultsResponse results = runWithRetries( @@ -836,7 +845,7 @@ public TableDataWriteChannel writer( JobId jobId, WriteChannelConfiguration writeChannelConfiguration) { return new TableDataWriteChannel( getOptions(), - jobId.setProjectId(getOptions().getProjectId()).setLocation(getOptions().getLocation()), + jobId.setProjectId(getOptions().getProjectId()), writeChannelConfiguration.setProjectId(getOptions().getProjectId())); } diff --git a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryOptions.java b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryOptions.java index 080bb57521f3..7b8418306e23 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryOptions.java +++ b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryOptions.java @@ -33,6 +33,7 @@ public class BigQueryOptions extends ServiceOptions { private static final String BIGQUERY_SCOPE = "https://www.googleapis.com/auth/bigquery"; private static final Set SCOPES = ImmutableSet.of(BIGQUERY_SCOPE); private static final long serialVersionUID = -2437598817433266049L; + private String location; public static class DefaultBigQueryFactory implements BigQueryFactory { @@ -56,6 +57,8 @@ public ServiceRpc create(BigQueryOptions options) { public static class Builder extends ServiceOptions.Builder { + private String location; + private Builder() {} private Builder(BigQueryOptions options) { @@ -71,6 +74,11 @@ public Builder setTransportOptions(TransportOptions transportOptions) { return super.setTransportOptions(transportOptions); } + public Builder setLocation(String location) { + this.location = location; + return this; + } + @Override public BigQueryOptions build() { return new BigQueryOptions(this); @@ -112,6 +120,14 @@ protected BigQueryRpc getBigQueryRpcV2() { return (BigQueryRpc) getRpc(); } + public void setLocation(String location) { + this.location = location; + } + + public String getLocation() { + return location; + } + @SuppressWarnings("unchecked") @Override public Builder toBuilder() { 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 f4d01fd5a5cd..0b0975c97ef9 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 @@ -1139,7 +1139,7 @@ public JobId get() { bigqueryRpcMock.getJob( anyString(), EasyMock.eq(id), - EasyMock.eq((String) LOCATION), + EasyMock.eq((String) null), EasyMock.eq(EMPTY_RPC_OPTIONS))) .andReturn(newJobPb()); EasyMock.replay(bigqueryRpcMock); @@ -1173,7 +1173,7 @@ public void testCreateJobWithProjectId() { @Test public void testGetJob() { - EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) .andReturn(COMPLETE_COPY_JOB.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1183,7 +1183,7 @@ public void testGetJob() { @Test public void testGetJobFromJobId() { - EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) .andReturn(COMPLETE_COPY_JOB.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1195,7 +1195,7 @@ public void testGetJobFromJobId() { public void testGetJobFromJobIdWithProject() { JobId jobId = JobId.of(OTHER_PROJECT, JOB); JobInfo jobInfo = COPY_JOB.setProjectId(OTHER_PROJECT); - EasyMock.expect(bigqueryRpcMock.getJob(OTHER_PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getJob(OTHER_PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) .andReturn(jobInfo.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1293,7 +1293,7 @@ public com.google.api.services.bigquery.model.Job apply(Job job) { @Test public void testCancelJob() { - EasyMock.expect(bigqueryRpcMock.cancel(PROJECT, JOB, LOCATION)).andReturn(true); + EasyMock.expect(bigqueryRpcMock.cancel(PROJECT, JOB, null)).andReturn(true); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); assertTrue(bigquery.cancel(JOB)); @@ -1301,7 +1301,7 @@ public void testCancelJob() { @Test public void testCancelJobFromJobId() { - EasyMock.expect(bigqueryRpcMock.cancel(PROJECT, JOB, LOCATION)).andReturn(true); + EasyMock.expect(bigqueryRpcMock.cancel(PROJECT, JOB, null)).andReturn(true); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); assertTrue(bigquery.cancel(JobId.of(PROJECT, JOB))); @@ -1310,7 +1310,7 @@ public void testCancelJobFromJobId() { @Test public void testCancelJobFromJobIdWithProject() { JobId jobId = JobId.of(OTHER_PROJECT, JOB); - EasyMock.expect(bigqueryRpcMock.cancel(OTHER_PROJECT, JOB, LOCATION)).andReturn(true); + EasyMock.expect(bigqueryRpcMock.cancel(OTHER_PROJECT, JOB, null)).andReturn(true); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); assertTrue(bigquery.cancel(jobId)); @@ -1343,7 +1343,7 @@ public void testQueryRequestCompleted() throws InterruptedException { .andReturn(jobResponsePb); EasyMock.expect( bigqueryRpcMock.getQueryResults( - PROJECT, JOB, LOCATION, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb); EasyMock.expect( bigqueryRpcMock.listTableData( @@ -1397,7 +1397,7 @@ public void testQueryRequestCompletedOptions() throws InterruptedException { EasyMock.expect( bigqueryRpcMock.getQueryResults( - PROJECT, JOB, LOCATION, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb); EasyMock.expect(bigqueryRpcMock.listTableData(PROJECT, DATASET, TABLE, optionMap)) .andReturn( @@ -1451,11 +1451,11 @@ public void testQueryRequestCompletedOnSecondAttempt() throws InterruptedExcepti EasyMock.expect( bigqueryRpcMock.getQueryResults( - PROJECT, JOB, LOCATION, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb1); EasyMock.expect( bigqueryRpcMock.getQueryResults( - PROJECT, JOB, LOCATION, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) + PROJECT, JOB, null, BigQueryImpl.optionMap(Job.DEFAULT_QUERY_WAIT_OPTIONS))) .andReturn(responsePb2); EasyMock.expect( bigqueryRpcMock.listTableData( @@ -1490,7 +1490,7 @@ public void testGetQueryResults() { .setPageToken(CURSOR) .setTotalBytesProcessed(42L) .setTotalRows(BigInteger.valueOf(1L)); - EasyMock.expect(bigqueryRpcMock.getQueryResults(PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getQueryResults(PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) .andReturn(responsePb); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1513,7 +1513,7 @@ public void testGetQueryResultsWithProject() { .setTotalBytesProcessed(42L) .setTotalRows(BigInteger.valueOf(1L)); EasyMock.expect( - bigqueryRpcMock.getQueryResults(OTHER_PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) + bigqueryRpcMock.getQueryResults(OTHER_PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) .andReturn(responsePb); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); @@ -1534,7 +1534,7 @@ public void testGetQueryResultsWithOptions() { .setPageToken(CURSOR) .setTotalBytesProcessed(42L) .setTotalRows(BigInteger.valueOf(1L)); - EasyMock.expect(bigqueryRpcMock.getQueryResults(PROJECT, JOB, LOCATION, QUERY_RESULTS_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getQueryResults(PROJECT, JOB, null, QUERY_RESULTS_OPTIONS)) .andReturn(responsePb); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java index dee6d6388bf1..f93c62dcabb8 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java @@ -82,7 +82,6 @@ public abstract class ServiceOptions< private static final String DEFAULT_HOST = "https://www.googleapis.com"; private static final String LEGACY_PROJECT_ENV_NAME = "GCLOUD_PROJECT"; private static final String PROJECT_ENV_NAME = "GOOGLE_CLOUD_PROJECT"; - private static final String DEFAULT_LOCATION = "US"; private static final RetrySettings DEFAULT_RETRY_SETTINGS = getDefaultRetrySettingsBuilder().build(); @@ -92,7 +91,6 @@ public abstract class ServiceOptions< private static final long serialVersionUID = 9198896031667942014L; private final String projectId; - private final String location; private final String host; private final RetrySettings retrySettings; private final String serviceRpcFactoryClassName; @@ -136,7 +134,6 @@ protected Builder() {} @InternalApi("This class should only be extended within google-cloud-java") protected Builder(ServiceOptions options) { projectId = options.projectId; - location = options.location; host = options.host; credentials = options.credentials; retrySettings = options.retrySettings; @@ -182,16 +179,6 @@ public B setProjectId(String projectId) { return self(); } - /** - * Sets service location - * - * @return the builder - */ - public B setLocation(String location) { - this.location = location; - return self(); - } - /** * Sets service host. * @@ -276,7 +263,6 @@ protected ServiceOptions( Builder builder, ServiceDefaults serviceDefaults) { projectId = builder.projectId != null ? builder.projectId : getDefaultProject(); - location = builder.location != null ? builder.location : getDefaultLocation(); if (projectIdRequired()) { checkArgument( projectId != null, @@ -328,10 +314,6 @@ protected String getDefaultProject() { return getDefaultProjectId(); } - protected String getDefaultLocation() { - return DEFAULT_LOCATION; - } - /** * Returns the default project ID, or {@code null} if no default project ID could be found. This * method returns the first available project ID among the following sources: @@ -538,11 +520,6 @@ public String getProjectId() { return projectId; } - /** Returns the service location */ - public String getLocation() { - return location; - } - /** Returns the service host. */ public String getHost() { return host; @@ -646,7 +623,6 @@ public final String getUserAgent() { protected int baseHashCode() { return Objects.hash( projectId, - location, host, credentials, retrySettings, @@ -657,7 +633,6 @@ protected int baseHashCode() { protected boolean baseEquals(ServiceOptions other) { return Objects.equals(projectId, other.projectId) - && Objects.equals(location, other.location) && Objects.equals(host, other.host) && Objects.equals(credentials, other.credentials) && Objects.equals(retrySettings, other.retrySettings) From 91cbd36ce293c642f6108edd0a793e2e4956e778 Mon Sep 17 00:00:00 2001 From: Praful Makani Date: Fri, 11 Jan 2019 20:47:30 +0530 Subject: [PATCH 3/4] modified code and add test case --- .../google/cloud/bigquery/BigQueryImpl.java | 30 +++++++----- .../cloud/bigquery/BigQueryOptions.java | 9 ++-- .../cloud/bigquery/BigQueryImplTest.java | 47 ++++++++++++++++++- 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java index 6e52896a3757..09376fe13154 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java +++ b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java @@ -680,10 +680,12 @@ public Job getJob(String jobId, JobOption... options) { public Job getJob(JobId jobId, JobOption... options) { final Map optionsMap = optionMap(options); final JobId completeJobId = - jobId.setProjectId(getOptions().getProjectId()); - if (jobId.getLocation() == null && getOptions().getLocation() != null) { - completeJobId.setLocation(getOptions().getLocation()); - } + jobId + .setProjectId(getOptions().getProjectId()) + .setLocation( + jobId.getLocation() == null && getOptions().getLocation() != null + ? getOptions().getLocation() + : jobId.getLocation()); try { com.google.api.services.bigquery.model.Job answer = runWithRetries( @@ -747,10 +749,12 @@ public boolean cancel(String jobId) { @Override public boolean cancel(JobId jobId) { final JobId completeJobId = - jobId.setProjectId(getOptions().getProjectId()); - if (jobId.getLocation() == null && getOptions().getLocation() != null) { - completeJobId.setLocation(getOptions().getLocation()); - } + jobId + .setProjectId(getOptions().getProjectId()) + .setLocation( + jobId.getLocation() == null && getOptions().getLocation() != null + ? getOptions().getLocation() + : jobId.getLocation()); try { return runWithRetries( new Callable() { @@ -793,10 +797,12 @@ private static QueryResponse getQueryResults( final BigQueryOptions serviceOptions, final Map optionsMap) { final JobId completeJobId = - jobId.setProjectId(serviceOptions.getProjectId()); - if (jobId.getLocation() == null && serviceOptions.getLocation() != null) { - completeJobId.setLocation(serviceOptions.getLocation()); - } + jobId + .setProjectId(serviceOptions.getProjectId()) + .setLocation( + jobId.getLocation() == null && serviceOptions.getLocation() != null + ? serviceOptions.getLocation() + : jobId.getLocation()); try { GetQueryResultsResponse results = runWithRetries( diff --git a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryOptions.java b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryOptions.java index 7b8418306e23..fecf5005f157 100644 --- a/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryOptions.java +++ b/google-cloud-clients/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryOptions.java @@ -33,7 +33,7 @@ public class BigQueryOptions extends ServiceOptions { private static final String BIGQUERY_SCOPE = "https://www.googleapis.com/auth/bigquery"; private static final Set SCOPES = ImmutableSet.of(BIGQUERY_SCOPE); private static final long serialVersionUID = -2437598817433266049L; - private String location; + private final String location; public static class DefaultBigQueryFactory implements BigQueryFactory { @@ -87,6 +87,7 @@ public BigQueryOptions build() { private BigQueryOptions(Builder builder) { super(BigQueryFactory.class, BigQueryRpcFactory.class, builder, new BigQueryDefaults()); + this.location = builder.location; } private static class BigQueryDefaults implements ServiceDefaults { @@ -120,12 +121,8 @@ protected BigQueryRpc getBigQueryRpcV2() { return (BigQueryRpc) getRpc(); } - public void setLocation(String location) { - this.location = location; - } - public String getLocation() { - return location; + return location; } @SuppressWarnings("unchecked") 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 0b0975c97ef9..6d3348cae0a6 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 @@ -287,6 +287,15 @@ public class BigQueryImplTest { private BigQueryOptions createBigQueryOptionsForProject( String project, BigQueryRpcFactory rpcFactory) { + return BigQueryOptions.newBuilder() + .setProjectId(project) + .setServiceRpcFactory(rpcFactory) + .setRetrySettings(ServiceOptions.getNoRetrySettings()) + .build(); + } + + private BigQueryOptions createBigQueryOptionsForProjectWithLocation( + String project, BigQueryRpcFactory rpcFactory) { return BigQueryOptions.newBuilder() .setProjectId(project) .setLocation(LOCATION) @@ -1181,6 +1190,17 @@ public void testGetJob() { assertEquals(new Job(bigquery, new JobInfo.BuilderImpl(COMPLETE_COPY_JOB)), job); } + @Test + public void testGetJobWithLocation() { + EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) + .andReturn(COMPLETE_COPY_JOB.toPb()); + EasyMock.replay(bigqueryRpcMock); + BigQueryOptions options = createBigQueryOptionsForProjectWithLocation(PROJECT, rpcFactoryMock); + bigquery = options.getService(); + Job job = bigquery.getJob(JOB); + assertEquals(new Job(bigquery, new JobInfo.BuilderImpl(COMPLETE_COPY_JOB)), job); + } + @Test public void testGetJobFromJobId() { EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) @@ -1191,6 +1211,17 @@ public void testGetJobFromJobId() { assertEquals(new Job(bigquery, new JobInfo.BuilderImpl(COMPLETE_COPY_JOB)), job); } + @Test + public void testGetJobFromJobIdWithLocation() { + EasyMock.expect(bigqueryRpcMock.getJob(PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) + .andReturn(COMPLETE_COPY_JOB.toPb()); + EasyMock.replay(bigqueryRpcMock); + BigQueryOptions options = createBigQueryOptionsForProjectWithLocation(PROJECT, rpcFactoryMock); + bigquery = options.getService(); + Job job = bigquery.getJob(JobId.of(JOB)); + assertEquals(new Job(bigquery, new JobInfo.BuilderImpl(COMPLETE_COPY_JOB)), job); + } + @Test public void testGetJobFromJobIdWithProject() { JobId jobId = JobId.of(OTHER_PROJECT, JOB); @@ -1203,6 +1234,19 @@ public void testGetJobFromJobIdWithProject() { assertEquals(new Job(bigquery, new JobInfo.BuilderImpl(jobInfo)), job); } + @Test + public void testGetJobFromJobIdWithProjectWithLocation() { + JobId jobId = JobId.of(OTHER_PROJECT, JOB); + JobInfo jobInfo = COPY_JOB.setProjectId(OTHER_PROJECT); + EasyMock.expect(bigqueryRpcMock.getJob(OTHER_PROJECT, JOB, LOCATION, EMPTY_RPC_OPTIONS)) + .andReturn(jobInfo.toPb()); + EasyMock.replay(bigqueryRpcMock); + BigQueryOptions options = createBigQueryOptionsForProjectWithLocation(PROJECT, rpcFactoryMock); + bigquery = options.getService(); + Job job = bigquery.getJob(jobId); + assertEquals(new Job(bigquery, new JobInfo.BuilderImpl(jobInfo)), job); + } + @Test public void testListJobs() { bigquery = options.getService(); @@ -1512,8 +1556,7 @@ public void testGetQueryResultsWithProject() { .setPageToken(CURSOR) .setTotalBytesProcessed(42L) .setTotalRows(BigInteger.valueOf(1L)); - EasyMock.expect( - bigqueryRpcMock.getQueryResults(OTHER_PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) + EasyMock.expect(bigqueryRpcMock.getQueryResults(OTHER_PROJECT, JOB, null, EMPTY_RPC_OPTIONS)) .andReturn(responsePb); EasyMock.replay(bigqueryRpcMock); bigquery = options.getService(); From 1b22ee128cc32359fdc6b5b1e7608305056a9775 Mon Sep 17 00:00:00 2001 From: Praful Makani Date: Fri, 11 Jan 2019 21:02:05 +0530 Subject: [PATCH 4/4] removed unused location --- .../src/main/java/com/google/cloud/ServiceOptions.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java index f93c62dcabb8..141c99857da4 100644 --- a/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java +++ b/google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java @@ -118,7 +118,6 @@ public abstract static class Builder< B extends Builder> { private String projectId; - private String location; private String host; protected Credentials credentials; private RetrySettings retrySettings;