fix(pubsub): Fix project option in Project#topic and Project#subscrip…

…tion

* Ensure that project option is used when skip_lookup is false.
* Improve documentation of topic_name, subscription_name and snapshot_name.

closes: googleapis#9302
pr: googleapis#9303

google-cloud-pubsub/lib/google/cloud/pubsub/async_publisher.rb

@@ -44,8 +44,8 @@ module PubSub
       #
       #   topic.async_publisher.stop!
       #
-      # @attr_reader [String] topic_name The name of the topic the messages are published to. In the form of
-      #   "/projects/project-identifier/topics/topic-name".
+      # @attr_reader [String] topic_name The name of the topic the messages are published to. The value is a
+      #   fully-qualified topic name in the form `projects/{project_id}/topics/{topic_id}`.
       # @attr_reader [Integer] max_bytes The maximum size of messages to be collected before the batch is published.
       #   Default is 1,000,000 (1MB).
       # @attr_reader [Integer] max_messages The maximum number of messages to be collected before the batch is

google-cloud-pubsub/lib/google/cloud/pubsub/project.rb

@@ -75,10 +75,14 @@ def project_id
         ##
         # Retrieves topic by name.
         #
-        # @param [String] topic_name Name of a topic.
+        # @param [String] topic_name Name of a topic. The value can be a simple
+        #   topic ID (relative name), in which case the current project ID will
+        #   be supplied, or a fully-qualified topic name in the form
+        #   `projects/{project_id}/topics/{topic_id}`.
         # @param [String] project If the topic belongs to a project other than
         #   the one currently connected to, the alternate project ID can be
-        #   specified here. Optional.
+        #   specified here. Optional. Not used if a fully-qualified topic name
+        #   is provided for `topic_name`.
         # @param [Boolean] skip_lookup Optionally create a {Topic} object
         #   without verifying the topic resource exists on the Pub/Sub service.
         #   Calls made on this object will raise errors if the topic resource
@@ -147,7 +151,7 @@ def topic topic_name, project: nil, skip_lookup: nil, async: nil
           ensure_service!
           options = { project: project }
           return Topic.from_name topic_name, service, options if skip_lookup
-          grpc = service.get_topic topic_name
+          grpc = service.get_topic topic_name, options
           Topic.from_grpc grpc, service, async: async
         rescue Google::Cloud::NotFoundError
           nil
@@ -158,7 +162,16 @@ def topic topic_name, project: nil, skip_lookup: nil, async: nil
         ##
         # Creates a new topic.
         #
-        # @param [String] topic_name Name of a topic.
+        # @param [String] topic_name Name of a topic. Required.
+        #   The value can be a simple topic ID (relative name), in which
+        #   case the current project ID will be supplied, or a fully-qualified
+        #   topic name in the form `projects/{project_id}/topics/{topic_id}`.
+        #
+        #   The topic ID (relative name) must start with a letter, and
+        #   contain only letters (`[A-Za-z]`), numbers (`[0-9]`), dashes (`-`),
+        #   underscores (`_`), periods (`.`), tildes (`~`), plus (`+`) or percent
+        #   signs (`%`). It must be between 3 and 255 characters in length, and
+        #   it must not start with `goog`.
         # @param [Hash] labels A hash of user-provided labels associated with
         #   the topic. You can use these to organize and group your topics.
         #   Label keys and values can be no longer than 63 characters, can only
@@ -250,10 +263,14 @@ def topics token: nil, max: nil
         ##
         # Retrieves subscription by name.
         #
-        # @param [String] subscription_name Name of a subscription.
+        # @param [String] subscription_name Name of a subscription. The value can
+        #   be a simple subscription ID, in which case the current project ID
+        #   will be supplied, or a fully-qualified subscription name in the form
+        #   `projects/{project_id}/subscriptions/{subscription_id}`.
         # @param [String] project If the subscription belongs to a project other
         #   than the one currently connected to, the alternate project ID can be
-        #   specified here.
+        #   specified here. Not used if a fully-qualified subscription name is
+        #   provided for `subscription_name`.
         # @param [Boolean] skip_lookup Optionally create a {Subscription} object
         #   without verifying the subscription resource exists on the Pub/Sub
         #   service. Calls made on this object will raise errors if the service
@@ -283,7 +300,7 @@ def subscription subscription_name, project: nil, skip_lookup: nil
           ensure_service!
           options = { project: project }
           return Subscription.from_name subscription_name, service, options if skip_lookup
-          grpc = service.get_subscription subscription_name
+          grpc = service.get_subscription subscription_name, options
           Subscription.from_grpc grpc, service
         rescue Google::Cloud::NotFoundError
           nil

google-cloud-pubsub/lib/google/cloud/pubsub/snapshot.rb

@@ -58,8 +58,10 @@ def initialize
         end
 
         ##
-        # The name of the snapshot. Format is
-        # `projects/{project}/snapshots/{snap}`.
+        # The name of the snapshot.
+        #
+        # @return [String] A fully-qualified snapshot name in the form
+        #   `projects/{project_id}/snapshots/{snapshot_id}`.
         def name
           @grpc.name
         end

google-cloud-pubsub/lib/google/cloud/pubsub/subscription.rb

@@ -77,7 +77,9 @@ def initialize
         ##
         # The name of the subscription.
         #
-        # @return [String]
+        # @return [String] A fully-qualified subscription name in the form
+        #   `projects/{project_id}/subscriptions/{subscription_id}`.
+        #
         def name
           return @resource_name if reference?
           @grpc.name
@@ -1064,14 +1066,19 @@ def modify_ack_deadline new_deadline, *messages
         # * Any messages published to the subscription's topic following the
         #   successful completion of the `create_snapshot` operation.
         #
-        # @param [String, nil] snapshot_name Name of the new snapshot. If the
-        #   name is not provided, the server will assign a random name
-        #   for this snapshot on the same project as the subscription. The
-        #   format is `projects/{project}/snapshots/{snap}`. The name must start
-        #   with a letter, and contain only letters ([A-Za-z]), numbers
-        #   ([0-9], dashes (-), underscores (_), periods (.), tildes (~), plus
-        #   (+) or percent signs (%). It must be between 3 and 255 characters in
-        #   length, and it must not start with "goog". Optional.
+        # @param [String, nil] snapshot_name Name of the new snapshot. Optional.
+        #   If the name is not provided, the server will assign a random name
+        #   for this snapshot on the same project as the subscription.
+        #   The value can be a simple snapshot ID (relative name), in which
+        #   case the current project ID will be supplied, or a fully-qualified
+        #   snapshot name in the form
+        #   `projects/{project_id}/snapshots/{snapshot_id}`.
+        #
+        #   The snapshot ID (relative name) must start with a letter, and
+        #   contain only letters (`[A-Za-z]`), numbers (`[0-9]`), dashes (`-`),
+        #   underscores (`_`), periods (`.`), tildes (`~`), plus (`+`) or percent
+        #   signs (`%`). It must be between 3 and 255 characters in length, and
+        #   it must not start with `goog`.
         # @param [Hash] labels A hash of user-provided labels associated with
         #   the snapshot. You can use these to organize and group your
         #   snapshots. Label keys and values can be no longer than 63

google-cloud-pubsub/lib/google/cloud/pubsub/topic.rb

@@ -85,10 +85,10 @@ def async_publisher
         end
 
         ##
-        # The name of the topic in the form of
-        # "/projects/project-identifier/topics/topic-name".
+        # The name of the topic.
         #
-        # @return [String]
+        # @return [String] A fully-qualified topic name in the form
+        #   `projects/{project_id}/topics/{topic_id}`.
         #
         def name
           return @resource_name if reference?
@@ -255,11 +255,17 @@ def delete
         ##
         # Creates a new {Subscription} object on the current Topic.
         #
-        # @param [String] subscription_name Name of the new subscription. Must
-        #   start with a letter, and contain only letters ([A-Za-z]), numbers
-        #   ([0-9], dashes (-), underscores (_), periods (.), tildes (~), plus
-        #   (+) or percent signs (%). It must be between 3 and 255 characters in
-        #   length, and it must not start with "goog". Required.
+        # @param [String] subscription_name Name of the new subscription. Required.
+        #   The value can be a simple subscription ID (relative name), in which
+        #   case the current project ID will be supplied, or a fully-qualified
+        #   subscription name in the form
+        #   `projects/{project_id}/subscriptions/{subscription_id}`.
+        #
+        #   The subscription ID (relative name) must start with a letter, and
+        #   contain only letters (`[A-Za-z]`), numbers (`[0-9]`), dashes (`-`),
+        #   underscores (`_`), periods (`.`), tildes (`~`), plus (`+`) or percent
+        #   signs (`%`). It must be between 3 and 255 characters in length, and
+        #   it must not start with `goog`.
         # @param [Integer] deadline The maximum number of seconds after a
         #   subscriber receives a message before the subscriber should
         #   acknowledge the message.
@@ -415,7 +421,11 @@ def subscribe subscription_name,
         ##
         # Retrieves subscription by name.
         #
-        # @param [String] subscription_name Name of a subscription.
+        # @param [String] subscription_name Name of a subscription. The value
+        #   can be a simple subscription ID (relative name), in which case the
+        #   current project ID will be supplied, or a fully-qualified
+        #   subscription name in the form
+        #   `projects/{project_id}/subscriptions/{subscription_id}`.
         # @param [Boolean] skip_lookup Optionally create a {Subscription} object
         #   without verifying the subscription resource exists on the Pub/Sub
         #   service. Calls made on this object will raise errors if the service

google-cloud-pubsub/test/google/cloud/pubsub/project_test.rb

@@ -78,6 +78,21 @@
     _(topic.persistence_regions).must_be :empty?
   end
 
+  it "creates a topic with fully-qualified topic path" do
+    new_topic_path = "projects/other-project/topics/new-topic-#{Time.now.to_i}"
+
+    create_res = Google::Cloud::PubSub::V1::Topic.new topic_hash(new_topic_path)
+    mock = Minitest::Mock.new
+    mock.expect :create_topic, create_res, [name: new_topic_path, labels: nil, kms_key_name: nil, message_storage_policy: nil]
+    pubsub.service.mocked_publisher = mock
+
+    topic = pubsub.create_topic new_topic_path
+
+    mock.verify
+
+    _(topic.name).must_equal new_topic_path
+  end
+
   it "creates a topic with new_topic_alias" do
     new_topic_name = "new-topic-#{Time.now.to_i}"
 
@@ -171,6 +186,21 @@
     _(topic).must_be :resource?
   end
 
+  it "gets a topic with fully-qualified topic path" do
+    topic_full_path = "projects/other-project/topics/found-topic"
+
+    get_res = Google::Cloud::PubSub::V1::Topic.new topic_hash(topic_full_path)
+    mock = Minitest::Mock.new
+    mock.expect :get_topic, get_res, [topic: topic_path(topic_full_path)]
+    pubsub.service.mocked_publisher = mock
+
+    topic = pubsub.topic topic_full_path
+
+    mock.verify
+
+    _(topic.name).must_equal topic_full_path
+  end
+
   it "gets a topic with get_topic alias" do
     topic_name = "found-topic"
 
@@ -228,6 +258,21 @@ def stub.get_topic *args
     _(topic).wont_be :resource?
   end
 
+  it "gets a topic with project option" do
+    topic_name = "found-topic"
+    topic_full_path = "projects/custom/topics/found-topic"
+
+    get_res = Google::Cloud::PubSub::V1::Topic.new topic_hash(topic_full_path)
+    mock = Minitest::Mock.new
+    mock.expect :get_topic, get_res, [topic: topic_full_path]
+    pubsub.service.mocked_publisher = mock
+
+    topic = pubsub.find_topic topic_name, project: "custom"
+    _(topic.name).must_equal topic_full_path
+    _(topic).wont_be :reference?
+    _(topic).must_be :resource?
+  end
+
   it "gets a topic with skip_lookup and project options" do
     topic_name = "found-topic"
     # No HTTP mock needed, since the lookup is not made
@@ -431,6 +476,21 @@ def stub.get_topic *args
     _(sub).must_be :resource?
   end
 
+  it "gets a subscription with fully-qualified subscription path" do
+    sub_full_path = "projects/other-project/subscriptions/found-sub-#{Time.now.to_i}"
+
+    get_res = Google::Cloud::PubSub::V1::Subscription.new subscription_hash("random-topic", sub_full_path)
+    mock = Minitest::Mock.new
+    mock.expect :get_subscription, get_res, [subscription: sub_full_path]
+    pubsub.service.mocked_subscriber = mock
+
+    sub = pubsub.subscription sub_full_path
+
+    mock.verify
+
+    _(sub.name).must_equal sub_full_path
+  end
+
   it "gets a subscription with get_subscription alias" do
     sub_name = "found-sub-#{Time.now.to_i}"
 
@@ -494,6 +554,23 @@ def stub.get_subscription *args
     _(sub).wont_be :resource?
   end
 
+  it "gets a subscription with project option" do
+    sub_name = "found-sub-#{Time.now.to_i}"
+    sub_full_path = "projects/custom/subscriptions/#{sub_name}"
+
+    get_res = Google::Cloud::PubSub::V1::Subscription.new subscription_hash("random-topic", sub_full_path)
+    mock = Minitest::Mock.new
+    mock.expect :get_subscription, get_res, [subscription: sub_full_path]
+    pubsub.service.mocked_subscriber = mock
+
+    sub = pubsub.subscription sub_name, project: "custom"
+    _(sub).wont_be :nil?
+    _(sub).must_be_kind_of Google::Cloud::PubSub::Subscription
+    _(sub.name).must_equal sub_full_path
+    _(sub).wont_be :reference?
+    _(sub).must_be :resource?
+  end
+
   it "gets a subscription with skip_lookup and project options" do
     sub_name = "found-sub-#{Time.now.to_i}"
     # No HTTP mock needed, since the lookup is not made

google-cloud-pubsub/test/google/cloud/pubsub/subscription_test.rb

@@ -124,6 +124,20 @@
     _(snapshot).must_be_kind_of Google::Cloud::PubSub::Snapshot
   end
 
+  it "creates a snapshot with fully-qualified snapshot path" do
+    new_snapshot_path = "projects/other-project/snapshots/new-snapshot-#{Time.now.to_i}"
+    create_res = Google::Cloud::PubSub::V1::Snapshot.new snapshot_hash(subscription_name, new_snapshot_path)
+    mock = Minitest::Mock.new
+    mock.expect :create_snapshot, create_res, [name: snapshot_path(new_snapshot_path), subscription: subscription_path(subscription_name), labels: nil]
+    subscription.service.mocked_subscriber = mock
+
+    snapshot = subscription.create_snapshot new_snapshot_path
+
+    mock.verify
+
+    _(snapshot.name).must_equal new_snapshot_path
+  end
+
   it "creates a snapshot with new_snapshot alias" do
     new_snapshot_name = "new-snapshot-#{Time.now.to_i}"
     create_res = Google::Cloud::PubSub::V1::Snapshot.new snapshot_hash(subscription_name, new_snapshot_name)

google-cloud-pubsub/test/google/cloud/pubsub/topic_test.rb

@@ -74,6 +74,21 @@
     _(sub).must_be_kind_of Google::Cloud::PubSub::Subscription
   end
 
+  it "creates a subscription with fully-qualified subscription path" do
+    new_sub_path = "projects/other-project/subscriptions/new-sub-#{Time.now.to_i}"
+
+    create_res = Google::Cloud::PubSub::V1::Subscription.new subscription_hash(topic_name, new_sub_path)
+    mock = Minitest::Mock.new
+    mock.expect :create_subscription, create_res, create_subscription_args(new_sub_path, topic_name)
+    topic.service.mocked_subscriber = mock
+
+    sub = topic.subscribe new_sub_path
+
+    mock.verify
+
+    _(sub.name).must_equal new_sub_path
+  end
+
   it "creates a subscription with create_subscription alias" do
     new_sub_name = "new-sub-#{Time.now.to_i}"
     create_res = Google::Cloud::PubSub::V1::Subscription.new subscription_hash(topic_name, new_sub_name)
@@ -313,6 +328,21 @@ def stub.create_subscription *args
     _(sub).must_be :resource?
   end
 
+  it "gets a subscription with fully-qualified subscription path" do
+    sub_full_path = "projects/other-project/subscriptions/found-sub-#{Time.now.to_i}"
+
+    get_res = Google::Cloud::PubSub::V1::Subscription.new subscription_hash(topic_name, sub_full_path)
+    mock = Minitest::Mock.new
+    mock.expect :get_subscription, get_res, [subscription: sub_full_path]
+    topic.service.mocked_subscriber = mock
+
+    sub = topic.subscription sub_full_path
+
+    mock.verify
+
+    _(sub.name).must_equal sub_full_path
+  end
+
   it "gets a subscription with get_subscription alias" do
     sub_name = "found-sub-#{Time.now.to_i}"
 

google-cloud-pubsub/test/helper.rb

@@ -144,7 +144,8 @@ def subscriptions_hash topic_name, num_subs, token = nil
     data
   end
 
-  def subscription_hash topic_name, sub_name,
+  def subscription_hash topic_name,
+                        sub_name,
                         deadline = 60,
                         endpoint = "http://example.com/callback",
                         labels: nil,
@@ -255,14 +256,17 @@ def project_path
   end
 
   def topic_path topic_name
+    return topic_name if topic_name.to_s.include? "/"
     "#{project_path}/topics/#{topic_name}"
   end
 
   def subscription_path subscription_name
+    return subscription_name if subscription_name.to_s.include? "/"
     "#{project_path}/subscriptions/#{subscription_name}"
   end
 
   def snapshot_path snapshot_name
+    return snapshot_name if snapshot_name.to_s.include?("/")
     "#{project_path}/snapshots/#{snapshot_name}"
   end