feat: Add Universe Domain to Java-Core by lqiu96 · Pull Request #2329 · googleapis/sdk-platform-java

lqiu96 · 2024-01-02T18:28:56Z

Google Auth Library v1.21.0 has been released.

ServiceOptions exposes Getters and Setters for Universe Domain. There is a helper method to resolve the endpoint and to validate the universe domain.

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

blakeli0 · 2024-01-02T22:08:38Z

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

  }
+
+  /** Returns the resolved endpoint for the Service to connect to Google Cloud */
+  public String getResolvedEndpoint(String serviceName) {


I think host in java-core is basically endpoint in Gax without the port, based on the value of DEFAULT_HOST. So we should either rename this to getResolvedHost and remove the port concatenation in formatEndpoint, or return host:port if host is specified.
Can you please verify the format of host?

I think the format of host is intended to be scheme://host:port. For gRPC, the channel is expecting something in format of domain:port and httpjson is open to anything valid.

I see that the handwritten libraries handle these inside their own codebase:

Storage gRPC (strips the scheme and adds a port if missing): https://github.com/googleapis/java-storage/blob/2b5b27eac3279db815b36b252830d0905ade0665/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java#L163-L170

Storage HttpJson (sets it directly): https://github.com/googleapis/java-storage/blob/2b5b27eac3279db815b36b252830d0905ade0665/google-cloud-storage/src/main/java/com/google/cloud/storage/spi/v1/HttpStorageRpc.java#L143

Spanner (enforces that a port exists): https://github.com/googleapis/java-spanner/blob/21f5eba7d46c9011803accf0aaf87e6072066419/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java#L1469-L1471

Bigquery (sets it directly): https://github.com/googleapis/java-bigquery/blob/7a1bbd2d33e71644d83c42b71c67bac099882a4a/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/spi/v2/HttpBigQueryRpc.java#L108

I think it should be that the gRPC clients use getResolvedEndpoint() and expect something in the format host:port. Apiary clients (httpjson) will use getResolvedApiaryEndpoint() and that will return the just the host.

…main

gapic-generator-java-pom-parent/pom.xml

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

suztomo · 2024-01-04T03:07:57Z

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

+    if (universeDomain != null && universeDomain.isEmpty()) {
+      throw new IllegalArgumentException("Universe Domain cannot be empty");


I felt it's wrong a method that "Returns the resolved endpoint for the Service to connect to Google Cloud" works only when universeDomain is set.

This is a required check. If universedomain is set as "", we must error out. If universeDomain is null, the resolvedUniverseDomain will be googleapis.com. Otherwise, the resolvedUniverseDomain will be whatever is inputted.

I misunderstood the meaning of the code. Added suggested edits.

Then how about setting the requirement in setUniverseDomain?

I think that makes sense. Guidance for temporary Apiary is to use whatever is passed in as the universe domain. The empty check is for handwritten and gapics. I'll ask about the empty string check, but it shouldn't be blocking for this.

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

suztomo · 2024-01-04T16:30:22Z

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

+
+  /**
+   * Temporarily used for BigQuery and Storage Apiary Wrapped Libraries. To be removed in the
+   * future. Returns the host to be used as the rootUrl.


I believe rootUrl is not a generic term but specific to our code. Can you add a link to rootUrl?

Sounds good, will add.

suztomo · 2024-01-04T16:45:06Z

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

+  /**
+   * Returns the resolved host for the Service to connect to Google Cloud
+   *
+   * <p>The resolved host will be in `https://www.{serviceName}.{resolvedUniverseDomain}/` format.


Would you add a reference about adding "www." (Javadoc's @see annotation)? I checked www.speech.googleapis.com but it (the DNS entry) does not exist.

suztomo@suztomo2:~$ ping www.speech.googleapis.com ping: www.speech.googleapis.com: Name or service not known

Added a link to DEFAULT_HOST

This works for cloudkms.googleapis.com not sure why there is an issue for speech.googleapis.com

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

blakeli0

LGTM. Please resolve outstanding comments before merging.

sonarqubecloud · 2024-01-04T19:14:48Z

Quality Gate passed for 'gapic-generator-java-root'

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
96.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

sonarqubecloud · 2024-01-04T19:18:46Z

Quality Gate failed for 'java_showcase_integration_tests'

Failed conditions

0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

suztomo · 2024-01-04T19:30:24Z

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

+   *     href="https://github.com/googleapis/sdk-platform-java/blob/097964f24fa1989bc74b4807a253f0be4e9dd1ea/java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java#L85">DEFAULT_HOST</a>
+   */
+  @InternalApi
+  public String getResolvedHost(String serviceName) {


Calling it as host is itchy. Fine but ensure to explain this to library owners.
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/URL.html#getHost()

* feat: Add Java-Core Universe Domain changes * chore: Move validate universe domain logic to ServiceOptions * chore: Add javadocs * chore: Add tests * chore: Fix lint issues * chore: Add project id to tests * chore: Fix format issues * chore: Address PR comments * chore: Update Apiary to return rootHostUrl * chore: Use Google Auth Library v1.21.0 * chore: Add tests for normalizeEndpoint() * chore: Address PR comments * chore: Address PR comments * chore: Fix comments * chore: Address PR comments * chore: Address PR comments * chore: Add links * chore: Add format to match DEFAULT_HOST * chore: Fix failing tests * chore: Update javadocs * chore: Remove www. prefix

lqiu96 added 4 commits January 2, 2024 12:25

feat: Add Java-Core Universe Domain changes

0397cc3

chore: Move validate universe domain logic to ServiceOptions

9ba6313

chore: Add javadocs

b0e7614

chore: Add tests

6cc86ac

product-auto-label bot added the size: m Pull request size is medium. label Jan 2, 2024

lqiu96 added 3 commits January 2, 2024 13:30

chore: Fix lint issues

c68e996

chore: Add project id to tests

2c23732

chore: Fix format issues

db4965a

blakeli0 reviewed Jan 2, 2024

View reviewed changes

lqiu96 added 4 commits January 3, 2024 13:00

chore: Address PR comments

3d63297

chore: Update Apiary to return rootHostUrl

5f8d48f

Merge remote-tracking branch 'origin/main' into java-core-universe_do…

881b5c1

…main

chore: Use Google Auth Library v1.21.0

8fcf832

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Jan 3, 2024

Merge branch 'main' into java-core-universe_domain

08bbe56

lqiu96 marked this pull request as ready for review January 3, 2024 22:31

lqiu96 requested a review from a team January 3, 2024 22:31

lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 3, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 3, 2024

lqiu96 commented Jan 3, 2024

View reviewed changes

gapic-generator-java-pom-parent/pom.xml Show resolved Hide resolved

chore: Add tests for normalizeEndpoint()

8e878f9

blakeli0 reviewed Jan 3, 2024

View reviewed changes

chore: Address PR comments

acb94a8

product-auto-label bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels Jan 4, 2024

suztomo reviewed Jan 4, 2024

View reviewed changes

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated Show resolved Hide resolved

suztomo reviewed Jan 4, 2024

View reviewed changes

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated Show resolved Hide resolved

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated Show resolved Hide resolved

suztomo reviewed Jan 4, 2024

View reviewed changes

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated Show resolved Hide resolved

suztomo reviewed Jan 4, 2024

View reviewed changes

chore: Fix comments

6709f2c

suztomo reviewed Jan 4, 2024

View reviewed changes

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated Show resolved Hide resolved

suztomo reviewed Jan 4, 2024

View reviewed changes

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated Show resolved Hide resolved

suztomo reviewed Jan 4, 2024

View reviewed changes

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated Show resolved Hide resolved

suztomo reviewed Jan 4, 2024

View reviewed changes

blakeli0 reviewed Jan 4, 2024

View reviewed changes

java-core/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated Show resolved Hide resolved

blakeli0 approved these changes Jan 4, 2024

View reviewed changes

chore: Address PR comments

ac02baf

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Jan 4, 2024

lqiu96 added 5 commits January 4, 2024 12:27

chore: Address PR comments

793211c

Merge branch 'main' into java-core-universe_domain

5a2e6db

chore: Add links

876b864

chore: Add format to match DEFAULT_HOST

a6f3234

chore: Fix failing tests

73db273

lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 4, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 4, 2024

lqiu96 added 2 commits January 4, 2024 13:29

chore: Update javadocs

60c096b

chore: Remove www. prefix

f63e8cd

lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 4, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 4, 2024

suztomo approved these changes Jan 4, 2024

View reviewed changes

lqiu96 merged commit 586ac9f into main Jan 4, 2024

lqiu96 deleted the java-core-universe_domain branch January 4, 2024 19:59

release-please bot mentioned this pull request Jan 4, 2024

chore(main): release 2.31.0 #2290

Merged

mpeddada1 mentioned this pull request Sep 16, 2024

feat: add properties to set universe domain and endpoint in bigquery GoogleCloudPlatform/spring-cloud-gcp#3158

Merged

		if (universeDomain != null && universeDomain.isEmpty()) {
		throw new IllegalArgumentException("Universe Domain cannot be empty");

Conversation

lqiu96 commented Jan 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lqiu96 Jan 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lqiu96 Jan 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

suztomo Jan 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

blakeli0 left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 4, 2024

Quality Gate passed for 'gapic-generator-java-root'

Uh oh!

sonarqubecloud bot commented Jan 4, 2024

Quality Gate failed for 'java_showcase_integration_tests'

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lqiu96 commented Jan 2, 2024 •

edited

Loading

lqiu96 Jan 3, 2024 •

edited

Loading

lqiu96 Jan 4, 2024 •

edited

Loading

suztomo Jan 4, 2024 •

edited

Loading