Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add TopicName #113

Merged
merged 11 commits into from Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
@@ -1 +1,7 @@
Fixes #<issue_number_goes_here> (it's a good idea to open an issue first for context and/or discussion)
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/java-pubsub/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [ ] Code coverage does not decrease (if any source code was changed)
- [ ] Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️
Expand Up @@ -59,6 +59,7 @@
import com.google.pubsub.v1.StreamingPullRequest;
import com.google.pubsub.v1.StreamingPullResponse;
import com.google.pubsub.v1.Subscription;
import com.google.pubsub.v1.TopicName;
import com.google.pubsub.v1.UpdateSnapshotRequest;
import com.google.pubsub.v1.UpdateSubscriptionRequest;
import java.io.IOException;
Expand All @@ -81,7 +82,7 @@
* <code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* ProjectSubscriptionName name = ProjectSubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]");
* ProjectTopicName topic = ProjectTopicName.of("[PROJECT]", "[TOPIC]");
* TopicName topic = TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]");
* PushConfig pushConfig = PushConfig.newBuilder().build();
* int ackDeadlineSeconds = 0;
* Subscription response = subscriptionAdminClient.createSubscription(name, topic, pushConfig, ackDeadlineSeconds);
Expand Down Expand Up @@ -211,7 +212,7 @@ public SubscriberStub getStub() {
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* ProjectSubscriptionName name = ProjectSubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]");
* ProjectTopicName topic = ProjectTopicName.of("[PROJECT]", "[TOPIC]");
* TopicName topic = TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]");
* PushConfig pushConfig = PushConfig.newBuilder().build();
* int ackDeadlineSeconds = 0;
* Subscription response = subscriptionAdminClient.createSubscription(name, topic, pushConfig, ackDeadlineSeconds);
Expand Down Expand Up @@ -248,7 +249,7 @@ public SubscriberStub getStub() {
*/
public final Subscription createSubscription(
ProjectSubscriptionName name,
ProjectTopicName topic,
TopicName topic,
PushConfig pushConfig,
int ackDeadlineSeconds) {
Subscription request =
Expand Down Expand Up @@ -279,7 +280,7 @@ public final Subscription createSubscription(
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* ProjectSubscriptionName name = ProjectSubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]");
* ProjectTopicName topic = ProjectTopicName.of("[PROJECT]", "[TOPIC]");
* TopicName topic = TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]");
* PushConfig pushConfig = PushConfig.newBuilder().build();
* int ackDeadlineSeconds = 0;
* Subscription response = subscriptionAdminClient.createSubscription(name.toString(), topic.toString(), pushConfig, ackDeadlineSeconds);
Expand Down Expand Up @@ -344,7 +345,7 @@ public final Subscription createSubscription(
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* ProjectSubscriptionName name = ProjectSubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]");
* TopicName topic = ProjectTopicName.of("[PROJECT]", "[TOPIC]");
* TopicName topic = TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]");
yihanzhen marked this conversation as resolved.
Show resolved Hide resolved
* Subscription request = Subscription.newBuilder()
* .setName(name.toString())
* .setTopic(topic.toString())
Expand All @@ -360,6 +361,21 @@ public final Subscription createSubscription(Subscription request) {
return createSubscriptionCallable().call(request);
}

public final Subscription createSubscription(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation? For this and any of the other public methods.

Copy link
Contributor Author

@yihanzhen yihanzhen Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this method is only added back for binary backward-compatibility. The functionality of this method is already covered by createSubscription(TopicName topic), as the old ProjectTopicName is the subclass of the new TopicName.

We are trying to remove the per-pattern resource names (ProjectTopicName in this case) across all cloud APIs because as we scale it's harder and harder for the generator to come up with meaningful class names. Because PubSub doesn't want a major version bump so we injected code back to the generated clients as a postprocessing step.

So far for such cases we've been only adding essentials (code) back but leaving examples and documentation for all APIs (example). It will be doable to inject the documentation and comments back but will be adding lots of pain for future maintenance. Since I think it'll be PubSub team to continue to maintain the library I want to confirm that you really want me to add it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should at least have a deprecated notice on this method, indicating the preference for the other createSubscription method and that this one will be removed in the next major version release. I think we do still have to document the public interface as whether or not it is being written manually or not, we need to ensure things are properly documented for our customers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Both deprecation information and documentation is added.

ProjectSubscriptionName name,
ProjectTopicName topic,
PushConfig pushConfig,
int ackDeadlineSeconds) {
Subscription request =
Subscription.newBuilder()
.setName(name == null ? null : name.toString())
.setTopic(topic == null ? null : topic.toString())
.setPushConfig(pushConfig)
.setAckDeadlineSeconds(ackDeadlineSeconds)
.build();
return createSubscription(request);
}

// AUTO-GENERATED DOCUMENTATION AND METHOD
/**
* Creates a subscription to a given topic. See the &lt;a
Expand All @@ -378,7 +394,7 @@ public final Subscription createSubscription(Subscription request) {
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* ProjectSubscriptionName name = ProjectSubscriptionName.of("[PROJECT]", "[SUBSCRIPTION]");
* TopicName topic = ProjectTopicName.of("[PROJECT]", "[TOPIC]");
* TopicName topic = TopicName.ofProjectTopicName("[PROJECT]", "[TOPIC]");
* Subscription request = Subscription.newBuilder()
* .setName(name.toString())
* .setTopic(topic.toString())
Expand Down Expand Up @@ -1924,39 +1940,10 @@ public final UnaryCallable<SeekRequest, SeekResponse> seekCallable() {
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* Policy policy = Policy.newBuilder().build();
* Policy response = subscriptionAdminClient.setIamPolicy(formattedResource, policy);
* }
* </code></pre>
*
* @param resource REQUIRED: The resource for which the policy is being specified. See the
* operation documentation for the appropriate value for this field.
* @param policy REQUIRED: The complete policy to be applied to the `resource`. The size of the
* policy is limited to a few 10s of KB. An empty policy is a valid policy but certain Cloud
* Platform services (such as Projects) might reject them.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Policy setIamPolicy(String resource, Policy policy) {
SetIamPolicyRequest request =
SetIamPolicyRequest.newBuilder().setResource(resource).setPolicy(policy).build();
return setIamPolicy(request);
}

// AUTO-GENERATED DOCUMENTATION AND METHOD
/**
* Sets the access control policy on the specified resource. Replaces any existing policy.
*
* <p>Can return Public Errors: NOT_FOUND, INVALID_ARGUMENT and PERMISSION_DENIED
*
* <p>Sample code:
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* ResourceName resource = ProjectName.of("[PROJECT]");
* Policy policy = Policy.newBuilder().build();
* SetIamPolicyRequest request = SetIamPolicyRequest.newBuilder()
* .setResource(formattedResource)
* .setResource(resource.toString())
* .setPolicy(policy)
* .build();
* Policy response = subscriptionAdminClient.setIamPolicy(request);
Expand All @@ -1970,6 +1957,12 @@ public final Policy setIamPolicy(SetIamPolicyRequest request) {
return setIamPolicyCallable().call(request);
}

public final Policy setIamPolicy(String resource, Policy policy) {
SetIamPolicyRequest request =
SetIamPolicyRequest.newBuilder().setResource(resource).setPolicy(policy).build();
return setIamPolicy(request);
}

// AUTO-GENERATED DOCUMENTATION AND METHOD
/**
* Sets the access control policy on the specified resource. Replaces any existing policy.
Expand All @@ -1980,10 +1973,10 @@ public final Policy setIamPolicy(SetIamPolicyRequest request) {
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* ResourceName resource = ProjectName.of("[PROJECT]");
* Policy policy = Policy.newBuilder().build();
* SetIamPolicyRequest request = SetIamPolicyRequest.newBuilder()
* .setResource(formattedResource)
* .setResource(resource.toString())
* .setPolicy(policy)
* .build();
* ApiFuture&lt;Policy&gt; future = subscriptionAdminClient.setIamPolicyCallable().futureCall(request);
Expand All @@ -2005,32 +1998,9 @@ public final UnaryCallable<SetIamPolicyRequest, Policy> setIamPolicyCallable() {
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* Policy response = subscriptionAdminClient.getIamPolicy(formattedResource);
* }
* </code></pre>
*
* @param resource REQUIRED: The resource for which the policy is being requested. See the
* operation documentation for the appropriate value for this field.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final Policy getIamPolicy(String resource) {
GetIamPolicyRequest request = GetIamPolicyRequest.newBuilder().setResource(resource).build();
return getIamPolicy(request);
}

// AUTO-GENERATED DOCUMENTATION AND METHOD
/**
* Gets the access control policy for a resource. Returns an empty policy if the resource exists
* and does not have a policy set.
*
* <p>Sample code:
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* ResourceName resource = ProjectName.of("[PROJECT]");
* GetIamPolicyRequest request = GetIamPolicyRequest.newBuilder()
* .setResource(formattedResource)
* .setResource(resource.toString())
* .build();
* Policy response = subscriptionAdminClient.getIamPolicy(request);
* }
Expand All @@ -2043,6 +2013,11 @@ public final Policy getIamPolicy(GetIamPolicyRequest request) {
return getIamPolicyCallable().call(request);
}

public final Policy getIamPolicy(String resource) {
GetIamPolicyRequest request = GetIamPolicyRequest.newBuilder().setResource(resource).build();
return getIamPolicy(request);
}

// AUTO-GENERATED DOCUMENTATION AND METHOD
/**
* Gets the access control policy for a resource. Returns an empty policy if the resource exists
Expand All @@ -2052,9 +2027,9 @@ public final Policy getIamPolicy(GetIamPolicyRequest request) {
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* ResourceName resource = ProjectName.of("[PROJECT]");
* GetIamPolicyRequest request = GetIamPolicyRequest.newBuilder()
* .setResource(formattedResource)
* .setResource(resource.toString())
* .build();
* ApiFuture&lt;Policy&gt; future = subscriptionAdminClient.getIamPolicyCallable().futureCall(request);
* // Do something
Expand All @@ -2079,19 +2054,23 @@ public final UnaryCallable<GetIamPolicyRequest, Policy> getIamPolicyCallable() {
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* ResourceName resource = ProjectName.of("[PROJECT]");
* List&lt;String&gt; permissions = new ArrayList&lt;&gt;();
* TestIamPermissionsResponse response = subscriptionAdminClient.testIamPermissions(formattedResource, permissions);
* TestIamPermissionsRequest request = TestIamPermissionsRequest.newBuilder()
* .setResource(resource.toString())
* .addAllPermissions(permissions)
* .build();
* TestIamPermissionsResponse response = subscriptionAdminClient.testIamPermissions(request);
* }
* </code></pre>
*
* @param resource REQUIRED: The resource for which the policy detail is being requested. See the
* operation documentation for the appropriate value for this field.
* @param permissions The set of permissions to check for the `resource`. Permissions with
* wildcards (such as '&#42;' or 'storage.&#42;') are not allowed. For more information see
* [IAM Overview](https://cloud.google.com/iam/docs/overview#permissions).
* @param request The request object containing all of the parameters for the API call.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final TestIamPermissionsResponse testIamPermissions(TestIamPermissionsRequest request) {
return testIamPermissionsCallable().call(request);
}

public final TestIamPermissionsResponse testIamPermissions(
String resource, List<String> permissions) {
TestIamPermissionsRequest request =
Expand All @@ -2115,40 +2094,10 @@ public final TestIamPermissionsResponse testIamPermissions(
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* List&lt;String&gt; permissions = new ArrayList&lt;&gt;();
* TestIamPermissionsRequest request = TestIamPermissionsRequest.newBuilder()
* .setResource(formattedResource)
* .addAllPermissions(permissions)
* .build();
* TestIamPermissionsResponse response = subscriptionAdminClient.testIamPermissions(request);
* }
* </code></pre>
*
* @param request The request object containing all of the parameters for the API call.
* @throws com.google.api.gax.rpc.ApiException if the remote call fails
*/
public final TestIamPermissionsResponse testIamPermissions(TestIamPermissionsRequest request) {
return testIamPermissionsCallable().call(request);
}

// AUTO-GENERATED DOCUMENTATION AND METHOD
/**
* Returns permissions that a caller has on the specified resource. If the resource does not
* exist, this will return an empty set of permissions, not a NOT_FOUND error.
*
* <p>Note: This operation is designed to be used for building permission-aware UIs and
* command-line tools, not for authorization checking. This operation may "fail open" without
* warning.
*
* <p>Sample code:
*
* <pre><code>
* try (SubscriptionAdminClient subscriptionAdminClient = SubscriptionAdminClient.create()) {
* String formattedResource = ProjectSubscriptionName.format("[PROJECT]", "[SUBSCRIPTION]");
* ResourceName resource = ProjectName.of("[PROJECT]");
* List&lt;String&gt; permissions = new ArrayList&lt;&gt;();
* TestIamPermissionsRequest request = TestIamPermissionsRequest.newBuilder()
* .setResource(formattedResource)
* .setResource(resource.toString())
* .addAllPermissions(permissions)
* .build();
* ApiFuture&lt;TestIamPermissionsResponse&gt; future = subscriptionAdminClient.testIamPermissionsCallable().futureCall(request);
Expand Down