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

Fully qualified subscription configuration support + immutable configuration in Pub/Sub #1110

Conversation

elefeint
Copy link
Contributor

@elefeint elefeint commented May 3, 2022

TLDR:

  • immutable maps
  • type-safe and unambiguous ProjectSubscriptionName map keys
  • switched map types to generic Map interface

Details:

  • In the past, settings for subscriptions not explicitly specified by the user were cached in the map as a reference to global settings. Now they are not; instead global subscriber is returned on-demand whenever properties are requested for an heretofore unknown subscription. This caused a couple of tests to change in behavior -- instead of testing for 2 entries in the map, they are now testing for only 1.
  • An instance of PubSubConfiguration effectively can't be used until .initialize() is called, allowing tying settings to fully qualified subscription names, using the passed-in project ID as a default.
  • PubSubConfiguration no longer allows retrieval of user-provided properties in the subscription map. Instead, an immutable map containing equivalent fully-qualified subscription keys can be retrieved with .getFullyQualifiedSubscriberProperties(). Both Spring bean wiring and our unit tests now use the setSubscription() setter to inject "user provided" properties.
  • NEW FEATURE: users can now configure the fully-qualified subscription name with spring.cloud.gcp.pubsub.subscription.fake-short-name.fully-qualified-name property.

What did not change?

  • I did not update many of the compute* methods to accept a fully qualified ProjectSubscriptionName because they are called from a place where only a string subscription name (either short or fully qualified) is available.
  • Almost no new tests; the old tests covered all cases.

Fixes #1005.

@elefeint elefeint requested a review from mpeddada1 May 9, 2022 19:08
@elefeint elefeint marked this pull request as ready for review May 9, 2022 19:08
@elefeint elefeint changed the title WIP: fully qualified subscription configuration support in Pub/Sub Fully qualified subscription configuration support + immutable configuration in Pub/Sub May 9, 2022
Copy link
Contributor

@mpeddada1 mpeddada1 left a comment

Choose a reason for hiding this comment

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

Looks great! This is a really nice fix.

@sonarcloud
Copy link

sonarcloud bot commented May 19, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

89.7% 89.7% Coverage
0.0% 0.0% Duplication

@elefeint elefeint requested a review from mpeddada1 May 20, 2022 13:58
Copy link
Contributor

@mpeddada1 mpeddada1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@elefeint elefeint merged commit 292a116 into main May 24, 2022
@elefeint elefeint deleted the pubsub-per-subscription-maps-store-config-in-pubsubconfig-like-before branch May 24, 2022 02:06
kateryna216 added a commit to kateryna216/spring-cloud-gcp that referenced this pull request Oct 20, 2022
…uration in Pub/Sub (GoogleCloudPlatform#1110)

* immutable maps
* type-safe and unambiguous ProjectSubscriptionName map keys
* switched map types to generic Map interface
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong mechanism for creating subscription specific FlowControlSettings in GcpPubSubAutoConfiguration
2 participants