Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

PubsubTemplate subscribe in other projects #1880

Merged
merged 4 commits into from
Sep 12, 2019
Merged

Conversation

meltsufin
Copy link
Contributor

This change to the DefaultSubscriberFactory allows the project id of the
subscription to be overwitten directly from the subscription specification in the
subscription name string.

This allows use-cases like
pubSubTemplate.subscribe("projects/other-project/subscriptions/the-subscription",
messageReceiver).

This change is applied across the board to allow subscribing to,
creating, and deleting subscriptions using fully-qualified subscription names.

Fixes #1877.
Related to #1678.

This change to the DefaultSubscriberFactory allows the project id of the
subscription to be overwitten directly from the subscription specification in the
subscription name string.

This allows use-cases like
pubSubTemplate.subscribe("projects/other-project/subscriptions/the-subscription",
messageReceiver).

This change is applied across the board to allow  subscribing to,
creating, and deleting subscriptions using fully-qualified subscription names.

Fixes #1877.
Related to #1678.
@meltsufin meltsufin added the pubsub GCP PubSub label Sep 12, 2019
@codecov
Copy link

codecov bot commented Sep 12, 2019

Codecov Report

Merging #1880 into master will increase coverage by 1.36%.
The diff coverage is 77.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1880      +/-   ##
============================================
+ Coverage      70.2%   71.57%   +1.36%     
- Complexity     1780     1817      +37     
============================================
  Files           236      236              
  Lines          6794     6706      -88     
  Branches        695      689       -6     
============================================
+ Hits           4770     4800      +30     
+ Misses         1705     1589     -116     
+ Partials        319      317       -2
Flag Coverage Δ Complexity Δ
#unittests 71.57% <77.77%> (+1.36%) 1817 <2> (+37) ⬆️
Impacted Files Coverage Δ Complexity Δ
...ork/cloud/gcp/pubsub/support/PubSubTopicUtils.java 100% <ø> (ø) 2 <0> (ø) ⬇️
.../springframework/cloud/gcp/pubsub/PubSubAdmin.java 5.06% <0%> (ø) 1 <0> (ø) ⬇️
...ud/gcp/pubsub/support/PubSubSubscriptionUtils.java 100% <100%> (ø) 2 <2> (?)
...bsub/core/subscriber/PubSubSubscriberTemplate.java 76.92% <100%> (+0.29%) 23 <0> (ø) ⬇️
...d/gcp/pubsub/support/DefaultSubscriberFactory.java 15.06% <50%> (ø) 3 <0> (ø) ⬇️
...undry/GcpCloudFoundryEnvironmentPostProcessor.java
...cp/data/firestore/util/ObservableReactiveUtil.java 44.68% <0%> (+44.68%) 3% <0%> (+3%) ⬆️
...oud/gcp/data/firestore/FirestoreDataException.java 50% <0%> (+50%) 1% <0%> (+1%) ⬆️
...restore/mapping/FirestorePersistentEntityImpl.java 54.54% <0%> (+54.54%) 3% <0%> (+3%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed38643...9143429. Read the comment docs.

Copy link
Contributor

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Just a couple nit-picks.
Everything else LGTM

* @param subscriptionName the name of the new subscription
* @param topicName the name of the topic being subscribed to
* @param subscriptionName canonical subscription name, e.g., "subscriptionName", or the fully-qualified
* subscription name in the "projects/&lt;project_name&gt;/subscriptions/&lt;subscription_name&gt;" format
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we use {@code} instead?

}

/**
* Creates a {@link ProjectSubscriptionName} based on a subscription name within a project or the
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -48,7 +48,8 @@
*
* @deprecated as of 1.1, use {@link #subscribe(String, Consumer)} instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

The @deprecated tag must be last one:

<module name="AtclauseOrder"> 
	<property name="target" value="METHOD_DEF, CTOR_DEF, VARIABLE_DEF"/> 
   	<property name="tagOrder" value="@param, @return, @throws, @since, @deprecated, @see"/> 
</module> 

I'm not sure hot this passes a Checkstyle...

Copy link
Contributor

Choose a reason for hiding this comment

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

How about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, missed it

@@ -98,7 +98,7 @@ public PubSubAdmin(GcpProjectIdProvider projectIdProvider, TopicAdminClient topi
* Create a new topic on Google Cloud Pub/Sub.
*
* @param topicName the name for the new topic within the current project, or the
* fully-qualified topic name in the projects/&lt;project_name&gt;/topics/&lt;topic_name&gt; format
* fully-qualified topic name in the {@code "projects/<project_name>/topics/<topic_name>"} format
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to that wrapped into double quotes?

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.

@@ -48,7 +48,8 @@
*
* @deprecated as of 1.1, use {@link #subscribe(String, Consumer)} instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about this?

@meltsufin meltsufin merged commit 703c8a0 into master Sep 12, 2019
@meltsufin meltsufin deleted the project-subscription branch September 12, 2019 20:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pubsub GCP PubSub
Development

Successfully merging this pull request may close these issues.

Support fully-qualified subscription names in PubSubTemplate
2 participants