From fe71ad9a8a00bbb2821998216672b2627d6758e5 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Fri, 6 Dec 2019 13:58:19 +0530 Subject: [PATCH 01/15] add get_instance field_mask - add/updates docs and test --- .../acceptance/spanner/instance_test.rb | 24 ++++++++++++++ .../acceptance/spanner_helper.rb | 4 +-- .../lib/google/cloud/spanner/project.rb | 15 +++++++-- .../lib/google/cloud/spanner/service.rb | 5 +-- .../support/doctest_helper.rb | 32 +++++++++---------- .../google/cloud/spanner/batch_client_test.rb | 2 +- .../google/cloud/spanner/client/admin_test.rb | 2 +- .../cloud/spanner/project/instance_test.rb | 25 ++++++++++++++- 8 files changed, 83 insertions(+), 26 deletions(-) diff --git a/google-cloud-spanner/acceptance/spanner/instance_test.rb b/google-cloud-spanner/acceptance/spanner/instance_test.rb index 4a12f9e8f715..52d931124470 100644 --- a/google-cloud-spanner/acceptance/spanner/instance_test.rb +++ b/google-cloud-spanner/acceptance/spanner/instance_test.rb @@ -15,6 +15,8 @@ require "spanner_helper" describe "Spanner Instances", :spanner do + let(:instance_id) { $spanner_instance_id } + it "lists and gets instances" do all_instances = spanner.instances.all.to_a all_instances.wont_be :empty? @@ -26,6 +28,28 @@ first_instance.must_be_kind_of Google::Cloud::Spanner::Instance end + describe "get instance" do + it "get all instance fields" do + instance = spanner.instance instance_id + + instance.instance_id.must_equal instance_id + instance.path.wont_be_empty + instance.display_name.wont_be_empty + instance.nodes.must_be :>, 0 + instance.state.must_equal :READY + end + + it "get speicified instance fields" do + instance = spanner.instance instance_id, fields: ["name"] + + instance.instance_id.must_equal instance_id + instance.path.wont_be_empty + instance.display_name.must_be_empty + instance.nodes.must_equal 0 + instance.state.must_equal :STATE_UNSPECIFIED + end + end + describe "IAM Policies and Permissions" do let(:service_account) { spanner.service.credentials.client.issuer } diff --git a/google-cloud-spanner/acceptance/spanner_helper.rb b/google-cloud-spanner/acceptance/spanner_helper.rb index 409fa19e926b..7040d38f1a0e 100644 --- a/google-cloud-spanner/acceptance/spanner_helper.rb +++ b/google-cloud-spanner/acceptance/spanner_helper.rb @@ -254,9 +254,9 @@ def default_item_rows fixture = Object.new fixture.extend Acceptance::SpannerTest::Fixtures -instance = $spanner.instance "google-cloud-ruby-tests" +instance = $spanner.instance $spanner_instance_id instance ||= begin - inst_job = $spanner.create_instance "google-cloud-ruby-tests", name: "google-cloud-ruby-tests", config: "regional-us-central1", nodes: 1 + inst_job = $spanner.create_instance $spanner_instance_id, name: "google-cloud-ruby-tests", config: "regional-us-central1", nodes: 1 inst_job.wait_until_done! fail GRPC::BadStatus.new(inst_job.error.code, inst_job.error.message) if inst_job.error? inst_job.instance diff --git a/google-cloud-spanner/lib/google/cloud/spanner/project.rb b/google-cloud-spanner/lib/google/cloud/spanner/project.rb index 229702bac7f4..21a9b77abbf0 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/project.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/project.rb @@ -135,7 +135,9 @@ def instances token: nil, max: nil # Retrieves a Cloud Spanner instance by unique identifier. # # @param [String] instance_id The unique identifier for the instance. - # + # @param [Array] fields Specifies the subset of fields that + # should be returned. dIf fields not provided then all fields will + # returned. Optional. # @return [Google::Cloud::Spanner::Instance, nil] The instance, or `nil` # if the instance does not exist. # @@ -151,9 +153,16 @@ def instances token: nil, max: nil # spanner = Google::Cloud::Spanner.new # instance = spanner.instance "non-existing" # nil # - def instance instance_id + # @example Get instance detail with specified fields. + # require "google/cloud/spanner" + # + # spanner = Google::Cloud::Spanner.new + # instance = spanner.instance "my-instance", fields: ["name"] + # + def instance instance_id, fields: nil ensure_service! - grpc = service.get_instance instance_id + + grpc = service.get_instance instance_id, fields: fields Instance.from_grpc grpc, service rescue Google::Cloud::NotFoundError nil diff --git a/google-cloud-spanner/lib/google/cloud/spanner/service.rb b/google-cloud-spanner/lib/google/cloud/spanner/service.rb index 1f758e370b3d..2015f95152d6 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/service.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/service.rb @@ -120,9 +120,10 @@ def list_instances token: nil, max: nil end end - def get_instance name + def get_instance name, fields: nil + field_mask = Google::Protobuf::FieldMask.new(paths: fields || []) execute do - instances.get_instance instance_path(name) + instances.get_instance instance_path(name), field_mask: field_mask end end diff --git a/google-cloud-spanner/support/doctest_helper.rb b/google-cloud-spanner/support/doctest_helper.rb index 5e02001bb434..2ab70f5a6671 100644 --- a/google-cloud-spanner/support/doctest_helper.rb +++ b/google-cloud-spanner/support/doctest_helper.rb @@ -108,14 +108,14 @@ def mock_spanner mock_client = Minitest::Mock.new mock_instances.expect :create_instance, create_instance_resp(client: mock_client), ["projects/my-project", "my-new-instance", Google::Spanner::Admin::Instance::V1::Instance] mock_client.expect :get_operation, OpenStruct.new(done: true), ["1234567890", {:options=>nil}] - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-new-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-new-instance", Hash] end end doctest.before "Google::Cloud::Spanner::Instance#create_database" do mock_spanner do |mock, mock_instances, mock_databases| mock_client = Minitest::Mock.new - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_client.expect :get_operation, OpenStruct.new(done: true), ["1234567890", {:options=>nil}] mock_databases.expect :create_database, create_database_resp(client: mock_client), ["projects/my-project/instances/my-instance", "CREATE DATABASE `my-new-database`", {:extra_statements=>[]}] end @@ -123,21 +123,21 @@ def mock_spanner doctest.before "Google::Cloud::Spanner::Instance#database" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_databases.expect :get_database, database_resp, ["projects/my-project/instances/my-instance/databases/my-database"] end end doctest.before "Google::Cloud::Spanner::Instance#database@Will return `nil` if instance does not exist." do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_databases.expect :get_database, nil, ["projects/my-project/instances/my-instance/databases/my-database"] end end doctest.before "Google::Cloud::Spanner::Instance#databases" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_databases.expect :list_databases, databases_resp(token: "token"), ["projects/my-project/instances/my-instance", Hash] mock_databases.expect :list_databases, databases_resp, ["projects/my-project/instances/my-instance", Hash] end @@ -145,7 +145,7 @@ def mock_spanner doctest.before "Google::Cloud::Spanner::Instance#policy" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_instances.expect :get_iam_policy, policy_resp, ["projects/my-project/instances/my-instance"] mock_instances.expect :set_iam_policy, policy_resp, ["projects/my-project/instances/my-instance", Google::Iam::V1::Policy] end @@ -153,7 +153,7 @@ def mock_spanner doctest.before "Google::Cloud::Spanner::Instance#update_policy" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_instances.expect :get_iam_policy, policy_resp, ["projects/my-project/instances/my-instance"] mock_instances.expect :set_iam_policy, policy_resp, ["projects/my-project/instances/my-instance", Google::Iam::V1::Policy] end @@ -161,14 +161,14 @@ def mock_spanner doctest.before "Google::Cloud::Spanner::Instance#test_permissions" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_instances.expect :test_iam_permissions, test_permissions_res, ["projects/my-project/instances/my-instance", ["spanner.instances.get", "spanner.instances.update"]] end end doctest.before "Google::Cloud::Spanner::Instance#delete" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_instances.expect :delete_instance, nil, ["projects/my-project/instances/my-instance"] end end @@ -310,7 +310,7 @@ def mock_spanner doctest.before "Google::Cloud::Spanner::Policy" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_instances.expect :get_iam_policy, policy_resp, ["projects/my-project/instances/my-instance"] mock_instances.expect :set_iam_policy, policy_resp, ["projects/my-project/instances/my-instance", Google::Iam::V1::Policy] end @@ -320,7 +320,7 @@ def mock_spanner doctest.before "Google::Cloud::Spanner::Project" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_databases.expect :get_database, database_resp, ["projects/my-project/instances/my-instance/databases/my-database"] end end @@ -363,19 +363,19 @@ def mock_spanner mock_client = Minitest::Mock.new mock_client.expect :get_operation, OpenStruct.new(done: true), ["1234567890", {:options=>nil}] mock_instances.expect :create_instance, create_instance_resp(client: mock_client), ["projects/my-project", "my-new-instance", Google::Spanner::Admin::Instance::V1::Instance] - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-new-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-new-instance", Hash] end end doctest.before "Google::Cloud::Spanner::Project#instance" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] end end doctest.before "Google::Cloud::Spanner::Project#instance@Will return `nil` if instance does not exist." do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/non-existing"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/non-existing", Hash] end end @@ -416,7 +416,7 @@ def mock_spanner doctest.before "Google::Cloud::Spanner::Project#databases" do mock_spanner do |mock, mock_instances, mock_databases| - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_databases.expect :list_databases, databases_resp(token: "token"), ["projects/my-project/instances/my-instance", Hash] mock_databases.expect :list_databases, databases_resp, ["projects/my-project/instances/my-instance", Hash] end @@ -550,7 +550,7 @@ def mock_spanner doctest.before "Google::Cloud::Spanner::Database" do mock_spanner do |mock, mock_instances, mock_databases| mock_client = Minitest::Mock.new - mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance"] + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] mock_client.expect :get_operation, OpenStruct.new(done: true), ["1234567890", {:options=>nil}] mock_databases.expect :create_database, create_database_resp(client: mock_client), ["projects/my-project/instances/my-instance", "CREATE DATABASE `my-new-database`", {:extra_statements=>[]}] mock_databases.expect :get_database, database_resp, ["projects/my-project/instances/my-instance/databases/my-new-database"] diff --git a/google-cloud-spanner/test/google/cloud/spanner/batch_client_test.rb b/google-cloud-spanner/test/google/cloud/spanner/batch_client_test.rb index 37c636cbd971..30b25805839b 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/batch_client_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/batch_client_test.rb @@ -48,7 +48,7 @@ it "retrieves the instance" do get_res = Google::Spanner::Admin::Instance::V1::Instance.new instance_hash(name: instance_id) mock = Minitest::Mock.new - mock.expect :get_instance, get_res, [instance_path(instance_id)] + mock.expect :get_instance, get_res, [instance_path(instance_id), Hash] spanner.service.mocked_instances = mock instance = spanner.instance instance_id diff --git a/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb index 0ca0691cf5cb..9222e71eda41 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb @@ -42,7 +42,7 @@ it "retrieves the instance" do get_res = Google::Spanner::Admin::Instance::V1::Instance.new instance_hash(name: instance_id) mock = Minitest::Mock.new - mock.expect :get_instance, get_res, [instance_path(instance_id)] + mock.expect :get_instance, get_res, [instance_path(instance_id), Hash] spanner.service.mocked_instances = mock instance = spanner.instance instance_id diff --git a/google-cloud-spanner/test/google/cloud/spanner/project/instance_test.rb b/google-cloud-spanner/test/google/cloud/spanner/project/instance_test.rb index 73bd60dda43e..c45c70d27622 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/project/instance_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/project/instance_test.rb @@ -20,7 +20,7 @@ get_res = Google::Spanner::Admin::Instance::V1::Instance.new instance_hash(name: instance_id) mock = Minitest::Mock.new - mock.expect :get_instance, get_res, [instance_path(instance_id)] + mock.expect :get_instance, get_res, [instance_path(instance_id), Hash] spanner.service.mocked_instances = mock instance = spanner.instance instance_id @@ -48,4 +48,27 @@ def stub.get_instance *args instance = spanner.instance not_found_instance_id instance.must_be :nil? end + + it "returns instance with provided fields" do + instance_id = "my-instance-id" + + get_res = Google::Spanner::Admin::Instance::V1::Instance.new name: instance_path(instance_id) + mock = Minitest::Mock.new + mock.expect :get_instance, get_res, [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["name"]) + ] + spanner.service.mocked_instances = mock + + instance = spanner.instance instance_id, fields: ["name"] + + mock.verify + + instance.project_id.must_equal project + instance.instance_id.must_equal instance_id + instance.path.must_equal instance_path(instance_id) + instance.name.must_equal "" + instance.node_count.must_equal 0 + instance.state.must_equal :STATE_UNSPECIFIED + end end From e99fdeea69dffa0aa18099beaded55e1a6b847e4 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Fri, 6 Dec 2019 14:40:55 +0530 Subject: [PATCH 02/15] instance endpoint_uris wrapper --- .../lib/google/cloud/spanner/instance.rb | 14 ++++++++++++++ .../test/google/cloud/spanner/instance_test.rb | 1 + 2 files changed, 15 insertions(+) diff --git a/google-cloud-spanner/lib/google/cloud/spanner/instance.rb b/google-cloud-spanner/lib/google/cloud/spanner/instance.rb index 551abc01537a..92d1be7bc060 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/instance.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/instance.rb @@ -188,6 +188,20 @@ def labels= labels ) end + ## + # The endpoint URIs based on the instance config. + # For example, instances located in a specific cloud region + # (or multi region) such as nam3, would have a nam3 specific + # endpoint URI. These endpoints are intended to optimize the network + # routing between the client and the instance's serving resources. + # If multiple endpoints are present, client may establish connections + # using any of the given URLs. + # + # @return [Array] The list of URIs. + def endpoint_uris + @grpc.endpoint_uris + end + def save job_grpc = service.update_instance @grpc Instance::Job.from_grpc job_grpc, service diff --git a/google-cloud-spanner/test/google/cloud/spanner/instance_test.rb b/google-cloud-spanner/test/google/cloud/spanner/instance_test.rb index e9054a712605..c4e95b6df898 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/instance_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/instance_test.rb @@ -27,5 +27,6 @@ instance.state.must_equal :READY instance.must_be :ready? instance.wont_be :creating? + instance.endpoint_uris.must_be_empty end end From b7e9d166d35ec358482e8c54345dac6ab9368154 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Wed, 11 Dec 2019 21:30:40 +0530 Subject: [PATCH 03/15] add client service proxy class for resource base routing - instance get method with fields mask --- .../lib/google/cloud/spanner/batch_client.rb | 23 +- .../lib/google/cloud/spanner/client.rb | 30 ++- .../cloud/spanner/client_service_proxy.rb | 222 ++++++++++++++++++ .../lib/google/cloud/spanner/project.rb | 30 ++- .../lib/google/cloud/spanner/results.rb | 4 +- .../lib/google/cloud/spanner/service.rb | 161 ++++++++----- .../lib/google/cloud/spanner/session.rb | 5 +- .../cloud/spanner/project/instance_test.rb | 8 +- 8 files changed, 401 insertions(+), 82 deletions(-) create mode 100644 google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb diff --git a/google-cloud-spanner/lib/google/cloud/spanner/batch_client.rb b/google-cloud-spanner/lib/google/cloud/spanner/batch_client.rb index c36d538f07e9..bfea6e9fa218 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/batch_client.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/batch_client.rb @@ -63,13 +63,26 @@ module Spanner # new_partition # class BatchClient + ## @private Service wrapper for batch data client api. + # @return [ClientServiceProxy] + attr_reader :service + ## # @private Creates a new Spanner BatchClient instance. - def initialize project, instance_id, database_id, session_labels: nil + def initialize \ + project, + instance_id, + database_id, + session_labels: nil, + enable_resource_based_routing: nil @project = project @instance_id = instance_id @database_id = database_id @session_labels = session_labels + @service = ClientServiceProxy.new \ + @project, + @instance_id, + enable_resource_based_routing: enable_resource_based_routing end # The unique identifier for the project. @@ -184,7 +197,7 @@ def batch_snapshot strong: nil, timestamp: nil, read_timestamp: nil, ensure_service! snp_session = session - snp_grpc = @project.service.create_snapshot \ + snp_grpc = @service.create_snapshot \ snp_session.path, strong: strong, timestamp: (timestamp || read_timestamp), staleness: (staleness || exact_staleness) @@ -231,7 +244,7 @@ def batch_snapshot strong: nil, timestamp: nil, read_timestamp: nil, def load_batch_snapshot serialized_snapshot ensure_service! - BatchSnapshot.load serialized_snapshot, service: @project.service + BatchSnapshot.load serialized_snapshot, service: @service end ## @@ -404,12 +417,12 @@ def ensure_service! # New session for each use. def session ensure_service! - grpc = @project.service.create_session \ + grpc = @service.create_session \ Admin::Database::V1::DatabaseAdminClient.database_path( project_id, instance_id, database_id ), labels: @session_labels - Session.from_grpc grpc, @project.service + Session.from_grpc grpc, @service end ## diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client.rb b/google-cloud-spanner/lib/google/cloud/spanner/client.rb index 5cc3550bd73f..de018eebdf1f 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/client.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/client.rb @@ -23,6 +23,7 @@ require "google/cloud/spanner/range" require "google/cloud/spanner/column_value" require "google/cloud/spanner/convert" +require "google/cloud/spanner/client_service_proxy" module Google module Cloud @@ -50,14 +51,22 @@ module Spanner # end # class Client + ## @private Service wrapper for data client api. + # @return [ClientServiceProxy] + attr_reader :service + ## # @private Creates a new Spanner Client instance. def initialize project, instance_id, database_id, session_labels: nil, - pool_opts: {} + pool_opts: {}, enable_resource_based_routing: nil @project = project @instance_id = instance_id @database_id = database_id @session_labels = session_labels + @service = ClientServiceProxy.new \ + @project, + @instance_id, + enable_resource_based_routing: enable_resource_based_routing @pool = Pool.new self, pool_opts end @@ -994,8 +1003,9 @@ def transaction deadline: 120 begin Thread.current[:transaction_id] = tx.transaction_id yield tx - commit_resp = @project.service.commit \ - tx.session.path, tx.mutations, transaction_id: tx.transaction_id + commit_resp = @service.commit \ + tx.session.path, tx.mutations, + transaction_id: tx.transaction_id return Convert.timestamp_to_time commit_resp.commit_timestamp rescue GRPC::Aborted, Google::Cloud::AbortedError => err # Re-raise if deadline has passed @@ -1093,7 +1103,7 @@ def snapshot strong: nil, timestamp: nil, read_timestamp: nil, @pool.with_session do |session| begin - snp_grpc = @project.service.create_snapshot \ + snp_grpc = @service.create_snapshot \ session.path, strong: strong, timestamp: (timestamp || read_timestamp), staleness: (staleness || exact_staleness) @@ -1286,12 +1296,12 @@ def reset # Creates a new session object every time. def create_new_session ensure_service! - grpc = @project.service.create_session \ + grpc = @service.create_session \ Admin::Database::V1::DatabaseAdminClient.database_path( project_id, instance_id, database_id ), labels: @session_labels - Session.from_grpc grpc, @project.service + Session.from_grpc grpc, @service end ## @@ -1314,13 +1324,15 @@ def batch_create_new_sessions total # def batch_create_sessions session_count ensure_service! - resp = @project.service.batch_create_sessions \ + resp = @service.batch_create_sessions \ Admin::Database::V1::DatabaseAdminClient.database_path( project_id, instance_id, database_id ), session_count, labels: @session_labels - resp.session.map { |grpc| Session.from_grpc grpc, @project.service } + resp.session.map do |grpc| + Session.from_grpc grpc, @service + end end # @private @@ -1385,7 +1397,7 @@ def single_use_transaction opts end def pdml_transaction session - pdml_tx_grpc = @project.service.create_pdml session.path + pdml_tx_grpc = @service.create_pdml session.path Google::Spanner::V1::TransactionSelector.new id: pdml_tx_grpc.id end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb new file mode 100644 index 000000000000..1c78f58bcec0 --- /dev/null +++ b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb @@ -0,0 +1,222 @@ +# Copyright 2019 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +module Google + module Cloud + module Spanner + ## + # @private + # + # # ClientServiceProxy + # + # Represents proxy Spanner service for resource based + # routing for data client. + class ClientServiceProxy < SimpleDelegator + # @private + # + # @param [Project] project + # @param [String] instance_id + # @param [true, false, nil] enable_resource_based_routing. + # Default resource-based routing is disabled. + # + def initialize project, instance_id, enable_resource_based_routing: nil + @project = project + @instance_id = instance_id + @enable_resource_based_routing = enable_resource_based_routing + + __setobj__ @project.service + end + + # Get instance endpoint uri. + # + # @return [String] If resource-based routing is enabled then returns + # first endpoint uri of instance, otherwise returns default + # global host uri. + def endpoint_uri + return @endpoint_uri if @endpoint_uri + + if resource_based_routing_enabled? + instance = @project.instance @instance_id, fields: ["endpoint_uri"] + @endpoint_uri = instance.endpoint_uris.first if instance + end + + @endpoint_uri ||= host + end + + def get_session session_name + super session_name, endpoint_uri: endpoint_uri + end + + def create_session database_name, labels: nil + super database_name, labels: labels, endpoint_uri: endpoint_uri + end + + def batch_create_sessions database_name, session_count, labels: nil + super \ + database_name, + session_count, + labels: labels, + endpoint_uri: endpoint_uri + end + + def delete_session session_name + super session_name, endpoint_uri: endpoint_uri + end + + def execute_streaming_sql \ + session_name, sql, + transaction: nil, + params: nil, + types: nil, + resume_token: nil, + partition_token: nil, + seqno: nil + super \ + session_name, sql, + transaction: transaction, + params: params, + types: types, + resume_token: resume_token, + partition_token: partition_token, + seqno: seqno, + endpoint_uri: endpoint_uri + end + + def execute_batch_dml session_name, transaction, statements, seqno + super \ + session_name, + transaction, + statements, + seqno, + endpoint_uri: endpoint_uri + end + + def streaming_read_table \ + session_name, + table_name, + columns, + keys: nil, + index: nil, + transaction: nil, + limit: nil, + resume_token: nil, + partition_token: nil + super \ + session_name, + table_name, + columns, + keys: keys, + index: index, + transaction: transaction, + limit: limit, + resume_token: resume_token, + partition_token: partition_token, + endpoint_uri: endpoint_uri + end + + def partition_read \ + session_name, + table_name, + columns, + transaction, + keys: nil, + index: nil, + partition_size_bytes: nil, + max_partitions: nil + super \ + session_name, + table_name, + columns, + transaction, + keys: keys, + index: index, + partition_size_bytes: partition_size_bytes, + max_partitions: max_partitions, + endpoint_uri: endpoint_uri + end + + def partition_query \ + session_name, + sql, + transaction, + params: nil, + types: nil, + partition_size_bytes: nil, + max_partitions: nil + super \ + session_name, + sql, + transaction, + params: params, + types: types, + partition_size_bytes: partition_size_bytes, + max_partitions: max_partitions, + endpoint_uri: endpoint_uri + end + + def commit session_name, mutations = [], transaction_id: nil + super \ + session_name, + mutations, + transaction_id: transaction_id, + endpoint_uri: endpoint_uri + end + + def rollback session_name, transaction_id + super session_name, transaction_id, endpoint_uri: endpoint_uri + end + + def begin_transaction session_name + super session_name, endpoint_uri: endpoint_uri + end + + def create_snapshot \ + session_name, + strong: nil, + timestamp: nil, + staleness: nil + super \ + session_name, + strong: strong, + timestamp: timestamp, + staleness: staleness, + endpoint_uri: endpoint_uri + end + + def create_pdml session_name + super session_name, endpoint_uri: endpoint_uri + end + + private + + # Check resource based routing is enabled or not. + # + # @return [Boolean] + def resource_based_routing_enabled? + case @enable_resource_based_routing + when TrueClass + true + when FalseClass + false + when NilClass + ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] == "YES" + else + false + end + end + end + end + end +end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/project.rb b/google-cloud-spanner/lib/google/cloud/spanner/project.rb index 21a9b77abbf0..20bef87363d7 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/project.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/project.rb @@ -479,12 +479,21 @@ def create_database instance_id, database_id, statements: [] # end # end # - def client instance_id, database_id, pool: {}, labels: nil + def client \ + instance_id, + database_id, + pool: {}, + labels: nil, + enable_resource_based_routing: nil # Convert from possible Google::Protobuf::Map labels = Hash[labels.map { |k, v| [String(k), String(v)] }] if labels - Client.new self, instance_id, database_id, - session_labels: labels, - pool_opts: valid_session_pool_options(pool) + Client.new \ + self, + instance_id, + database_id, + session_labels: labels, + pool_opts: valid_session_pool_options(pool), + enable_resource_based_routing: enable_resource_based_routing end ## @@ -538,10 +547,19 @@ def client instance_id, database_id, pool: {}, labels: nil # results = new_batch_snapshot.execute_partition \ # new_partition # - def batch_client instance_id, database_id, labels: nil + def batch_client \ + instance_id, + database_id, + labels: nil, + enable_resource_based_routing: nil # Convert from possible Google::Protobuf::Map labels = Hash[labels.map { |k, v| [String(k), String(v)] }] if labels - BatchClient.new self, instance_id, database_id, session_labels: labels + BatchClient.new \ + self, + instance_id, + database_id, + session_labels: labels, + enable_resource_based_routing: enable_resource_based_routing end protected diff --git a/google-cloud-spanner/lib/google/cloud/spanner/results.rb b/google-cloud-spanner/lib/google/cloud/spanner/results.rb index c4e2059be6d0..1788fb702057 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/results.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/results.rb @@ -265,7 +265,9 @@ def self.read service, session_path, table, columns, keys: nil, transaction: transaction, partition_token: partition_token } enum = service.streaming_read_table \ - session_path, table, columns, read_options + session_path, + table, columns, + read_options from_enum(enum, service).tap do |results| results.instance_variable_set :@session_path, session_path results.instance_variable_set :@table, table diff --git a/google-cloud-spanner/lib/google/cloud/spanner/service.rb b/google-cloud-spanner/lib/google/cloud/spanner/service.rb index 2015f95152d6..84f33a12976d 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/service.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/service.rb @@ -1,3 +1,4 @@ + # Copyright 2016 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -40,6 +41,7 @@ def initialize project, credentials, host: nil, timeout: nil, @host = host || V1::SpannerClient::SERVICE_ADDRESS @timeout = timeout @client_config = client_config || {} + @spanner_clients = {} end def channel @@ -58,15 +60,16 @@ def chan_creds GRPC::Core::CallCredentials.new credentials.client.updater_proc end - def service + def service endpoint_uri = nil return mocked_service if mocked_service - @service ||= \ + + @spanner_clients[endpoint_uri] ||= \ V1::SpannerClient.new( credentials: channel, timeout: timeout, client_config: client_config, - service_address: service_address, - service_port: service_port, + service_address: service_address(endpoint_uri), + service_port: service_port(endpoint_uri), lib_name: "gccl", lib_version: Google::Cloud::Spanner::VERSION ) @@ -264,47 +267,58 @@ def test_database_permissions instance_id, database_id, permissions end end - def get_session session_name + def get_session session_name, endpoint_uri: nil opts = default_options_from_session session_name execute do - service.get_session session_name, options: opts + service(endpoint_uri).get_session session_name, options: opts end end - def create_session database_name, labels: nil + def create_session database_name, labels: nil, endpoint_uri: nil opts = default_options_from_session database_name session = Google::Spanner::V1::Session.new labels: labels if labels execute do - service.create_session database_name, session: session, - options: opts + service(endpoint_uri).create_session database_name, session: session, + options: opts end end - def batch_create_sessions database_name, session_count, labels: nil + def batch_create_sessions \ + database_name, + session_count, + labels: nil, + endpoint_uri: nil opts = default_options_from_session database_name session = Google::Spanner::V1::Session.new labels: labels if labels execute do # The response may have fewer sessions than requested in the RPC. - service.batch_create_sessions database_name, - session_count, - session_template: session, - options: opts + service(endpoint_uri).batch_create_sessions \ + database_name, + session_count, + session_template: session, + options: opts end end - def delete_session session_name + def delete_session session_name, endpoint_uri: nil opts = default_options_from_session session_name execute do - service.delete_session session_name, options: opts + service(endpoint_uri).delete_session session_name, options: opts end end - def execute_streaming_sql session_name, sql, transaction: nil, - params: nil, types: nil, resume_token: nil, - partition_token: nil, seqno: nil + def execute_streaming_sql \ + session_name, sql, + transaction: nil, + params: nil, + types: nil, + resume_token: nil, + partition_token: nil, + seqno: nil, + endpoint_uri: nil opts = default_options_from_session session_name execute do - service.execute_streaming_sql \ + service(endpoint_uri).execute_streaming_sql \ session_name, sql, transaction: transaction, params: params, param_types: types, @@ -315,15 +329,21 @@ def execute_streaming_sql session_name, sql, transaction: nil, end end - def execute_batch_dml session_name, transaction, statements, seqno + def execute_batch_dml \ + session_name, + transaction, + statements, + seqno, + endpoint_uri: nil opts = default_options_from_session session_name statements = statements.map(&:to_grpc) results = execute do - service.execute_batch_dml session_name, - transaction, - statements, - seqno, - options: opts + service(endpoint_uri).execute_batch_dml \ + session_name, + transaction, + statements, + seqno, + options: opts end if results.status.code.zero? results.result_sets.map { |rs| rs.stats.row_count_exact } @@ -336,12 +356,20 @@ def execute_batch_dml session_name, transaction, statements, seqno end end - def streaming_read_table session_name, table_name, columns, keys: nil, - index: nil, transaction: nil, limit: nil, - resume_token: nil, partition_token: nil + def streaming_read_table \ + session_name, + table_name, + columns, + keys: nil, + index: nil, + transaction: nil, + limit: nil, + resume_token: nil, + partition_token: nil, + endpoint_uri: nil opts = default_options_from_session session_name execute do - service.streaming_read \ + service(endpoint_uri).streaming_read \ session_name, table_name, columns, keys, transaction: transaction, index: index, limit: limit, resume_token: resume_token, partition_token: partition_token, @@ -349,32 +377,45 @@ def streaming_read_table session_name, table_name, columns, keys: nil, end end - def partition_read session_name, table_name, columns, transaction, - keys: nil, index: nil, partition_size_bytes: nil, - max_partitions: nil + def partition_read \ + session_name, + table_name, + columns, + transaction, + keys: nil, + index: nil, + partition_size_bytes: nil, + max_partitions: nil, + endpoint_uri: nil partition_opts = partition_options partition_size_bytes, max_partitions opts = default_options_from_session session_name execute do - service.partition_read \ + service(endpoint_uri).partition_read \ session_name, table_name, keys, transaction: transaction, index: index, columns: columns, partition_options: partition_opts, options: opts end end - def partition_query session_name, sql, transaction, params: nil, - types: nil, partition_size_bytes: nil, - max_partitions: nil + def partition_query \ + session_name, + sql, + transaction, + params: nil, + types: nil, + partition_size_bytes: nil, + max_partitions: nil, + endpoint_uri: nil partition_opts = partition_options partition_size_bytes, max_partitions opts = default_options_from_session session_name execute do - service.partition_query \ + service(endpoint_uri).partition_query \ session_name, sql, transaction: transaction, params: params, param_types: types, @@ -382,7 +423,11 @@ def partition_query session_name, sql, transaction, params: nil, end end - def commit session_name, mutations = [], transaction_id: nil + def commit \ + session_name, + mutations = [], + transaction_id: nil, + endpoint_uri: nil tx_opts = nil if transaction_id.nil? tx_opts = Google::Spanner::V1::TransactionOptions.new( @@ -391,32 +436,37 @@ def commit session_name, mutations = [], transaction_id: nil end opts = default_options_from_session session_name execute do - service.commit \ + service(endpoint_uri).commit \ session_name, mutations, - transaction_id: transaction_id, single_use_transaction: tx_opts, + transaction_id: transaction_id, + single_use_transaction: tx_opts, options: opts end end - def rollback session_name, transaction_id + def rollback session_name, transaction_id, endpoint_uri: nil opts = default_options_from_session session_name execute do - service.rollback session_name, transaction_id, options: opts + service(endpoint_uri).rollback session_name, transaction_id, options: opts end end - def begin_transaction session_name + def begin_transaction session_name, endpoint_uri: nil tx_opts = Google::Spanner::V1::TransactionOptions.new( read_write: Google::Spanner::V1::TransactionOptions::ReadWrite.new ) opts = default_options_from_session session_name execute do - service.begin_transaction session_name, tx_opts, options: opts + service(endpoint_uri).begin_transaction session_name, tx_opts, options: opts end end - def create_snapshot session_name, strong: nil, timestamp: nil, - staleness: nil + def create_snapshot \ + session_name, + strong: nil, + timestamp: nil, + staleness: nil, + endpoint_uri: nil tx_opts = Google::Spanner::V1::TransactionOptions.new( read_only: Google::Spanner::V1::TransactionOptions::ReadOnly.new( { @@ -429,18 +479,19 @@ def create_snapshot session_name, strong: nil, timestamp: nil, ) opts = default_options_from_session session_name execute do - service.begin_transaction session_name, tx_opts, options: opts + service(endpoint_uri).begin_transaction session_name, tx_opts, options: opts end end - def create_pdml session_name + def create_pdml session_name, endpoint_uri: nil tx_opts = Google::Spanner::V1::TransactionOptions.new( partitioned_dml: \ Google::Spanner::V1::TransactionOptions::PartitionedDml.new ) opts = default_options_from_session session_name execute do - service.begin_transaction session_name, tx_opts, options: opts + service(endpoint_uri).begin_transaction \ + session_name, tx_opts, options: opts end end @@ -450,14 +501,12 @@ def inspect protected - def service_address - return nil if host.nil? - URI.parse("//#{host}").host + def service_address uri = nil + URI.parse("//#{uri || host}").host end - def service_port - return nil if host.nil? - URI.parse("//#{host}").port + def service_port uri = nil + URI.parse("//#{uri || host}").port end def default_options_from_session session_name diff --git a/google-cloud-spanner/lib/google/cloud/spanner/session.rb b/google-cloud-spanner/lib/google/cloud/spanner/session.rb index 62233e69b70d..7948d0b11e3f 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/session.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/session.rb @@ -404,8 +404,9 @@ def commit transaction_id: nil ensure_service! commit = Commit.new yield commit - commit_resp = service.commit path, commit.mutations, - transaction_id: transaction_id + commit_resp = service.commit \ + path, commit.mutations, + transaction_id: transaction_id @last_updated_at = Time.now Convert.timestamp_to_time commit_resp.commit_timestamp end diff --git a/google-cloud-spanner/test/google/cloud/spanner/project/instance_test.rb b/google-cloud-spanner/test/google/cloud/spanner/project/instance_test.rb index c45c70d27622..39fd4790ac3d 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/project/instance_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/project/instance_test.rb @@ -52,21 +52,23 @@ def stub.get_instance *args it "returns instance with provided fields" do instance_id = "my-instance-id" - get_res = Google::Spanner::Admin::Instance::V1::Instance.new name: instance_path(instance_id) + get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ + name: instance_path(instance_id), endpoint_uris: ["test.host.com"] mock = Minitest::Mock.new mock.expect :get_instance, get_res, [ instance_path(instance_id), - field_mask: Google::Protobuf::FieldMask.new(paths: ["name"]) + field_mask: Google::Protobuf::FieldMask.new(paths: ["name", "endpoint_uris"]) ] spanner.service.mocked_instances = mock - instance = spanner.instance instance_id, fields: ["name"] + instance = spanner.instance instance_id, fields: ["name", "endpoint_uris"] mock.verify instance.project_id.must_equal project instance.instance_id.must_equal instance_id instance.path.must_equal instance_path(instance_id) + instance.endpoint_uris.must_equal ["test.host.com"] instance.name.must_equal "" instance.node_count.must_equal 0 instance.state.must_equal :STATE_UNSPECIFIED From deedf543acf4240cd6c4edb3e84d2158b7f8533e Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Tue, 17 Dec 2019 19:02:31 +0530 Subject: [PATCH 04/15] update client and batch client docs and doctests --- .../cloud/spanner/client_service_proxy.rb | 3 +- .../lib/google/cloud/spanner/project.rb | 51 ++++++++++++++++++- .../support/doctest_helper.rb | 38 +++++++++----- 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb index 1c78f58bcec0..d33a91162162 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb @@ -211,7 +211,8 @@ def resource_based_routing_enabled? when FalseClass false when NilClass - ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] == "YES" + ["YES", "yes"].include? \ + ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] else false end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/project.rb b/google-cloud-spanner/lib/google/cloud/spanner/project.rb index 20bef87363d7..448dd1ae2e1b 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/project.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/project.rb @@ -461,7 +461,10 @@ def create_database instance_id, database_id, statements: [] # * Label values must be between 0 and 63 characters long and must # conform to the regular expression `([a-z]([-a-z0-9]*[a-z0-9])?)?`. # * No more than 64 labels can be associated with a given resource. - # + # @param [Boolean, nil] enable_resource_based_routing Enable/Disable + # resource-based routing for data operation, by default is disabled. + # Resource based routing can be enabled using the environment + # `GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING` to `YES` or `yes`. # @return [Client] The newly created client. # # @example @@ -479,6 +482,22 @@ def create_database instance_id, database_id, statements: [] # end # end # + # @example Enable resource based routing. + # require "google/cloud/spanner" + # + # spanner = Google::Cloud::Spanner.new + # + # db = spanner.client \ + # "my-instance", "my-database", enable_resource_based_routing: true + # + # db.transaction do |tx| + # results = tx.execute_query "SELECT * FROM users" + # + # results.rows.each do |row| + # puts "User #{row[:id]} is #{row[:name]}" + # end + # end + # def client \ instance_id, database_id, @@ -519,6 +538,10 @@ def client \ # * Label values must be between 0 and 63 characters long and must # conform to the regular expression `([a-z]([-a-z0-9]*[a-z0-9])?)?`. # * No more than 64 labels can be associated with a given resource. + # @param [Boolean, nil] enable_resource_based_routing Enable/Disable + # resource-based routing for data operation, by default is disabled. + # Resource based routing can be enabled using the environment + # `GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING` to `YES` or `yes`. # # @return [Client] The newly created client. # @@ -547,6 +570,32 @@ def client \ # results = new_batch_snapshot.execute_partition \ # new_partition # + # @example Enable resource based routing. + # require "google/cloud/spanner" + # + # spanner = Google::Cloud::Spanner.new + # + # batch_client = spanner.batch_client \ + # "my-instance", "my-database", enable_resource_based_routing: true + # + # batch_snapshot = batch_client.batch_snapshot + # serialized_snapshot = batch_snapshot.dump + # + # partitions = batch_snapshot.partition_read "users", [:id, :name] + # + # partition = partitions.first + # serialized_partition = partition.dump + # + # # In a separate process + # new_batch_snapshot = batch_client.load_batch_snapshot \ + # serialized_snapshot + # + # new_partition = batch_client.load_partition \ + # serialized_partition + # + # results = new_batch_snapshot.execute_partition \ + # new_partition + # def batch_client \ instance_id, database_id, diff --git a/google-cloud-spanner/support/doctest_helper.rb b/google-cloud-spanner/support/doctest_helper.rb index 2ab70f5a6671..0aaad587b53d 100644 --- a/google-cloud-spanner/support/doctest_helper.rb +++ b/google-cloud-spanner/support/doctest_helper.rb @@ -347,6 +347,18 @@ def mock_spanner end end + doctest.before "Google::Cloud::Spanner::Project#batch_client@Enable resource based routing." do + mock_spanner do |mock, mock_instances, mock_databases| + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] + mock.expect :create_session, session_grpc, ["projects/my-project/instances/my-instance/databases/my-database", Hash] + mock.expect :begin_transaction, tx_resp, ["session-name", Google::Spanner::V1::TransactionOptions, Hash] + mock.expect :partition_read, OpenStruct.new(partitions: [Google::Spanner::V1::Partition.new(partition_token: "partition-token")]), + ["session-name", "users", Google::Spanner::V1::KeySet, Hash] + mock.expect :streaming_read, results_enum, ["session-name", "users", ["id", "name"], Google::Spanner::V1::KeySet, Hash] + mock.expect :delete_session, session_grpc, ["session-name", Hash] + end + end + doctest.before "Google::Cloud::Spanner::Project#client" do mock_spanner do |mock, mock_instances, mock_databases| mock.expect :batch_create_sessions, OpenStruct.new(session: Array.new(10) { session_grpc }), ["projects/my-project/instances/my-instance/databases/my-database", 10, Hash] @@ -358,6 +370,18 @@ def mock_spanner end end + doctest.before "Google::Cloud::Spanner::Project#client@Enable resource based routing." do + mock_spanner do |mock, mock_instances, mock_databases| + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] + mock.expect :batch_create_sessions, OpenStruct.new(session: Array.new(10) { session_grpc }), ["projects/my-project/instances/my-instance/databases/my-database", 10, Hash] + 5.times do + mock.expect :begin_transaction, tx_resp, ["session-name", Google::Spanner::V1::TransactionOptions, Hash] + end + mock.expect :execute_streaming_sql, results_enum, ["session-name", "SELECT * FROM users", Hash] + mock.expect :commit, commit_resp, ["session-name", Array, Hash] + end + end + doctest.before "Google::Cloud::Spanner::Project#create_instance" do mock_spanner do |mock, mock_instances, mock_databases| mock_client = Minitest::Mock.new @@ -826,17 +850,6 @@ def project "my-project" end -def instance_hash name: "my-instance", nodes: 1, state: "READY", labels: {} - { - name: "projects/#{project}/instances/#{name}", - config: "projects/#{project}/instanceConfigs/regional-us-central1", - display_name: name.split("-").map(&:capitalize).join(" "), - nodeCount: nodes, - state: state, - labels: labels - } -end - def job_grpc Google::Longrunning::Operation.new( name: "1234567890", @@ -895,7 +908,8 @@ def instance_hash name: "my-instance", nodes: 1, state: "READY", labels: {} display_name: name.split("-").map(&:capitalize).join(" "), node_count: nodes, state: state, - labels: labels + labels: labels, + endpoint_uris: [] } end From a2c9c93b52adc2216e0ef0f86bc0a3753e8a7c20 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Thu, 19 Dec 2019 12:55:36 +0530 Subject: [PATCH 05/15] fix field mask param name for get instance - add unit test for client service proxy --- .../cloud/spanner/client_service_proxy.rb | 8 +- .../lib/google/cloud/spanner/service.rb | 8 + .../google/cloud/spanner/client/admin_test.rb | 5 + .../client_service_proxy/endpont_uri_test.rb | 94 +++++ .../spanner/client_service_proxy_test.rb | 340 ++++++++++++++++++ 5 files changed, 452 insertions(+), 3 deletions(-) create mode 100644 google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb create mode 100644 google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb index d33a91162162..dbc2373e0e8a 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb @@ -48,7 +48,7 @@ def endpoint_uri return @endpoint_uri if @endpoint_uri if resource_based_routing_enabled? - instance = @project.instance @instance_id, fields: ["endpoint_uri"] + instance = @project.instance @instance_id, fields: ["endpoint_uris"] @endpoint_uri = instance.endpoint_uris.first if instance end @@ -76,7 +76,8 @@ def delete_session session_name end def execute_streaming_sql \ - session_name, sql, + session_name, + sql, transaction: nil, params: nil, types: nil, @@ -84,7 +85,8 @@ def execute_streaming_sql \ partition_token: nil, seqno: nil super \ - session_name, sql, + session_name, + sql, transaction: transaction, params: params, types: types, diff --git a/google-cloud-spanner/lib/google/cloud/spanner/service.rb b/google-cloud-spanner/lib/google/cloud/spanner/service.rb index 84f33a12976d..1c0245fe0bca 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/service.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/service.rb @@ -60,6 +60,13 @@ def chan_creds GRPC::Core::CallCredentials.new credentials.client.updater_proc end + ## + # Create and cache spanner client connection based on instance + # endpoint uri for resource based request routing. + # + # @param [String, nil] endpoint_uri Instance endpoint uri. + # @return [V1::SpannerClient] + # def service endpoint_uri = nil return mocked_service if mocked_service @@ -125,6 +132,7 @@ def list_instances token: nil, max: nil def get_instance name, fields: nil field_mask = Google::Protobuf::FieldMask.new(paths: fields || []) + execute do instances.get_instance instance_path(name), field_mask: field_mask end diff --git a/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb index 9222e71eda41..67d4a5eac87e 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb @@ -73,4 +73,9 @@ database.database_id.must_equal database_id database.path.must_equal database_path(instance_id, database_id) end + + it "set service as a client service proxy" do + client.service.must_be_kind_of Google::Cloud::Spanner::Service + client.service.class.must_equal Google::Cloud::Spanner::ClientServiceProxy + end end diff --git a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb new file mode 100644 index 000000000000..e721f5cdf8e8 --- /dev/null +++ b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb @@ -0,0 +1,94 @@ +# Copyright 2016 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "helper" + +describe Google::Cloud::Spanner::ClientServiceProxy, :endpoint_uri, :mock_spanner do + let(:instance_id) { "my-instance-id" } + + after do + ENV.delete "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING" + end + + it "gets default endpoint uri" do + client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ + spanner, instance_id + client_service_proxy.endpoint_uri.must_equal \ + Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + end + + it "gets default endpoint uri if resource based routing disabled" do + client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ + spanner, instance_id, enable_resource_based_routing: false + client_service_proxy.endpoint_uri.must_equal \ + Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + end + + it "gets endpoint uri if resource based routing enabled" do + get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ + name: instance_path(instance_id), endpoint_uris: ["test.host.com"] + mock = Minitest::Mock.new + mock.expect :get_instance, get_res, [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) + ] + spanner.service.mocked_instances = mock + + client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ + spanner, instance_id, enable_resource_based_routing: true + + client_service_proxy.endpoint_uri.must_equal "test.host.com" + + mock.verify + end + + it "gets endpoint uri if resource based routing enabled using an environment variable" do + ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] = "YES" + + get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ + name: instance_path(instance_id), endpoint_uris: ["test.host.com"] + mock = Minitest::Mock.new + mock.expect :get_instance, get_res, [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) + ] + spanner.service.mocked_instances = mock + + client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ + spanner, instance_id, enable_resource_based_routing: true + + client_service_proxy.endpoint_uri.must_equal "test.host.com" + + mock.verify + end + + it "gets default endpoint uri if resource based routing enabled and instance endpoint uris not present" do + get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ + name: instance_path(instance_id), endpoint_uris: [] + mock = Minitest::Mock.new + mock.expect :get_instance, get_res, [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) + ] + spanner.service.mocked_instances = mock + + client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ + spanner, instance_id, enable_resource_based_routing: true + + client_service_proxy.endpoint_uri.must_equal \ + Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + + mock.verify + end +end diff --git a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb new file mode 100644 index 000000000000..1b330990dc1d --- /dev/null +++ b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb @@ -0,0 +1,340 @@ +# Copyright 2019 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "helper" + +describe Google::Cloud::Spanner::ClientServiceProxy, :mock_spanner do + let(:instance_id) { "my-instance-id" } + let(:database_id) { "my-database-id" } + let(:session_id) { "session123" } + let(:session_name) { session_path(instance_id, database_id, session_id) } + let(:client_service_proxy) { + Google::Cloud::Spanner::ClientServiceProxy.new \ + spanner, instance_id, enable_resource_based_routing: true + } + let(:instance_endpoint_uri) { "test.host.com" } + let(:get_instance_req) { + [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) + ] + } + let(:get_insance_res){ + Google::Spanner::Admin::Instance::V1::Instance.new \ + name: instance_path(instance_id), endpoint_uris: [instance_endpoint_uri] + } + let(:statement){ + Google::Spanner::V1::ExecuteBatchDmlRequest::Statement.new \ + sql: "UPDATE users SET age = @age", + params: Google::Protobuf::Struct.new(fields: + { "age" => Google::Protobuf::Value.new(string_value: "29") } + ), + param_types: { "age" => Google::Spanner::V1::Type.new(code: :INT64) } + } + let(:tx_selector) { + Google::Spanner::V1::TransactionSelector.new id: "tx123" + } + + it "knows the identifiers" do + client_service_proxy.must_be_kind_of Google::Cloud::Spanner::Service + client_service_proxy.instance_variable_get("@project").must_equal spanner + client_service_proxy.instance_variable_get("@instance_id").must_equal instance_id + client_service_proxy.instance_variable_get("@enable_resource_based_routing").must_equal true + end + + describe "api calls" do + before :each do + @mock = Minitest::Mock.new + @mock.expect :get_instance, get_insance_res, [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) + ] + spanner.service.mocked_instances = @mock + end + + it "add endpoint uri to get_session" do + api_call_stub = proc do |session_name, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :get_session, api_call_stub do + client_service_proxy.get_session session_name + end + + @mock.verify + end + + it "add endpoint uri to create_session" do + api_call_stub = proc do |database_name, labels: nil, endpoint_uri: nil| + database_name.must_equal database_path(instance_id, database_id) + labels.must_equal ["test-session"] + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :create_session, api_call_stub do + client_service_proxy.create_session \ + database_path(instance_id, database_id), labels: ["test-session"] + end + + @mock.verify + end + + it "add endpoint uri to batch_create_sessions" do + api_call_stub = proc do |database_name, session_count, labels: nil, endpoint_uri: nil| + database_name.must_equal database_path(instance_id, database_id) + session_count.must_equal 5 + labels.must_equal ["test-session"] + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :batch_create_sessions, api_call_stub do + client_service_proxy.batch_create_sessions \ + database_path(instance_id, database_id), 5, labels: ["test-session"] + end + + @mock.verify + end + + it "add endpoint uri to delete_session" do + api_call_stub = proc do |session_name, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :delete_session, api_call_stub do + client_service_proxy.delete_session session_name + end + + @mock.verify + end + + it "add endpoint uri to execute_streaming_sql" do + api_call_stub = proc do |session_name, sql, transaction: nil, + params: nil, types: nil, resume_token: nil, partition_token: nil, + seqno: nil, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + sql.must_equal statement.sql + transaction.must_equal tx_selector + params.must_equal statement.params + types.must_equal statement.param_types + resume_token.must_equal "test-resume-token" + partition_token.must_equal "test-partition-token" + seqno.must_equal 1 + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :execute_streaming_sql, api_call_stub do + client_service_proxy.execute_streaming_sql \ + session_name, + statement.sql, + transaction: tx_selector, + params: statement.params, + types: statement.param_types, + resume_token: "test-resume-token", + partition_token: "test-partition-token", + seqno: 1 + end + + @mock.verify + end + + it "add endpoint uri to execute_batch_dml" do + api_call_stub = proc do |session_name, transaction, statements, seqno, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + transaction.must_equal tx_selector + statements.must_equal [statement] + seqno.must_equal 1 + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :execute_batch_dml, api_call_stub do + client_service_proxy.execute_batch_dml \ + session_name, tx_selector, [statement], 1 + end + + @mock.verify + end + + it "add endpoint uri to streaming_read_table" do + api_call_stub = proc do |session_name, table_name, columns, + keys: nil, index: nil, transaction: nil, limit: nil, resume_token: nil, + partition_token: nil, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + table_name.must_equal "users" + columns.must_equal ["id", "name"] + keys.must_equal Google::Spanner::V1::KeySet.new(all: true) + index.must_equal "id" + transaction.must_equal tx_selector + limit.must_equal 100 + resume_token.must_equal "test-resume-token" + partition_token.must_equal "test-partition-token" + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :streaming_read_table, api_call_stub do + client_service_proxy.streaming_read_table \ + session_name, "users", ["id", "name"], + keys: Google::Spanner::V1::KeySet.new(all: true), + index: "id", + transaction: tx_selector, + limit: 100, + resume_token: "test-resume-token", + partition_token: "test-partition-token" + end + + @mock.verify + end + + it "add endpoint uri to partition_read" do + api_call_stub = proc do |session_name, table_name, columns, transaction, + keys: nil, index: nil, partition_size_bytes: nil, max_partitions: nil, + endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + table_name.must_equal "users" + columns.must_equal ["id", "name"] + transaction.must_equal tx_selector + keys.must_equal Google::Spanner::V1::KeySet.new(all: true) + index.must_equal "id" + partition_size_bytes.must_equal 1024 + max_partitions.must_equal 3 + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :partition_read, api_call_stub do + client_service_proxy.partition_read \ + session_name, "users", ["id", "name"], tx_selector, + keys: Google::Spanner::V1::KeySet.new(all: true), + index: "id", + partition_size_bytes: 1024, + max_partitions: 3 + end + + @mock.verify + end + + it "add endpoint uri to partition_query" do + api_call_stub = proc do |session_name, sql, transaction, params: nil, + types: nil, partition_size_bytes: nil, max_partitions: nil, + endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + sql.must_equal statement.sql + transaction.must_equal tx_selector + params.must_equal statement.params + types.must_equal statement.param_types + partition_size_bytes.must_equal 2048 + max_partitions.must_equal 5 + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :partition_query, api_call_stub do + client_service_proxy.partition_query \ + session_name, statement.sql, tx_selector, + params: statement.params, + types: statement.param_types, + partition_size_bytes: 2048, + max_partitions: 5 + end + + @mock.verify + end + + it "add endpoint uri to begin_transaction" do + api_call_stub = proc do |session_name, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :begin_transaction, api_call_stub do + client_service_proxy.begin_transaction session_name + end + + @mock.verify + end + + it "add endpoint uri to commit" do + req_mutations = [ + Google::Spanner::V1::Mutation.new( + update: Google::Spanner::V1::Mutation::Write.new( + table: "users", columns: %w(id name active), + values: [Google::Cloud::Spanner::Convert.object_to_grpc_value([1, "Charlie", false]).list_value] + ) + ) + ] + api_call_stub = proc do |session_name, mutations, transaction_id: nil, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + mutations.must_equal req_mutations + transaction_id.must_equal "txn123" + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :commit, api_call_stub do + client_service_proxy.commit session_name, req_mutations, transaction_id: "txn123" + end + + @mock.verify + end + + focus + it "add endpoint uri to rollback" do + api_call_stub = proc do |session_name, transaction_id, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + transaction_id.must_equal "txn123" + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :rollback, api_call_stub do + client_service_proxy.rollback session_name, "txn123" + end + + @mock.verify + end + + focus + it "add endpoint uri to create_snapshot" do + timestamp = Time.now + api_call_stub = proc do |session_name, strong: nil, timestamp: nil, + staleness: nil, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + strong.must_equal true + timestamp.must_equal timestamp + staleness.must_equal 30 + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :create_snapshot, api_call_stub do + client_service_proxy.create_snapshot \ + session_name, + strong: true, + timestamp: timestamp, + staleness: 30 + end + + @mock.verify + end + + it "add endpoint uri to create_pdml" do + api_call_stub = proc do |session_name, endpoint_uri: nil| + session_name.must_equal session_path(instance_id, database_id, session_id) + endpoint_uri.must_equal instance_endpoint_uri + end + + spanner.service.stub :create_pdml, api_call_stub do + client_service_proxy.create_pdml session_name + end + + @mock.verify + end + end +end From 44c7005336dfcd1fcf2b276580df37cff4069008 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Sat, 21 Dec 2019 15:26:39 +0530 Subject: [PATCH 06/15] unit and acceptance test cases for database service client --- .../acceptance/spanner/client_test.rb | 36 ++++++++++ .../spanner/client_service_proxy_test.rb | 2 - .../test/google/cloud/spanner/service_test.rb | 65 +++++++++++++++++++ 3 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 google-cloud-spanner/acceptance/spanner/client_test.rb create mode 100644 google-cloud-spanner/test/google/cloud/spanner/service_test.rb diff --git a/google-cloud-spanner/acceptance/spanner/client_test.rb b/google-cloud-spanner/acceptance/spanner/client_test.rb new file mode 100644 index 000000000000..840bf0029f18 --- /dev/null +++ b/google-cloud-spanner/acceptance/spanner/client_test.rb @@ -0,0 +1,36 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "spanner_helper" +require "concurrent" + +describe "Spanner Client", :spanner do + let(:spanner) { $spanner } + let(:instance_id) { $spanner_instance_id } + let(:database_id) { $spanner_database_id } + + it "create client connection without resource based routing" do + spanner.client instance_id, database_id + spanner_clients = spanner.service.instance_variable_get("@spanner_clients") + spanner_clients.length.must_equal 1 + spanner_clients.must_include Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + end + + it "create client connection with resource based routing" do + spanner.client instance_id, database_id, enable_resource_based_routing: true + spanner_clients = spanner.service.instance_variable_get("@spanner_clients") + spanner_clients.length.must_equal 1 + spanner_clients.must_include Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + end +end diff --git a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb index 1b330990dc1d..38ce4a3dcaa9 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb @@ -286,7 +286,6 @@ @mock.verify end - focus it "add endpoint uri to rollback" do api_call_stub = proc do |session_name, transaction_id, endpoint_uri: nil| session_name.must_equal session_path(instance_id, database_id, session_id) @@ -301,7 +300,6 @@ @mock.verify end - focus it "add endpoint uri to create_snapshot" do timestamp = Time.now api_call_stub = proc do |session_name, strong: nil, timestamp: nil, diff --git a/google-cloud-spanner/test/google/cloud/spanner/service_test.rb b/google-cloud-spanner/test/google/cloud/spanner/service_test.rb new file mode 100644 index 000000000000..65dba247b60f --- /dev/null +++ b/google-cloud-spanner/test/google/cloud/spanner/service_test.rb @@ -0,0 +1,65 @@ +# Copyright 2019 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "helper" + +describe Google::Cloud::Spanner::Service, :mock_spanner do + it "creates and cache service client connection based on endpoint uri" do + endpoint_uri = Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + api_call_count = 0 + spanner_client_newe_stub = proc do |credentials: nil, timeout: nil, + client_config: nil, service_address: nil, service_port: nil, + lib_name: nil, lib_version: nil| + service_address.must_equal endpoint_uri + api_call_count += 1 + OpenStruct.new(endpoint_uri: service_address) + end + + Google::Cloud::Spanner::V1::SpannerClient.stub :new, spanner_client_newe_stub do + spanner.service.service endpoint_uri + spanner_clients = spanner.service.instance_variable_get("@spanner_clients") + spanner_clients.must_include endpoint_uri + spanner_clients.length.must_equal 1 + api_call_count.must_equal 1 + + spanner.service.service endpoint_uri + api_call_count.must_equal 1 + end + end + + it "creates and cache service client connection for diffrent endpoint uris" do + endpoint_uris = ["name1.test-spanner.com", "nam2.test-spanner.com"] + api_call_count = 0 + spanner_client_newe_stub = proc do |credentials: nil, timeout: nil, + client_config: nil, service_address: nil, service_port: nil, + lib_name: nil, lib_version: nil| + endpoint_uris.must_include service_address + api_call_count += 1 + OpenStruct.new(endpoint_uri: service_address) + end + + Google::Cloud::Spanner::V1::SpannerClient.stub :new, spanner_client_newe_stub do + spanner.service.service endpoint_uris[0] + spanner.service.service endpoint_uris[1] + spanner_clients = spanner.service.instance_variable_get("@spanner_clients") + spanner_clients.length.must_equal 2 + + endpoint_uris.each_with_index do |endpoint_uri| + spanner_clients.must_include endpoint_uri + end + + api_call_count.must_equal 2 + end + end +end From acb179b1696e1c0e68e9f403ed1afe221098009b Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Thu, 26 Dec 2019 21:52:36 +0530 Subject: [PATCH 07/15] add warning message if permission denied for get endpoint uri --- .../lib/google/cloud/spanner/client_service_proxy.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb index dbc2373e0e8a..746535faafa8 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb @@ -48,8 +48,13 @@ def endpoint_uri return @endpoint_uri if @endpoint_uri if resource_based_routing_enabled? - instance = @project.instance @instance_id, fields: ["endpoint_uris"] - @endpoint_uri = instance.endpoint_uris.first if instance + begin + instance = @project.instance @instance_id, fields: ["endpoint_uris"] + @endpoint_uri = instance.endpoint_uris.first if instance + rescue Google::Cloud::PermissionDeniedError + warn "To use resource based routing please add" \ + "'spanner.instances.get' permission." + end end @endpoint_uri ||= host From efb233ac475dc3601dae479a1345d39d58a13ddb Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Mon, 30 Dec 2019 15:32:44 +0530 Subject: [PATCH 08/15] add test cases for get instance permission denied --- .../cloud/spanner/client_service_proxy.rb | 2 +- .../client_service_proxy/endpont_uri_test.rb | 29 +++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb index 746535faafa8..eb4339302bb7 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb @@ -53,7 +53,7 @@ def endpoint_uri @endpoint_uri = instance.endpoint_uris.first if instance rescue Google::Cloud::PermissionDeniedError warn "To use resource based routing please add" \ - "'spanner.instances.get' permission." + " 'spanner.instances.get' permission." end end diff --git a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb index e721f5cdf8e8..1248a91d08d9 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb @@ -21,21 +21,21 @@ ENV.delete "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING" end - it "gets default endpoint uri" do + it "returns default endpoint uri" do client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ spanner, instance_id client_service_proxy.endpoint_uri.must_equal \ Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS end - it "gets default endpoint uri if resource based routing disabled" do + it "returns default endpoint uri if resource based routing disabled" do client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ spanner, instance_id, enable_resource_based_routing: false client_service_proxy.endpoint_uri.must_equal \ Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS end - it "gets endpoint uri if resource based routing enabled" do + it "returns endpoint uri if resource based routing enabled" do get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ name: instance_path(instance_id), endpoint_uris: ["test.host.com"] mock = Minitest::Mock.new @@ -53,7 +53,7 @@ mock.verify end - it "gets endpoint uri if resource based routing enabled using an environment variable" do + it "returns endpoint uri if resource based routing enabled using an environment variable" do ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] = "YES" get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ @@ -73,7 +73,7 @@ mock.verify end - it "gets default endpoint uri if resource based routing enabled and instance endpoint uris not present" do + it "returns default endpoint uri if resource based routing enabled and instance endpoint uris not present" do get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ name: instance_path(instance_id), endpoint_uris: [] mock = Minitest::Mock.new @@ -91,4 +91,23 @@ mock.verify end + + it "returns default endpoint uri if get instance permission denied" do + stub = OpenStruct.new(api_call_count: 0) + + def stub.get_instance *args + self.api_call_count += 1 + gax_error = Google::Gax::GaxError.new "permission denied" + gax_error.instance_variable_set :@cause, GRPC::BadStatus.new(7, "permission denied") + raise gax_error + end + spanner.service.mocked_instances = stub + + client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ + spanner, instance_id, enable_resource_based_routing: true + + client_service_proxy.endpoint_uri.must_equal \ + Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + stub.api_call_count.must_equal 1 + end end From af73fe26190d4c791af6cfb0abc014b5e56ec036 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Wed, 22 Jan 2020 17:29:50 +0530 Subject: [PATCH 09/15] removed client_service_proxy and updated client, batch client - tests and doc changes --- .../acceptance/spanner/client_test.rb | 18 +- .../acceptance/spanner_helper.rb | 2 +- .../lib/google/cloud/spanner/batch_client.rb | 17 +- .../lib/google/cloud/spanner/client.rb | 61 ++-- .../cloud/spanner/client_service_proxy.rb | 230 ------------ .../lib/google/cloud/spanner/project.rb | 22 +- .../cloud/spanner/resource_based_routing.rb | 69 ++++ .../lib/google/cloud/spanner/results.rb | 4 +- .../lib/google/cloud/spanner/service.rb | 92 ++--- .../lib/google/cloud/spanner/session.rb | 5 +- .../support/doctest_helper.rb | 12 + .../google/cloud/spanner/client/admin_test.rb | 5 - .../client/resource_based_routing_test.rb | 137 +++++++ .../client_service_proxy/endpont_uri_test.rb | 113 ------ .../spanner/client_service_proxy_test.rb | 338 ------------------ .../test/google/cloud/spanner/service_test.rb | 65 ---- 16 files changed, 334 insertions(+), 856 deletions(-) delete mode 100644 google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb create mode 100644 google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb create mode 100644 google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb delete mode 100644 google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb delete mode 100644 google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb delete mode 100644 google-cloud-spanner/test/google/cloud/spanner/service_test.rb diff --git a/google-cloud-spanner/acceptance/spanner/client_test.rb b/google-cloud-spanner/acceptance/spanner/client_test.rb index 840bf0029f18..6dc477aebe4f 100644 --- a/google-cloud-spanner/acceptance/spanner/client_test.rb +++ b/google-cloud-spanner/acceptance/spanner/client_test.rb @@ -1,4 +1,4 @@ -# Copyright 2018 Google LLC +# Copyright 2020 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -21,16 +21,16 @@ let(:database_id) { $spanner_database_id } it "create client connection without resource based routing" do - spanner.client instance_id, database_id - spanner_clients = spanner.service.instance_variable_get("@spanner_clients") - spanner_clients.length.must_equal 1 - spanner_clients.must_include Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + client = spanner.client instance_id, database_id + client.service.host.must_equal Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS end it "create client connection with resource based routing" do - spanner.client instance_id, database_id, enable_resource_based_routing: true - spanner_clients = spanner.service.instance_variable_get("@spanner_clients") - spanner_clients.length.must_equal 1 - spanner_clients.must_include Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + client = spanner.client instance_id, database_id, enable_resource_based_routing: true + client.resource_based_routing_enabled?.must_equal true + instance = spanner.instance instance_id, fields: ["endpoint_uris"] + # currently instance not returning endpoint uris, set to default if no endpoint uri present. + host = instance.endpoint_uris.first || Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + client.service.host.must_equal host end end diff --git a/google-cloud-spanner/acceptance/spanner_helper.rb b/google-cloud-spanner/acceptance/spanner_helper.rb index 7040d38f1a0e..72b6c44deac1 100644 --- a/google-cloud-spanner/acceptance/spanner_helper.rb +++ b/google-cloud-spanner/acceptance/spanner_helper.rb @@ -256,7 +256,7 @@ def default_item_rows instance = $spanner.instance $spanner_instance_id instance ||= begin - inst_job = $spanner.create_instance $spanner_instance_id, name: "google-cloud-ruby-tests", config: "regional-us-central1", nodes: 1 + inst_job = $spanner.create_instance $spanner_instance_id, name: $spanner_instance_id, config: "regional-us-central1", nodes: 1 inst_job.wait_until_done! fail GRPC::BadStatus.new(inst_job.error.code, inst_job.error.message) if inst_job.error? inst_job.instance diff --git a/google-cloud-spanner/lib/google/cloud/spanner/batch_client.rb b/google-cloud-spanner/lib/google/cloud/spanner/batch_client.rb index bfea6e9fa218..c5682f44d4f5 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/batch_client.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/batch_client.rb @@ -17,6 +17,7 @@ require "google/cloud/spanner/project" require "google/cloud/spanner/session" require "google/cloud/spanner/batch_snapshot" +require "google/cloud/spanner/resource_based_routing" module Google module Cloud @@ -63,6 +64,9 @@ module Spanner # new_partition # class BatchClient + # @!parse extend ResourceBasedRouting + include ResourceBasedRouting + ## @private Service wrapper for batch data client api. # @return [ClientServiceProxy] attr_reader :service @@ -74,15 +78,18 @@ def initialize \ instance_id, database_id, session_labels: nil, - enable_resource_based_routing: nil + enable_resource_based_routing: false @project = project @instance_id = instance_id @database_id = database_id @session_labels = session_labels - @service = ClientServiceProxy.new \ - @project, - @instance_id, - enable_resource_based_routing: enable_resource_based_routing + @enable_resource_based_routing = enable_resource_based_routing + + if resource_based_routing_enabled? + @service = resource_based_routing_service + end + + @service ||= @project.service end # The unique identifier for the project. diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client.rb b/google-cloud-spanner/lib/google/cloud/spanner/client.rb index de018eebdf1f..aa6e383de8f5 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/client.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/client.rb @@ -23,7 +23,7 @@ require "google/cloud/spanner/range" require "google/cloud/spanner/column_value" require "google/cloud/spanner/convert" -require "google/cloud/spanner/client_service_proxy" +require "google/cloud/spanner/resource_based_routing" module Google module Cloud @@ -50,30 +50,57 @@ module Spanner # end # end # + # @example Enable resource based routing. + # + # require "google/cloud" + # + # spanner = Google::Cloud::Spanner.new + # + # db = spanner.client( + # "my-instance", + # "my-database", + # enable_resource_based_routing: true + # ) + # + # db.transaction do |tx| + # results = tx.execute_query "SELECT * FROM users" + # + # results.rows.each do |row| + # puts "User #{row[:id]} is #{row[:name]}" + # end + # end + # class Client - ## @private Service wrapper for data client api. - # @return [ClientServiceProxy] + # @!parse extend ResourceBasedRouting + include ResourceBasedRouting + + ## + # @private The Service object. + # @return [Spanner::Service] attr_reader :service ## # @private Creates a new Spanner Client instance. def initialize project, instance_id, database_id, session_labels: nil, - pool_opts: {}, enable_resource_based_routing: nil + pool_opts: {}, enable_resource_based_routing: false @project = project @instance_id = instance_id @database_id = database_id @session_labels = session_labels - @service = ClientServiceProxy.new \ - @project, - @instance_id, - enable_resource_based_routing: enable_resource_based_routing + @enable_resource_based_routing = enable_resource_based_routing + + if resource_based_routing_enabled? + @service = resource_based_routing_service + end + + @service ||= @project.service @pool = Pool.new self, pool_opts end # The unique identifier for the project. # @return [String] def project_id - @project.service.project + @service.project end # The unique identifier for the instance. @@ -929,8 +956,7 @@ def commit &block end end - # rubocop:disable Metrics/AbcSize - # rubocop:disable Metrics/MethodLength + # rubocop:disable Metrics/AbcSize,Metrics/MethodLength ## # Creates a transaction for reads and writes that execute atomically at @@ -1004,8 +1030,7 @@ def transaction deadline: 120 Thread.current[:transaction_id] = tx.transaction_id yield tx commit_resp = @service.commit \ - tx.session.path, tx.mutations, - transaction_id: tx.transaction_id + tx.session.path, tx.mutations, transaction_id: tx.transaction_id return Convert.timestamp_to_time commit_resp.commit_timestamp rescue GRPC::Aborted, Google::Cloud::AbortedError => err # Re-raise if deadline has passed @@ -1032,9 +1057,7 @@ def transaction deadline: 120 end end end - - # rubocop:enable Metrics/AbcSize - # rubocop:enable Metrics/MethodLength + # rubocop:enable Metrics/AbcSize,Metrics/MethodLength ## # Creates a snapshot read-only transaction for reads that execute @@ -1330,9 +1353,7 @@ def batch_create_sessions session_count ), session_count, labels: @session_labels - resp.session.map do |grpc| - Session.from_grpc grpc, @service - end + resp.session.map { |grpc| Session.from_grpc grpc, @service } end # @private @@ -1352,7 +1373,7 @@ def inspect # @private Raise an error unless an active connection to the service is # available. def ensure_service! - raise "Must have active connection to service" unless @project.service + raise "Must have active connection to service" unless @service end ## diff --git a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb b/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb deleted file mode 100644 index eb4339302bb7..000000000000 --- a/google-cloud-spanner/lib/google/cloud/spanner/client_service_proxy.rb +++ /dev/null @@ -1,230 +0,0 @@ -# Copyright 2019 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - - -module Google - module Cloud - module Spanner - ## - # @private - # - # # ClientServiceProxy - # - # Represents proxy Spanner service for resource based - # routing for data client. - class ClientServiceProxy < SimpleDelegator - # @private - # - # @param [Project] project - # @param [String] instance_id - # @param [true, false, nil] enable_resource_based_routing. - # Default resource-based routing is disabled. - # - def initialize project, instance_id, enable_resource_based_routing: nil - @project = project - @instance_id = instance_id - @enable_resource_based_routing = enable_resource_based_routing - - __setobj__ @project.service - end - - # Get instance endpoint uri. - # - # @return [String] If resource-based routing is enabled then returns - # first endpoint uri of instance, otherwise returns default - # global host uri. - def endpoint_uri - return @endpoint_uri if @endpoint_uri - - if resource_based_routing_enabled? - begin - instance = @project.instance @instance_id, fields: ["endpoint_uris"] - @endpoint_uri = instance.endpoint_uris.first if instance - rescue Google::Cloud::PermissionDeniedError - warn "To use resource based routing please add" \ - " 'spanner.instances.get' permission." - end - end - - @endpoint_uri ||= host - end - - def get_session session_name - super session_name, endpoint_uri: endpoint_uri - end - - def create_session database_name, labels: nil - super database_name, labels: labels, endpoint_uri: endpoint_uri - end - - def batch_create_sessions database_name, session_count, labels: nil - super \ - database_name, - session_count, - labels: labels, - endpoint_uri: endpoint_uri - end - - def delete_session session_name - super session_name, endpoint_uri: endpoint_uri - end - - def execute_streaming_sql \ - session_name, - sql, - transaction: nil, - params: nil, - types: nil, - resume_token: nil, - partition_token: nil, - seqno: nil - super \ - session_name, - sql, - transaction: transaction, - params: params, - types: types, - resume_token: resume_token, - partition_token: partition_token, - seqno: seqno, - endpoint_uri: endpoint_uri - end - - def execute_batch_dml session_name, transaction, statements, seqno - super \ - session_name, - transaction, - statements, - seqno, - endpoint_uri: endpoint_uri - end - - def streaming_read_table \ - session_name, - table_name, - columns, - keys: nil, - index: nil, - transaction: nil, - limit: nil, - resume_token: nil, - partition_token: nil - super \ - session_name, - table_name, - columns, - keys: keys, - index: index, - transaction: transaction, - limit: limit, - resume_token: resume_token, - partition_token: partition_token, - endpoint_uri: endpoint_uri - end - - def partition_read \ - session_name, - table_name, - columns, - transaction, - keys: nil, - index: nil, - partition_size_bytes: nil, - max_partitions: nil - super \ - session_name, - table_name, - columns, - transaction, - keys: keys, - index: index, - partition_size_bytes: partition_size_bytes, - max_partitions: max_partitions, - endpoint_uri: endpoint_uri - end - - def partition_query \ - session_name, - sql, - transaction, - params: nil, - types: nil, - partition_size_bytes: nil, - max_partitions: nil - super \ - session_name, - sql, - transaction, - params: params, - types: types, - partition_size_bytes: partition_size_bytes, - max_partitions: max_partitions, - endpoint_uri: endpoint_uri - end - - def commit session_name, mutations = [], transaction_id: nil - super \ - session_name, - mutations, - transaction_id: transaction_id, - endpoint_uri: endpoint_uri - end - - def rollback session_name, transaction_id - super session_name, transaction_id, endpoint_uri: endpoint_uri - end - - def begin_transaction session_name - super session_name, endpoint_uri: endpoint_uri - end - - def create_snapshot \ - session_name, - strong: nil, - timestamp: nil, - staleness: nil - super \ - session_name, - strong: strong, - timestamp: timestamp, - staleness: staleness, - endpoint_uri: endpoint_uri - end - - def create_pdml session_name - super session_name, endpoint_uri: endpoint_uri - end - - private - - # Check resource based routing is enabled or not. - # - # @return [Boolean] - def resource_based_routing_enabled? - case @enable_resource_based_routing - when TrueClass - true - when FalseClass - false - when NilClass - ["YES", "yes"].include? \ - ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] - else - false - end - end - end - end - end -end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/project.rb b/google-cloud-spanner/lib/google/cloud/spanner/project.rb index 448dd1ae2e1b..8475f5698768 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/project.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/project.rb @@ -461,10 +461,11 @@ def create_database instance_id, database_id, statements: [] # * Label values must be between 0 and 63 characters long and must # conform to the regular expression `([a-z]([-a-z0-9]*[a-z0-9])?)?`. # * No more than 64 labels can be associated with a given resource. - # @param [Boolean, nil] enable_resource_based_routing Enable/Disable - # resource-based routing for data operation, by default is disabled. - # Resource based routing can be enabled using the environment - # `GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING` to `YES` or `yes`. + # @param [Boolean] enable_resource_based_routing Enable/Disable + # resource-based routing for data operation, by default it is + # disabled. Resource based routing can be enabled using the + # environment `GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING` to + # `TRUE` or `true`. # @return [Client] The newly created client. # # @example @@ -503,7 +504,7 @@ def client \ database_id, pool: {}, labels: nil, - enable_resource_based_routing: nil + enable_resource_based_routing: false # Convert from possible Google::Protobuf::Map labels = Hash[labels.map { |k, v| [String(k), String(v)] }] if labels Client.new \ @@ -538,10 +539,11 @@ def client \ # * Label values must be between 0 and 63 characters long and must # conform to the regular expression `([a-z]([-a-z0-9]*[a-z0-9])?)?`. # * No more than 64 labels can be associated with a given resource. - # @param [Boolean, nil] enable_resource_based_routing Enable/Disable - # resource-based routing for data operation, by default is disabled. - # Resource based routing can be enabled using the environment - # `GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING` to `YES` or `yes`. + # @param [Boolean] enable_resource_based_routing Enable/Disable + # resource-based routing for data operation, by default it + # is disabled. Resource based routing can be enabled using the + # environment `GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING` to + # `TRUE` or `true`. # # @return [Client] The newly created client. # @@ -600,7 +602,7 @@ def batch_client \ instance_id, database_id, labels: nil, - enable_resource_based_routing: nil + enable_resource_based_routing: false # Convert from possible Google::Protobuf::Map labels = Hash[labels.map { |k, v| [String(k), String(v)] }] if labels BatchClient.new \ diff --git a/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb b/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb new file mode 100644 index 000000000000..107acb2869d1 --- /dev/null +++ b/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb @@ -0,0 +1,69 @@ +# Copyright 2020 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +require "time" +require "date" +require "stringio" +require "base64" +require "google/cloud/spanner/data" + +module Google + module Cloud + module Spanner + ## + # @private Helper module for resource based routing client. + module ResourceBasedRouting + # Check resource based routing is enabled or not. + # + # @return [Boolean] + def resource_based_routing_enabled? + return true if @enable_resource_based_routing + + ["TRUE", "true"].include? \ + ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] + end + + protected + + # Get Service instance using instance first endpoint uri. + # + # @return [Spanner::Service, nil] Returns service instance if instance + # endpoint uris present. + def resource_based_routing_service + instance = @project.instance @instance_id, fields: ["endpoint_uris"] + return if instance.nil? || instance.endpoint_uris.empty? + + Spanner::Service.new \ + @project.project_id, + @project.service.credentials, + host: instance.endpoint_uris.first, + timeout: @project.service.timeout, + client_config: @project.service.client_config + rescue Google::Cloud::PermissionDeniedError + warn <<~WARN + Warning: The client library attempted to connect to an endpoint + closer to your Cloud Spanner data but was unable to do so. + The client library will fall back and route requests to the global + Spanner endpoint (spanner.googleapis.com), which may result in + increased latency. We recommend including the scope + https://www.googleapis.com/auth/spanner.admin so that the client + library can get an instance-specific endpoint and efficiently route + requests. + WARN + end + end + end + end +end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/results.rb b/google-cloud-spanner/lib/google/cloud/spanner/results.rb index 1788fb702057..c4e2059be6d0 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/results.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/results.rb @@ -265,9 +265,7 @@ def self.read service, session_path, table, columns, keys: nil, transaction: transaction, partition_token: partition_token } enum = service.streaming_read_table \ - session_path, - table, columns, - read_options + session_path, table, columns, read_options from_enum(enum, service).tap do |results| results.instance_variable_set :@session_path, session_path results.instance_variable_set :@table, table diff --git a/google-cloud-spanner/lib/google/cloud/spanner/service.rb b/google-cloud-spanner/lib/google/cloud/spanner/service.rb index 1c0245fe0bca..0a19b5ada798 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/service.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/service.rb @@ -1,4 +1,3 @@ - # Copyright 2016 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -41,7 +40,6 @@ def initialize project, credentials, host: nil, timeout: nil, @host = host || V1::SpannerClient::SERVICE_ADDRESS @timeout = timeout @client_config = client_config || {} - @spanner_clients = {} end def channel @@ -60,23 +58,15 @@ def chan_creds GRPC::Core::CallCredentials.new credentials.client.updater_proc end - ## - # Create and cache spanner client connection based on instance - # endpoint uri for resource based request routing. - # - # @param [String, nil] endpoint_uri Instance endpoint uri. - # @return [V1::SpannerClient] - # - def service endpoint_uri = nil + def service return mocked_service if mocked_service - - @spanner_clients[endpoint_uri] ||= \ + @service ||= \ V1::SpannerClient.new( credentials: channel, timeout: timeout, client_config: client_config, - service_address: service_address(endpoint_uri), - service_port: service_port(endpoint_uri), + service_address: service_address, + service_port: service_port, lib_name: "gccl", lib_version: Google::Cloud::Spanner::VERSION ) @@ -275,18 +265,18 @@ def test_database_permissions instance_id, database_id, permissions end end - def get_session session_name, endpoint_uri: nil + def get_session session_name opts = default_options_from_session session_name execute do - service(endpoint_uri).get_session session_name, options: opts + service.get_session session_name, options: opts end end - def create_session database_name, labels: nil, endpoint_uri: nil + def create_session database_name, labels: nil opts = default_options_from_session database_name session = Google::Spanner::V1::Session.new labels: labels if labels execute do - service(endpoint_uri).create_session database_name, session: session, + service.create_session database_name, session: session, options: opts end end @@ -294,13 +284,12 @@ def create_session database_name, labels: nil, endpoint_uri: nil def batch_create_sessions \ database_name, session_count, - labels: nil, - endpoint_uri: nil + labels: nil opts = default_options_from_session database_name session = Google::Spanner::V1::Session.new labels: labels if labels execute do # The response may have fewer sessions than requested in the RPC. - service(endpoint_uri).batch_create_sessions \ + service.batch_create_sessions \ database_name, session_count, session_template: session, @@ -308,10 +297,10 @@ def batch_create_sessions \ end end - def delete_session session_name, endpoint_uri: nil + def delete_session session_name opts = default_options_from_session session_name execute do - service(endpoint_uri).delete_session session_name, options: opts + service.delete_session session_name, options: opts end end @@ -322,11 +311,10 @@ def execute_streaming_sql \ types: nil, resume_token: nil, partition_token: nil, - seqno: nil, - endpoint_uri: nil + seqno: nil opts = default_options_from_session session_name execute do - service(endpoint_uri).execute_streaming_sql \ + service.execute_streaming_sql \ session_name, sql, transaction: transaction, params: params, param_types: types, @@ -341,12 +329,11 @@ def execute_batch_dml \ session_name, transaction, statements, - seqno, - endpoint_uri: nil + seqno opts = default_options_from_session session_name statements = statements.map(&:to_grpc) results = execute do - service(endpoint_uri).execute_batch_dml \ + service.execute_batch_dml \ session_name, transaction, statements, @@ -373,11 +360,10 @@ def streaming_read_table \ transaction: nil, limit: nil, resume_token: nil, - partition_token: nil, - endpoint_uri: nil + partition_token: nil opts = default_options_from_session session_name execute do - service(endpoint_uri).streaming_read \ + service.streaming_read \ session_name, table_name, columns, keys, transaction: transaction, index: index, limit: limit, resume_token: resume_token, partition_token: partition_token, @@ -393,15 +379,14 @@ def partition_read \ keys: nil, index: nil, partition_size_bytes: nil, - max_partitions: nil, - endpoint_uri: nil + max_partitions: nil partition_opts = partition_options partition_size_bytes, max_partitions opts = default_options_from_session session_name execute do - service(endpoint_uri).partition_read \ + service.partition_read \ session_name, table_name, keys, transaction: transaction, index: index, columns: columns, partition_options: partition_opts, options: opts @@ -415,15 +400,14 @@ def partition_query \ params: nil, types: nil, partition_size_bytes: nil, - max_partitions: nil, - endpoint_uri: nil + max_partitions: nil partition_opts = partition_options partition_size_bytes, max_partitions opts = default_options_from_session session_name execute do - service(endpoint_uri).partition_query \ + service.partition_query \ session_name, sql, transaction: transaction, params: params, param_types: types, @@ -434,8 +418,7 @@ def partition_query \ def commit \ session_name, mutations = [], - transaction_id: nil, - endpoint_uri: nil + transaction_id: nil tx_opts = nil if transaction_id.nil? tx_opts = Google::Spanner::V1::TransactionOptions.new( @@ -444,7 +427,7 @@ def commit \ end opts = default_options_from_session session_name execute do - service(endpoint_uri).commit \ + service.commit \ session_name, mutations, transaction_id: transaction_id, single_use_transaction: tx_opts, @@ -452,20 +435,20 @@ def commit \ end end - def rollback session_name, transaction_id, endpoint_uri: nil + def rollback session_name, transaction_id opts = default_options_from_session session_name execute do - service(endpoint_uri).rollback session_name, transaction_id, options: opts + service.rollback session_name, transaction_id, options: opts end end - def begin_transaction session_name, endpoint_uri: nil + def begin_transaction session_name tx_opts = Google::Spanner::V1::TransactionOptions.new( read_write: Google::Spanner::V1::TransactionOptions::ReadWrite.new ) opts = default_options_from_session session_name execute do - service(endpoint_uri).begin_transaction session_name, tx_opts, options: opts + service.begin_transaction session_name, tx_opts, options: opts end end @@ -473,8 +456,7 @@ def create_snapshot \ session_name, strong: nil, timestamp: nil, - staleness: nil, - endpoint_uri: nil + staleness: nil tx_opts = Google::Spanner::V1::TransactionOptions.new( read_only: Google::Spanner::V1::TransactionOptions::ReadOnly.new( { @@ -487,18 +469,18 @@ def create_snapshot \ ) opts = default_options_from_session session_name execute do - service(endpoint_uri).begin_transaction session_name, tx_opts, options: opts + service.begin_transaction session_name, tx_opts, options: opts end end - def create_pdml session_name, endpoint_uri: nil + def create_pdml session_name tx_opts = Google::Spanner::V1::TransactionOptions.new( partitioned_dml: \ Google::Spanner::V1::TransactionOptions::PartitionedDml.new ) opts = default_options_from_session session_name execute do - service(endpoint_uri).begin_transaction \ + service.begin_transaction \ session_name, tx_opts, options: opts end end @@ -509,12 +491,14 @@ def inspect protected - def service_address uri = nil - URI.parse("//#{uri || host}").host + def service_address + return nil if host.nil? + URI.parse("//#{host}").host end - def service_port uri = nil - URI.parse("//#{uri || host}").port + def service_port + return nil if host.nil? + URI.parse("//#{host}").port end def default_options_from_session session_name diff --git a/google-cloud-spanner/lib/google/cloud/spanner/session.rb b/google-cloud-spanner/lib/google/cloud/spanner/session.rb index 7948d0b11e3f..62233e69b70d 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/session.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/session.rb @@ -404,9 +404,8 @@ def commit transaction_id: nil ensure_service! commit = Commit.new yield commit - commit_resp = service.commit \ - path, commit.mutations, - transaction_id: transaction_id + commit_resp = service.commit path, commit.mutations, + transaction_id: transaction_id @last_updated_at = Time.now Convert.timestamp_to_time commit_resp.commit_timestamp end diff --git a/google-cloud-spanner/support/doctest_helper.rb b/google-cloud-spanner/support/doctest_helper.rb index 0aaad587b53d..bd62bb0c8729 100644 --- a/google-cloud-spanner/support/doctest_helper.rb +++ b/google-cloud-spanner/support/doctest_helper.rb @@ -459,6 +459,18 @@ def mock_spanner end end + doctest.before "Google::Cloud::Spanner::Client@Enable resource based routing." do + mock_spanner do |mock, mock_instances, mock_databases| + mock_instances.expect :get_instance, OpenStruct.new(instance_hash), ["projects/my-project/instances/my-instance", Hash] + mock.expect :batch_create_sessions, OpenStruct.new(session: Array.new(10) { session_grpc }), ["projects/my-project/instances/my-instance/databases/my-database", 10, Hash] + 5.times do + mock.expect :begin_transaction, tx_resp, ["session-name", Google::Spanner::V1::TransactionOptions, Hash] + end + mock.expect :execute_streaming_sql, results_enum, ["session-name", "SELECT * FROM users", Hash] + mock.expect :commit, commit_resp, ["session-name", Array, Hash] + end + end + doctest.before "Google::Cloud::Spanner::Client#execute" do mock_spanner do |mock, mock_instances, mock_databases| mock.expect :batch_create_sessions, OpenStruct.new(session: Array.new(10) { session_grpc }), ["projects/my-project/instances/my-instance/databases/my-database", 10, Hash] diff --git a/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb index 67d4a5eac87e..9222e71eda41 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client/admin_test.rb @@ -73,9 +73,4 @@ database.database_id.must_equal database_id database.path.must_equal database_path(instance_id, database_id) end - - it "set service as a client service proxy" do - client.service.must_be_kind_of Google::Cloud::Spanner::Service - client.service.class.must_equal Google::Cloud::Spanner::ClientServiceProxy - end end diff --git a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb new file mode 100644 index 000000000000..b66c03052f00 --- /dev/null +++ b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb @@ -0,0 +1,137 @@ +# Copyright 2020 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "helper" + +describe Google::Cloud::Spanner::Client, :resource_based_routing, :mock_spanner do + let(:instance_id) { "my-instance-id" } + let(:database_id) { "my-database-id" } + let(:session_id) { "session123" } + let(:session_grpc) { Google::Spanner::V1::Session.new name: session_path(instance_id, database_id, session_id) } + let(:session) { Google::Cloud::Spanner::Session.from_grpc session_grpc, spanner.service } + let(:pool_opts) { { min: 0, max: 4 } } + + before do + session.instance_variable_set :@last_updated_at, Time.now + ENV.delete "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING" + end + + after do + ENV.delete "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING" + end + + + it "sets service with default host" do + client = Google::Cloud::Spanner::Client.new \ + spanner, instance_id, database_id, pool_opts: { min: 0, max: 4 } + + client.service.host.must_equal \ + Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + client.resource_based_routing_enabled?.must_equal false + end + + it "sets service host to default service uri if resource based routing disabled" do + client = Google::Cloud::Spanner::Client.new \ + spanner, instance_id, database_id, pool_opts: { min: 0, max: 4 }, + enable_resource_based_routing: false + + client.service.host.must_equal \ + Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + client.resource_based_routing_enabled?.must_equal false + end + + it "set service host to instance endpoint url if resource based routing enabled" do + get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ + name: instance_path(instance_id), endpoint_uris: ["test.host.com"] + mock = Minitest::Mock.new + mock.expect :get_instance, get_res, [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) + ] + spanner.service.mocked_instances = mock + + Google::Cloud::Spanner::Pool.stub :new, Object.new do + client = Google::Cloud::Spanner::Client.new \ + spanner, instance_id, database_id, enable_resource_based_routing: true + client.resource_based_routing_enabled?.must_equal true + client.service.host.must_equal "test.host.com" + end + + mock.verify + end + + it "set service host with instance endpoint url if resource based routing enabled using an environment variable" do + ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] = "TRUE" + + get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ + name: instance_path(instance_id), endpoint_uris: ["nam1-test.host.com"] + mock = Minitest::Mock.new + mock.expect :get_instance, get_res, [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) + ] + spanner.service.mocked_instances = mock + + Google::Cloud::Spanner::Pool.stub :new, Object.new do + client = Google::Cloud::Spanner::Client.new \ + spanner, instance_id, database_id, enable_resource_based_routing: true + client.resource_based_routing_enabled?.must_equal true + client.service.host.must_equal "nam1-test.host.com" + end + + mock.verify + end + + it "set default endpoint uri if resource based routing enabled and instance endpoint uris not present" do + get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ + name: instance_path(instance_id), endpoint_uris: [] + mock = Minitest::Mock.new + mock.expect :get_instance, get_res, [ + instance_path(instance_id), + field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) + ] + spanner.service.mocked_instances = mock + + Google::Cloud::Spanner::Pool.stub :new, Object.new do + client = Google::Cloud::Spanner::Client.new \ + spanner, instance_id, database_id, enable_resource_based_routing: true + client.service.host.must_equal \ + Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + client.resource_based_routing_enabled?.must_equal true + end + + mock.verify + end + + it "returns default endpoint uri if get instance permission denied" do + stub = OpenStruct.new(api_call_count: 0) + + def stub.get_instance *args + self.api_call_count += 1 + gax_error = Google::Gax::GaxError.new "permission denied" + gax_error.instance_variable_set :@cause, GRPC::BadStatus.new(7, "permission denied") + raise gax_error + end + spanner.service.mocked_instances = stub + + Google::Cloud::Spanner::Pool.stub :new, Object.new do + client = Google::Cloud::Spanner::Client.new \ + spanner, instance_id, database_id, enable_resource_based_routing: true + client.service.host.must_equal \ + Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS + stub.api_call_count.must_equal 1 + client.resource_based_routing_enabled?.must_equal true + end + end +end diff --git a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb deleted file mode 100644 index 1248a91d08d9..000000000000 --- a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy/endpont_uri_test.rb +++ /dev/null @@ -1,113 +0,0 @@ -# Copyright 2016 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -require "helper" - -describe Google::Cloud::Spanner::ClientServiceProxy, :endpoint_uri, :mock_spanner do - let(:instance_id) { "my-instance-id" } - - after do - ENV.delete "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING" - end - - it "returns default endpoint uri" do - client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ - spanner, instance_id - client_service_proxy.endpoint_uri.must_equal \ - Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS - end - - it "returns default endpoint uri if resource based routing disabled" do - client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ - spanner, instance_id, enable_resource_based_routing: false - client_service_proxy.endpoint_uri.must_equal \ - Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS - end - - it "returns endpoint uri if resource based routing enabled" do - get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ - name: instance_path(instance_id), endpoint_uris: ["test.host.com"] - mock = Minitest::Mock.new - mock.expect :get_instance, get_res, [ - instance_path(instance_id), - field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) - ] - spanner.service.mocked_instances = mock - - client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ - spanner, instance_id, enable_resource_based_routing: true - - client_service_proxy.endpoint_uri.must_equal "test.host.com" - - mock.verify - end - - it "returns endpoint uri if resource based routing enabled using an environment variable" do - ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] = "YES" - - get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ - name: instance_path(instance_id), endpoint_uris: ["test.host.com"] - mock = Minitest::Mock.new - mock.expect :get_instance, get_res, [ - instance_path(instance_id), - field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) - ] - spanner.service.mocked_instances = mock - - client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ - spanner, instance_id, enable_resource_based_routing: true - - client_service_proxy.endpoint_uri.must_equal "test.host.com" - - mock.verify - end - - it "returns default endpoint uri if resource based routing enabled and instance endpoint uris not present" do - get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ - name: instance_path(instance_id), endpoint_uris: [] - mock = Minitest::Mock.new - mock.expect :get_instance, get_res, [ - instance_path(instance_id), - field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) - ] - spanner.service.mocked_instances = mock - - client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ - spanner, instance_id, enable_resource_based_routing: true - - client_service_proxy.endpoint_uri.must_equal \ - Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS - - mock.verify - end - - it "returns default endpoint uri if get instance permission denied" do - stub = OpenStruct.new(api_call_count: 0) - - def stub.get_instance *args - self.api_call_count += 1 - gax_error = Google::Gax::GaxError.new "permission denied" - gax_error.instance_variable_set :@cause, GRPC::BadStatus.new(7, "permission denied") - raise gax_error - end - spanner.service.mocked_instances = stub - - client_service_proxy = Google::Cloud::Spanner::ClientServiceProxy.new \ - spanner, instance_id, enable_resource_based_routing: true - - client_service_proxy.endpoint_uri.must_equal \ - Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS - stub.api_call_count.must_equal 1 - end -end diff --git a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb deleted file mode 100644 index 38ce4a3dcaa9..000000000000 --- a/google-cloud-spanner/test/google/cloud/spanner/client_service_proxy_test.rb +++ /dev/null @@ -1,338 +0,0 @@ -# Copyright 2019 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -require "helper" - -describe Google::Cloud::Spanner::ClientServiceProxy, :mock_spanner do - let(:instance_id) { "my-instance-id" } - let(:database_id) { "my-database-id" } - let(:session_id) { "session123" } - let(:session_name) { session_path(instance_id, database_id, session_id) } - let(:client_service_proxy) { - Google::Cloud::Spanner::ClientServiceProxy.new \ - spanner, instance_id, enable_resource_based_routing: true - } - let(:instance_endpoint_uri) { "test.host.com" } - let(:get_instance_req) { - [ - instance_path(instance_id), - field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) - ] - } - let(:get_insance_res){ - Google::Spanner::Admin::Instance::V1::Instance.new \ - name: instance_path(instance_id), endpoint_uris: [instance_endpoint_uri] - } - let(:statement){ - Google::Spanner::V1::ExecuteBatchDmlRequest::Statement.new \ - sql: "UPDATE users SET age = @age", - params: Google::Protobuf::Struct.new(fields: - { "age" => Google::Protobuf::Value.new(string_value: "29") } - ), - param_types: { "age" => Google::Spanner::V1::Type.new(code: :INT64) } - } - let(:tx_selector) { - Google::Spanner::V1::TransactionSelector.new id: "tx123" - } - - it "knows the identifiers" do - client_service_proxy.must_be_kind_of Google::Cloud::Spanner::Service - client_service_proxy.instance_variable_get("@project").must_equal spanner - client_service_proxy.instance_variable_get("@instance_id").must_equal instance_id - client_service_proxy.instance_variable_get("@enable_resource_based_routing").must_equal true - end - - describe "api calls" do - before :each do - @mock = Minitest::Mock.new - @mock.expect :get_instance, get_insance_res, [ - instance_path(instance_id), - field_mask: Google::Protobuf::FieldMask.new(paths: ["endpoint_uris"]) - ] - spanner.service.mocked_instances = @mock - end - - it "add endpoint uri to get_session" do - api_call_stub = proc do |session_name, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :get_session, api_call_stub do - client_service_proxy.get_session session_name - end - - @mock.verify - end - - it "add endpoint uri to create_session" do - api_call_stub = proc do |database_name, labels: nil, endpoint_uri: nil| - database_name.must_equal database_path(instance_id, database_id) - labels.must_equal ["test-session"] - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :create_session, api_call_stub do - client_service_proxy.create_session \ - database_path(instance_id, database_id), labels: ["test-session"] - end - - @mock.verify - end - - it "add endpoint uri to batch_create_sessions" do - api_call_stub = proc do |database_name, session_count, labels: nil, endpoint_uri: nil| - database_name.must_equal database_path(instance_id, database_id) - session_count.must_equal 5 - labels.must_equal ["test-session"] - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :batch_create_sessions, api_call_stub do - client_service_proxy.batch_create_sessions \ - database_path(instance_id, database_id), 5, labels: ["test-session"] - end - - @mock.verify - end - - it "add endpoint uri to delete_session" do - api_call_stub = proc do |session_name, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :delete_session, api_call_stub do - client_service_proxy.delete_session session_name - end - - @mock.verify - end - - it "add endpoint uri to execute_streaming_sql" do - api_call_stub = proc do |session_name, sql, transaction: nil, - params: nil, types: nil, resume_token: nil, partition_token: nil, - seqno: nil, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - sql.must_equal statement.sql - transaction.must_equal tx_selector - params.must_equal statement.params - types.must_equal statement.param_types - resume_token.must_equal "test-resume-token" - partition_token.must_equal "test-partition-token" - seqno.must_equal 1 - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :execute_streaming_sql, api_call_stub do - client_service_proxy.execute_streaming_sql \ - session_name, - statement.sql, - transaction: tx_selector, - params: statement.params, - types: statement.param_types, - resume_token: "test-resume-token", - partition_token: "test-partition-token", - seqno: 1 - end - - @mock.verify - end - - it "add endpoint uri to execute_batch_dml" do - api_call_stub = proc do |session_name, transaction, statements, seqno, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - transaction.must_equal tx_selector - statements.must_equal [statement] - seqno.must_equal 1 - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :execute_batch_dml, api_call_stub do - client_service_proxy.execute_batch_dml \ - session_name, tx_selector, [statement], 1 - end - - @mock.verify - end - - it "add endpoint uri to streaming_read_table" do - api_call_stub = proc do |session_name, table_name, columns, - keys: nil, index: nil, transaction: nil, limit: nil, resume_token: nil, - partition_token: nil, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - table_name.must_equal "users" - columns.must_equal ["id", "name"] - keys.must_equal Google::Spanner::V1::KeySet.new(all: true) - index.must_equal "id" - transaction.must_equal tx_selector - limit.must_equal 100 - resume_token.must_equal "test-resume-token" - partition_token.must_equal "test-partition-token" - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :streaming_read_table, api_call_stub do - client_service_proxy.streaming_read_table \ - session_name, "users", ["id", "name"], - keys: Google::Spanner::V1::KeySet.new(all: true), - index: "id", - transaction: tx_selector, - limit: 100, - resume_token: "test-resume-token", - partition_token: "test-partition-token" - end - - @mock.verify - end - - it "add endpoint uri to partition_read" do - api_call_stub = proc do |session_name, table_name, columns, transaction, - keys: nil, index: nil, partition_size_bytes: nil, max_partitions: nil, - endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - table_name.must_equal "users" - columns.must_equal ["id", "name"] - transaction.must_equal tx_selector - keys.must_equal Google::Spanner::V1::KeySet.new(all: true) - index.must_equal "id" - partition_size_bytes.must_equal 1024 - max_partitions.must_equal 3 - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :partition_read, api_call_stub do - client_service_proxy.partition_read \ - session_name, "users", ["id", "name"], tx_selector, - keys: Google::Spanner::V1::KeySet.new(all: true), - index: "id", - partition_size_bytes: 1024, - max_partitions: 3 - end - - @mock.verify - end - - it "add endpoint uri to partition_query" do - api_call_stub = proc do |session_name, sql, transaction, params: nil, - types: nil, partition_size_bytes: nil, max_partitions: nil, - endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - sql.must_equal statement.sql - transaction.must_equal tx_selector - params.must_equal statement.params - types.must_equal statement.param_types - partition_size_bytes.must_equal 2048 - max_partitions.must_equal 5 - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :partition_query, api_call_stub do - client_service_proxy.partition_query \ - session_name, statement.sql, tx_selector, - params: statement.params, - types: statement.param_types, - partition_size_bytes: 2048, - max_partitions: 5 - end - - @mock.verify - end - - it "add endpoint uri to begin_transaction" do - api_call_stub = proc do |session_name, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :begin_transaction, api_call_stub do - client_service_proxy.begin_transaction session_name - end - - @mock.verify - end - - it "add endpoint uri to commit" do - req_mutations = [ - Google::Spanner::V1::Mutation.new( - update: Google::Spanner::V1::Mutation::Write.new( - table: "users", columns: %w(id name active), - values: [Google::Cloud::Spanner::Convert.object_to_grpc_value([1, "Charlie", false]).list_value] - ) - ) - ] - api_call_stub = proc do |session_name, mutations, transaction_id: nil, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - mutations.must_equal req_mutations - transaction_id.must_equal "txn123" - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :commit, api_call_stub do - client_service_proxy.commit session_name, req_mutations, transaction_id: "txn123" - end - - @mock.verify - end - - it "add endpoint uri to rollback" do - api_call_stub = proc do |session_name, transaction_id, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - transaction_id.must_equal "txn123" - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :rollback, api_call_stub do - client_service_proxy.rollback session_name, "txn123" - end - - @mock.verify - end - - it "add endpoint uri to create_snapshot" do - timestamp = Time.now - api_call_stub = proc do |session_name, strong: nil, timestamp: nil, - staleness: nil, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - strong.must_equal true - timestamp.must_equal timestamp - staleness.must_equal 30 - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :create_snapshot, api_call_stub do - client_service_proxy.create_snapshot \ - session_name, - strong: true, - timestamp: timestamp, - staleness: 30 - end - - @mock.verify - end - - it "add endpoint uri to create_pdml" do - api_call_stub = proc do |session_name, endpoint_uri: nil| - session_name.must_equal session_path(instance_id, database_id, session_id) - endpoint_uri.must_equal instance_endpoint_uri - end - - spanner.service.stub :create_pdml, api_call_stub do - client_service_proxy.create_pdml session_name - end - - @mock.verify - end - end -end diff --git a/google-cloud-spanner/test/google/cloud/spanner/service_test.rb b/google-cloud-spanner/test/google/cloud/spanner/service_test.rb deleted file mode 100644 index 65dba247b60f..000000000000 --- a/google-cloud-spanner/test/google/cloud/spanner/service_test.rb +++ /dev/null @@ -1,65 +0,0 @@ -# Copyright 2019 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# https://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -require "helper" - -describe Google::Cloud::Spanner::Service, :mock_spanner do - it "creates and cache service client connection based on endpoint uri" do - endpoint_uri = Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS - api_call_count = 0 - spanner_client_newe_stub = proc do |credentials: nil, timeout: nil, - client_config: nil, service_address: nil, service_port: nil, - lib_name: nil, lib_version: nil| - service_address.must_equal endpoint_uri - api_call_count += 1 - OpenStruct.new(endpoint_uri: service_address) - end - - Google::Cloud::Spanner::V1::SpannerClient.stub :new, spanner_client_newe_stub do - spanner.service.service endpoint_uri - spanner_clients = spanner.service.instance_variable_get("@spanner_clients") - spanner_clients.must_include endpoint_uri - spanner_clients.length.must_equal 1 - api_call_count.must_equal 1 - - spanner.service.service endpoint_uri - api_call_count.must_equal 1 - end - end - - it "creates and cache service client connection for diffrent endpoint uris" do - endpoint_uris = ["name1.test-spanner.com", "nam2.test-spanner.com"] - api_call_count = 0 - spanner_client_newe_stub = proc do |credentials: nil, timeout: nil, - client_config: nil, service_address: nil, service_port: nil, - lib_name: nil, lib_version: nil| - endpoint_uris.must_include service_address - api_call_count += 1 - OpenStruct.new(endpoint_uri: service_address) - end - - Google::Cloud::Spanner::V1::SpannerClient.stub :new, spanner_client_newe_stub do - spanner.service.service endpoint_uris[0] - spanner.service.service endpoint_uris[1] - spanner_clients = spanner.service.instance_variable_get("@spanner_clients") - spanner_clients.length.must_equal 2 - - endpoint_uris.each_with_index do |endpoint_uri| - spanner_clients.must_include endpoint_uri - end - - api_call_count.must_equal 2 - end - end -end From 13eb5cde1f3f9dcf81be46732ca4c3a814fc48d1 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Thu, 23 Jan 2020 06:57:55 +0530 Subject: [PATCH 10/15] fix docs and updated test cases, reformat service code --- .../acceptance/spanner/client_test.rb | 2 +- .../lib/google/cloud/spanner/project.rb | 4 +- .../cloud/spanner/resource_based_routing.rb | 12 ++-- .../lib/google/cloud/spanner/service.rb | 72 +++++-------------- .../client/resource_based_routing_test.rb | 12 ++-- 5 files changed, 33 insertions(+), 69 deletions(-) diff --git a/google-cloud-spanner/acceptance/spanner/client_test.rb b/google-cloud-spanner/acceptance/spanner/client_test.rb index 6dc477aebe4f..111388db6175 100644 --- a/google-cloud-spanner/acceptance/spanner/client_test.rb +++ b/google-cloud-spanner/acceptance/spanner/client_test.rb @@ -29,7 +29,7 @@ client = spanner.client instance_id, database_id, enable_resource_based_routing: true client.resource_based_routing_enabled?.must_equal true instance = spanner.instance instance_id, fields: ["endpoint_uris"] - # currently instance not returning endpoint uris, set to default if no endpoint uri present. + # Set to default if no endpoint uri present. host = instance.endpoint_uris.first || Google::Cloud::Spanner::V1::SpannerClient::SERVICE_ADDRESS client.service.host.must_equal host end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/project.rb b/google-cloud-spanner/lib/google/cloud/spanner/project.rb index 8475f5698768..0050b3b6fb07 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/project.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/project.rb @@ -136,7 +136,7 @@ def instances token: nil, max: nil # # @param [String] instance_id The unique identifier for the instance. # @param [Array] fields Specifies the subset of fields that - # should be returned. dIf fields not provided then all fields will + # should be returned. If fields not provided then all fields will be # returned. Optional. # @return [Google::Cloud::Spanner::Instance, nil] The instance, or `nil` # if the instance does not exist. @@ -153,7 +153,7 @@ def instances token: nil, max: nil # spanner = Google::Cloud::Spanner.new # instance = spanner.instance "non-existing" # nil # - # @example Get instance detail with specified fields. + # @example Get instance details with specified fields. # require "google/cloud/spanner" # # spanner = Google::Cloud::Spanner.new diff --git a/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb b/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb index 107acb2869d1..660ef6ea2747 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb @@ -53,14 +53,14 @@ def resource_based_routing_service client_config: @project.service.client_config rescue Google::Cloud::PermissionDeniedError warn <<~WARN - Warning: The client library attempted to connect to an endpoint + The client library attempted to connect to an endpoint closer to your Cloud Spanner data but was unable to do so. - The client library will fall back and route requests to the global - Spanner endpoint (spanner.googleapis.com), which may result in + The client library will fallback and route requests to the + endpoint given in the client options, which may result in increased latency. We recommend including the scope - https://www.googleapis.com/auth/spanner.admin so that the client - library can get an instance-specific endpoint and efficiently route - requests. + https://www.googleapis.com/auth/spanner.admin so that + the client library can get an instance-specific endpoint + and efficiently route requests. WARN end end diff --git a/google-cloud-spanner/lib/google/cloud/spanner/service.rb b/google-cloud-spanner/lib/google/cloud/spanner/service.rb index 0a19b5ada798..acb69f76aee5 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/service.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/service.rb @@ -277,14 +277,11 @@ def create_session database_name, labels: nil session = Google::Spanner::V1::Session.new labels: labels if labels execute do service.create_session database_name, session: session, - options: opts + options: opts end end - def batch_create_sessions \ - database_name, - session_count, - labels: nil + def batch_create_sessions database_name, session_count, labels: nil opts = default_options_from_session database_name session = Google::Spanner::V1::Session.new labels: labels if labels execute do @@ -304,14 +301,9 @@ def delete_session session_name end end - def execute_streaming_sql \ - session_name, sql, - transaction: nil, - params: nil, - types: nil, - resume_token: nil, - partition_token: nil, - seqno: nil + def execute_streaming_sql session_name, sql, transaction: nil, + params: nil, types: nil, resume_token: nil, + partition_token: nil, seqno: nil opts = default_options_from_session session_name execute do service.execute_streaming_sql \ @@ -325,11 +317,7 @@ def execute_streaming_sql \ end end - def execute_batch_dml \ - session_name, - transaction, - statements, - seqno + def execute_batch_dml session_name, transaction, statements, seqno opts = default_options_from_session session_name statements = statements.map(&:to_grpc) results = execute do @@ -351,16 +339,9 @@ def execute_batch_dml \ end end - def streaming_read_table \ - session_name, - table_name, - columns, - keys: nil, - index: nil, - transaction: nil, - limit: nil, - resume_token: nil, - partition_token: nil + def streaming_read_table session_name, table_name, columns, keys: nil, + index: nil, transaction: nil, limit: nil, + resume_token: nil, partition_token: nil opts = default_options_from_session session_name execute do service.streaming_read \ @@ -371,15 +352,9 @@ def streaming_read_table \ end end - def partition_read \ - session_name, - table_name, - columns, - transaction, - keys: nil, - index: nil, - partition_size_bytes: nil, - max_partitions: nil + def partition_read session_name, table_name, columns, transaction, + keys: nil, index: nil, partition_size_bytes: nil, + max_partitions: nil partition_opts = partition_options partition_size_bytes, max_partitions @@ -393,14 +368,9 @@ def partition_read \ end end - def partition_query \ - session_name, - sql, - transaction, - params: nil, - types: nil, - partition_size_bytes: nil, - max_partitions: nil + def partition_query session_name, sql, transaction, params: nil, + types: nil, partition_size_bytes: nil, + max_partitions: nil partition_opts = partition_options partition_size_bytes, max_partitions @@ -415,10 +385,7 @@ def partition_query \ end end - def commit \ - session_name, - mutations = [], - transaction_id: nil + def commit session_name, mutations = [], transaction_id: nil tx_opts = nil if transaction_id.nil? tx_opts = Google::Spanner::V1::TransactionOptions.new( @@ -452,11 +419,8 @@ def begin_transaction session_name end end - def create_snapshot \ - session_name, - strong: nil, - timestamp: nil, - staleness: nil + def create_snapshot session_name, strong: nil, timestamp: nil, + staleness: nil tx_opts = Google::Spanner::V1::TransactionOptions.new( read_only: Google::Spanner::V1::TransactionOptions::ReadOnly.new( { diff --git a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb index b66c03052f00..b4d9a05d03a6 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb @@ -51,9 +51,9 @@ client.resource_based_routing_enabled?.must_equal false end - it "set service host to instance endpoint url if resource based routing enabled" do + it "set service host to instance endpoint uri if resource based routing enabled" do get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ - name: instance_path(instance_id), endpoint_uris: ["test.host.com"] + name: instance_path(instance_id), endpoint_uris: ["test1.host.com", "test2.host.com"] mock = Minitest::Mock.new mock.expect :get_instance, get_res, [ instance_path(instance_id), @@ -65,17 +65,17 @@ client = Google::Cloud::Spanner::Client.new \ spanner, instance_id, database_id, enable_resource_based_routing: true client.resource_based_routing_enabled?.must_equal true - client.service.host.must_equal "test.host.com" + client.service.host.must_equal "test1.host.com" end mock.verify end - it "set service host with instance endpoint url if resource based routing enabled using an environment variable" do + it "set service host with instance endpoint uri if resource based routing enabled using an environment variable" do ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] = "TRUE" get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ - name: instance_path(instance_id), endpoint_uris: ["nam1-test.host.com"] + name: instance_path(instance_id), endpoint_uris: ["test1.host.com", "test2.host.com"] mock = Minitest::Mock.new mock.expect :get_instance, get_res, [ instance_path(instance_id), @@ -87,7 +87,7 @@ client = Google::Cloud::Spanner::Client.new \ spanner, instance_id, database_id, enable_resource_based_routing: true client.resource_based_routing_enabled?.must_equal true - client.service.host.must_equal "nam1-test.host.com" + client.service.host.must_equal "test1.host.com" end mock.verify From 253f59a0ab508141b1fdb59b3ff931a715969664 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Thu, 23 Jan 2020 07:15:39 +0530 Subject: [PATCH 11/15] fix doc --- google-cloud-spanner/lib/google/cloud/spanner/project.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/lib/google/cloud/spanner/project.rb b/google-cloud-spanner/lib/google/cloud/spanner/project.rb index 0050b3b6fb07..4123de43583e 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/project.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/project.rb @@ -136,8 +136,8 @@ def instances token: nil, max: nil # # @param [String] instance_id The unique identifier for the instance. # @param [Array] fields Specifies the subset of fields that - # should be returned. If fields not provided then all fields will be - # returned. Optional. + # should be returned. If fields are not provided then all fields will + # be returned. Optional. # @return [Google::Cloud::Spanner::Instance, nil] The instance, or `nil` # if the instance does not exist. # From 1fc45f0194199a40cc2f461060f05583779121c2 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Fri, 24 Jan 2020 10:44:23 +0530 Subject: [PATCH 12/15] fix typos and changes in instance get permission denied test --- google-cloud-spanner/acceptance/spanner/instance_test.rb | 2 +- .../lib/google/cloud/spanner/resource_based_routing.rb | 8 +------- .../cloud/spanner/client/resource_based_routing_test.rb | 6 +++--- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/google-cloud-spanner/acceptance/spanner/instance_test.rb b/google-cloud-spanner/acceptance/spanner/instance_test.rb index 52d931124470..838b657ca0db 100644 --- a/google-cloud-spanner/acceptance/spanner/instance_test.rb +++ b/google-cloud-spanner/acceptance/spanner/instance_test.rb @@ -39,7 +39,7 @@ instance.state.must_equal :READY end - it "get speicified instance fields" do + it "get specified instance fields" do instance = spanner.instance instance_id, fields: ["name"] instance.instance_id.must_equal instance_id diff --git a/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb b/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb index 660ef6ea2747..46e7fa237eb6 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb @@ -13,12 +13,6 @@ # limitations under the License. -require "time" -require "date" -require "stringio" -require "base64" -require "google/cloud/spanner/data" - module Google module Cloud module Spanner @@ -37,7 +31,7 @@ def resource_based_routing_enabled? protected - # Get Service instance using instance first endpoint uri. + # Returns a Service that uses the first endpoint uri for the instance. # # @return [Spanner::Service, nil] Returns service instance if instance # endpoint uris present. diff --git a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb index b4d9a05d03a6..e44259f40499 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb @@ -119,9 +119,9 @@ def stub.get_instance *args self.api_call_count += 1 - gax_error = Google::Gax::GaxError.new "permission denied" - gax_error.instance_variable_set :@cause, GRPC::BadStatus.new(7, "permission denied") - raise gax_error + raise Google::Cloud::Error.from_error( + GRPC::PermissionDenied.new "permission denied" + ) end spanner.service.mocked_instances = stub From d207e43a4de7aa073794a2d358d2b13f3f21758a Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Sat, 8 Feb 2020 10:52:33 +0530 Subject: [PATCH 13/15] update resource based routing env variable name --- google-cloud-spanner/lib/google/cloud/spanner/project.rb | 4 ++-- .../lib/google/cloud/spanner/resource_based_routing.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-spanner/lib/google/cloud/spanner/project.rb b/google-cloud-spanner/lib/google/cloud/spanner/project.rb index 4123de43583e..e1ff181792ec 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/project.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/project.rb @@ -464,7 +464,7 @@ def create_database instance_id, database_id, statements: [] # @param [Boolean] enable_resource_based_routing Enable/Disable # resource-based routing for data operation, by default it is # disabled. Resource based routing can be enabled using the - # environment `GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING` to + # environment `GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING` to # `TRUE` or `true`. # @return [Client] The newly created client. # @@ -542,7 +542,7 @@ def client \ # @param [Boolean] enable_resource_based_routing Enable/Disable # resource-based routing for data operation, by default it # is disabled. Resource based routing can be enabled using the - # environment `GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING` to + # environment `GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING` to # `TRUE` or `true`. # # @return [Client] The newly created client. diff --git a/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb b/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb index 46e7fa237eb6..2f09f8a5d4f3 100644 --- a/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb +++ b/google-cloud-spanner/lib/google/cloud/spanner/resource_based_routing.rb @@ -26,7 +26,7 @@ def resource_based_routing_enabled? return true if @enable_resource_based_routing ["TRUE", "true"].include? \ - ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] + ENV["GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING"] end protected From 83bbd8cd5644f7a389ff2a3f767a4a12bec93f4f Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Sat, 8 Feb 2020 10:56:37 +0530 Subject: [PATCH 14/15] fix test case --- .../google/cloud/spanner/client/resource_based_routing_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb index e44259f40499..2d8ab733c094 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb @@ -72,7 +72,7 @@ end it "set service host with instance endpoint uri if resource based routing enabled using an environment variable" do - ENV["GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"] = "TRUE" + ENV["GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING"] = "TRUE" get_res = Google::Spanner::Admin::Instance::V1::Instance.new \ name: instance_path(instance_id), endpoint_uris: ["test1.host.com", "test2.host.com"] From 6af2a4d1f3389ad4fb5b5308b7a8975fc2be3498 Mon Sep 17 00:00:00 2001 From: Jiren Patel Date: Sun, 9 Feb 2020 20:03:55 +0530 Subject: [PATCH 15/15] fix test cases --- .../cloud/spanner/client/resource_based_routing_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb index 2d8ab733c094..585b8f26a243 100644 --- a/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb +++ b/google-cloud-spanner/test/google/cloud/spanner/client/resource_based_routing_test.rb @@ -24,11 +24,11 @@ before do session.instance_variable_set :@last_updated_at, Time.now - ENV.delete "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING" + ENV.delete "GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING" end after do - ENV.delete "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING" + ENV.delete "GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING" end