From 5470452e7b446e25b59d92ae20eed2960b53ed33 Mon Sep 17 00:00:00 2001 From: Artem Date: Sun, 22 Jul 2018 15:29:26 +1000 Subject: [PATCH] Refactor compute get methods This should fix both #33 and #352 for compute --- lib/fog/compute/google/models/addresses.rb | 12 ++++++--- .../compute/google/models/backend_services.rb | 5 ++-- lib/fog/compute/google/models/disk_types.rb | 11 ++++---- lib/fog/compute/google/models/disks.rb | 14 +++++++--- lib/fog/compute/google/models/firewalls.rb | 5 ++-- .../compute/google/models/forwarding_rules.rb | 10 +++---- .../compute/google/models/global_addresses.rb | 5 ++-- .../google/models/global_forwarding_rules.rb | 5 ++-- .../google/models/http_health_checks.rb | 7 ++--- lib/fog/compute/google/models/images.rb | 27 ++++++++++--------- .../google/models/instance_group_managers.rb | 12 +++++---- .../compute/google/models/instance_groups.rb | 18 ++++++------- .../google/models/instance_templates.rb | 5 ++-- .../compute/google/models/machine_types.rb | 17 +++++++++--- lib/fog/compute/google/models/networks.rb | 5 ++-- lib/fog/compute/google/models/operations.rb | 14 +++++----- lib/fog/compute/google/models/projects.rb | 5 ++-- lib/fog/compute/google/models/regions.rb | 5 ++-- lib/fog/compute/google/models/routes.rb | 5 ++-- lib/fog/compute/google/models/servers.rb | 14 +++++----- lib/fog/compute/google/models/snapshots.rb | 9 ++++--- .../compute/google/models/ssl_certificates.rb | 7 ++--- lib/fog/compute/google/models/subnetworks.rb | 12 ++++++--- .../google/models/target_http_proxies.rb | 2 +- .../google/models/target_https_proxies.rb | 5 ++-- .../compute/google/models/target_instances.rb | 16 +++++------ lib/fog/compute/google/models/target_pools.rb | 17 +++++------- lib/fog/compute/google/models/url_maps.rb | 5 ++-- lib/fog/compute/google/models/zones.rb | 6 +++-- test/helpers/test_collection.rb | 2 -- test/unit/compute/test_common_collections.rb | 2 -- 31 files changed, 159 insertions(+), 125 deletions(-) diff --git a/lib/fog/compute/google/models/addresses.rb b/lib/fog/compute/google/models/addresses.rb index adb5470d72..dcbef112d2 100755 --- a/lib/fog/compute/google/models/addresses.rb +++ b/lib/fog/compute/google/models/addresses.rb @@ -23,9 +23,15 @@ def all(region: nil, filter: nil, max_results: nil, order_by: nil, page_token: n load(data.map(&:to_h)) end - def get(identity, region) - if address = service.get_address(identity, region).to_h - new(address) + def get(identity, region = nil) + if region + address = service.get_address(identity, region).to_h + return new(address) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + address = response.first unless response.empty? + return address end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/backend_services.rb b/lib/fog/compute/google/models/backend_services.rb index f55c44bd58..5e7d7d1335 100644 --- a/lib/fog/compute/google/models/backend_services.rb +++ b/lib/fog/compute/google/models/backend_services.rb @@ -10,8 +10,9 @@ def all(_filters = {}) end def get(identity) - if backend_service = service.get_backend_service(identity) - new(backend_service.to_h) + if identity + backend_service = service.get_backend_service(identity).to_h + return new(backend_service) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/disk_types.rb b/lib/fog/compute/google/models/disk_types.rb index cc81bd6509..6e424ed62e 100644 --- a/lib/fog/compute/google/models/disk_types.rb +++ b/lib/fog/compute/google/models/disk_types.rb @@ -24,15 +24,14 @@ def all(zone: nil, filter: nil, max_results: nil, end def get(identity, zone = nil) - response = nil if zone - response = service.get_disk_type(identity, zone).to_h + disk_type = service.get_disk_type(identity, zone).to_h + return new(disk_type) else - disk_types = all(:filter => "name eq .*#{identity}") - response = disk_types.first.attributes unless disk_types.empty? + response = all(:filter => "name eq .*#{identity}") + disk_type = response.first unless response.empty? + return disk_type end - return nil if response.nil? - new(response) rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/disks.rb b/lib/fog/compute/google/models/disks.rb index 9588fdd99a..d7fe166e88 100644 --- a/lib/fog/compute/google/models/disks.rb +++ b/lib/fog/compute/google/models/disks.rb @@ -23,10 +23,16 @@ def all(zone: nil, filter: nil, max_results: nil, order_by: nil, load(data.map(&:to_h)) end - def get(identity, zone) - response = service.get_disk(identity, zone) - return nil if response.nil? - new(response.to_h) + def get(identity, zone = nil) + if zone + disk = service.get_disk(identity, zone).to_h + return new(disk) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + disk = response.first unless response.empty? + return disk + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/firewalls.rb b/lib/fog/compute/google/models/firewalls.rb index 072002415d..854b4a1daa 100644 --- a/lib/fog/compute/google/models/firewalls.rb +++ b/lib/fog/compute/google/models/firewalls.rb @@ -10,8 +10,9 @@ def all(opts = {}) end def get(identity) - if firewall = service.get_firewall(identity) - new(firewall.to_h) + if identity + firewall = service.get_firewall(identity).to_h + return new(firewall) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/forwarding_rules.rb b/lib/fog/compute/google/models/forwarding_rules.rb index bbc1e13b98..cf90c4727f 100644 --- a/lib/fog/compute/google/models/forwarding_rules.rb +++ b/lib/fog/compute/google/models/forwarding_rules.rb @@ -29,18 +29,16 @@ def all(region: nil, filter: nil, max_results: nil, end def get(identity, region = nil) - response = nil if region - response = service.get_forwarding_rule(identity, region).to_h + forwarding_rule = service.get_forwarding_rule(identity, region).to_h + return new(forwarding_rule) elsif identity response = all( :filter => "name eq #{identity}", :max_results => 1 ).first - response = response.attributes unless response.nil? + forwarding_rule = response.first unless response.empty? + return forwarding_rule end - - return nil if response.nil? - new(response) end end end diff --git a/lib/fog/compute/google/models/global_addresses.rb b/lib/fog/compute/google/models/global_addresses.rb index c04133c644..76ba049d7b 100644 --- a/lib/fog/compute/google/models/global_addresses.rb +++ b/lib/fog/compute/google/models/global_addresses.rb @@ -10,8 +10,9 @@ def all(options = {}) end def get(identity) - if address = service.get_global_address(identity).to_h - new(address) + if identity + address = service.get_global_address(identity).to_h + return new(address) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/global_forwarding_rules.rb b/lib/fog/compute/google/models/global_forwarding_rules.rb index c0e514079e..9009951fca 100644 --- a/lib/fog/compute/google/models/global_forwarding_rules.rb +++ b/lib/fog/compute/google/models/global_forwarding_rules.rb @@ -10,8 +10,9 @@ def all(opts = {}) end def get(identity) - if rule = service.get_global_forwarding_rule(identity).to_h - new(rule) + if identity + rule = service.get_global_forwarding_rule(identity).to_h + return new(rule) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/http_health_checks.rb b/lib/fog/compute/google/models/http_health_checks.rb index a981d2f3d0..eedc3f2102 100644 --- a/lib/fog/compute/google/models/http_health_checks.rb +++ b/lib/fog/compute/google/models/http_health_checks.rb @@ -10,9 +10,10 @@ def all(_filters = {}) end def get(identity) - response = service.get_http_health_check(identity) - return nil if response.nil? - new(response.to_h) + if identity + http_health_check = service.get_http_health_check(identity).to_h + return new(http_health_check) + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/images.rb b/lib/fog/compute/google/models/images.rb index 0da155d467..6d6d18ce44 100644 --- a/lib/fog/compute/google/models/images.rb +++ b/lib/fog/compute/google/models/images.rb @@ -47,21 +47,22 @@ def current def get(identity, project = nil) project.nil? ? projects = all_projects : projects = [project] - data = nil - projects.each do |proj| - begin - data = service.get_image(identity, proj).to_h - data[:project] = proj - rescue ::Google::Apis::ClientError => e - next if e.status_code == 404 - break - else - break + if identity + projects.each do |proj| + begin + response = service.get_image(identity, proj).to_h + # TODO: Remove this - see #405 + response[:project] = proj + image = response + return new(image) + rescue ::Google::Apis::ClientError => e + next if e.status_code == 404 + break + else + break + end end end - - return nil if data.nil? - new(data) end def get_from_family(family, project = nil) diff --git a/lib/fog/compute/google/models/instance_group_managers.rb b/lib/fog/compute/google/models/instance_group_managers.rb index f96ac7896e..4102b8b2a7 100644 --- a/lib/fog/compute/google/models/instance_group_managers.rb +++ b/lib/fog/compute/google/models/instance_group_managers.rb @@ -26,11 +26,13 @@ def all(zone: nil, filter: nil, max_results: nil, def get(identity, zone = nil) if zone - if instance_group_manager = service.get_instance_group_manager(identity, zone) - new(instance_group_manager.to_h) - end - else - all(:filter => "name eq .*#{identity}").first + instance_group_manager = service.get_instance_group_manager(identity, zone).to_h + return new(instance_group_manager) + elsif identity + response = all(:filter => "name eq .*#{identity}", + :max_results => 1) + instance_group_manager = response.first unless response.empty? + return instance_group_manager end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/instance_groups.rb b/lib/fog/compute/google/models/instance_groups.rb index 86d11ebd3b..c1d246ae45 100644 --- a/lib/fog/compute/google/models/instance_groups.rb +++ b/lib/fog/compute/google/models/instance_groups.rb @@ -18,16 +18,14 @@ def all(filters = {}) end def get(identity, zone = nil) - if zone.nil? - zones = service.list_aggregated_instance_groups(:filter => "name eq .*#{identity}").items - instance_groups = zones.each_value.map(&:instance_groups).compact.first - if instance_groups - zone = instance_groups.first.zone.split("/")[-1] - end - end - - if instance_group = service.get_instance_group(identity, zone) - new(instance_group.to_h) + if zone + instance_group = service.get_instance_group(identity, zone).to_h + new(instance_group) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + instance_group = response.first unless response.empty? + return instance_group end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/instance_templates.rb b/lib/fog/compute/google/models/instance_templates.rb index c5fc746dc3..d211c918e6 100644 --- a/lib/fog/compute/google/models/instance_templates.rb +++ b/lib/fog/compute/google/models/instance_templates.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if instance_template = service.get_instance_template(identity) - new(instance_template.to_h) + if identity + instance_template = service.get_instance_template(identity).to_h + return new(instance_template) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/machine_types.rb b/lib/fog/compute/google/models/machine_types.rb index a03f4901c2..8d9e8490f1 100644 --- a/lib/fog/compute/google/models/machine_types.rb +++ b/lib/fog/compute/google/models/machine_types.rb @@ -23,10 +23,19 @@ def all(zone: nil, filter: nil, max_results: nil, order_by: nil, page_token: nil load(data.map(&:to_h) || []) end - def get(identity, zone) - machine_type = service.get_machine_type(identity, zone).to_h - return nil if machine_type.nil? - new(machine_type) + def get(identity, zone = nil) + if zone + machine_type = service.get_machine_type(identity, zone).to_h + return new(machine_type) + elsif identity + # This isn't very functional since it just shows the first available + # machine type globally, but needed due to overall compatibility + # See: https://github.com/fog/fog-google/issues/352 + response = all(:filter => "name eq #{identity}", + :max_results => 1) + machine_type = response.first unless response.empty? + return machine_type + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/networks.rb b/lib/fog/compute/google/models/networks.rb index c4956cbc3c..c8922f079e 100644 --- a/lib/fog/compute/google/models/networks.rb +++ b/lib/fog/compute/google/models/networks.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if network = service.get_network(identity).to_h - new(network) + if identity + network = service.get_network(identity).to_h + return new(network) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/operations.rb b/lib/fog/compute/google/models/operations.rb index b864e2724b..ae3462c9bd 100644 --- a/lib/fog/compute/google/models/operations.rb +++ b/lib/fog/compute/google/models/operations.rb @@ -26,15 +26,15 @@ def all(zone: nil, region: nil, filter: nil, max_results: nil, def get(identity, zone = nil, region = nil) if !zone.nil? - response = service.get_zone_operation(zone, identity) + operation = service.get_zone_operation(zone, identity).to_h + return new(operation) elsif !region.nil? - response = service.get_region_operation(region, identity) - else - response = service.get_global_operation(identity) + operation = service.get_region_operation(region, identity).to_h + return new(operation) + elsif identity + operation = service.get_global_operation(identity).to_h + return new(operation) end - - return nil if response.nil? - new(response.to_h) rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/projects.rb b/lib/fog/compute/google/models/projects.rb index a2e7ae9ba8..9de7110ad2 100644 --- a/lib/fog/compute/google/models/projects.rb +++ b/lib/fog/compute/google/models/projects.rb @@ -5,8 +5,9 @@ class Projects < Fog::Collection model Fog::Compute::Google::Project def get(identity) - if project = service.get_project(identity).to_h - new(project) + if identity + project = service.get_project(identity).to_h + return new(project) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/regions.rb b/lib/fog/compute/google/models/regions.rb index 3b062b48fe..1f8613e78f 100755 --- a/lib/fog/compute/google/models/regions.rb +++ b/lib/fog/compute/google/models/regions.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if region = service.get_region(identity).to_h - new(region) + if identity + region = service.get_region(identity).to_h + return new(region) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/routes.rb b/lib/fog/compute/google/models/routes.rb index 751b27e4d5..2b6c35e4ea 100644 --- a/lib/fog/compute/google/models/routes.rb +++ b/lib/fog/compute/google/models/routes.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if route = service.get_route(identity).to_h - new(route) + if identity + route = service.get_route(identity).to_h + return new(route) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/servers.rb b/lib/fog/compute/google/models/servers.rb index 53530748b3..81d5b7da35 100644 --- a/lib/fog/compute/google/models/servers.rb +++ b/lib/fog/compute/google/models/servers.rb @@ -28,15 +28,15 @@ def all(zone: nil, filter: nil, max_results: nil, # TODO: This method needs to take self_links as well as names def get(identity, zone = nil) - response = nil if zone - response = service.get_server(identity, zone).to_h - elseif identity - server = all(:filter => "name eq .*#{identity}").first - response = server.attributes if server + server = service.get_server(identity, zone).to_h + return new(server) + elsif identity + response = all(:filter => "name eq .*#{identity}", + :max_results => 1) + server = response.first unless response.empty? + return server end - return nil if response.nil? - new(response) rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/snapshots.rb b/lib/fog/compute/google/models/snapshots.rb index 1b8023f30c..35bab9e809 100644 --- a/lib/fog/compute/google/models/snapshots.rb +++ b/lib/fog/compute/google/models/snapshots.rb @@ -17,10 +17,11 @@ def all load(items) end - def get(snap_id) - response = service.get_snapshot(snap_id) - return nil if response.nil? - new(response.to_h) + def get(identity) + if identity + snapshot = service.get_snapshot(identity).to_h + return new(snapshot) + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/ssl_certificates.rb b/lib/fog/compute/google/models/ssl_certificates.rb index 231cd5d328..6174d4c138 100644 --- a/lib/fog/compute/google/models/ssl_certificates.rb +++ b/lib/fog/compute/google/models/ssl_certificates.rb @@ -4,9 +4,10 @@ class Google class SslCertificates < Fog::Collection model Fog::Compute::Google::SslCertificate - def get(certificate_name) - if certificate = service.get_ssl_certificate(certificate_name) - new(certificate.to_h) + def get(identity) + if identity + ssl_certificate = service.get_ssl_certificate(certificate_name).to_h + return new(ssl_certificate) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/subnetworks.rb b/lib/fog/compute/google/models/subnetworks.rb index c977e00491..5b46aab79b 100644 --- a/lib/fog/compute/google/models/subnetworks.rb +++ b/lib/fog/compute/google/models/subnetworks.rb @@ -23,9 +23,15 @@ def all(region: nil, filter: nil, max_results: nil, order_by: nil, page_token: n load(data || []) end - def get(identity, region) - if subnetwork = service.get_subnetwork(identity, region).to_h - new(subnetwork) + def get(identity, region = nil) + if region + subnetwork = service.get_subnetwork(identity, region).to_h + return new(subnetwork) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + subnetwork = response.first unless response.empty? + return subnetwork end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/target_http_proxies.rb b/lib/fog/compute/google/models/target_http_proxies.rb index 7af21c696e..e88f9e8813 100644 --- a/lib/fog/compute/google/models/target_http_proxies.rb +++ b/lib/fog/compute/google/models/target_http_proxies.rb @@ -12,7 +12,7 @@ def all(_filters = {}) def get(identity) if identity target_http_proxy = service.get_target_http_proxy(identity).to_h - new(target_http_proxy) if target_http_proxy + return new(target_http_proxy) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/target_https_proxies.rb b/lib/fog/compute/google/models/target_https_proxies.rb index f481d65ab3..3dda59d6e0 100644 --- a/lib/fog/compute/google/models/target_https_proxies.rb +++ b/lib/fog/compute/google/models/target_https_proxies.rb @@ -10,8 +10,9 @@ def all(_filters = {}) end def get(identity) - if target_https_proxy = service.get_target_https_proxy(identity).to_h - new(target_https_proxy) + if identity + target_https_proxy = service.get_target_https_proxy(identity).to_h + return new(target_https_proxy) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/target_instances.rb b/lib/fog/compute/google/models/target_instances.rb index 9a1d3c1e38..6713070f1e 100644 --- a/lib/fog/compute/google/models/target_instances.rb +++ b/lib/fog/compute/google/models/target_instances.rb @@ -26,16 +26,16 @@ def all(zone: nil, filter: nil, max_results: nil, order_by: nil, load(data) end - def get(target_instance, zone = nil) - response = nil + def get(identity, zone = nil) if zone - response = service.get_target_instance(target_instance, zone).to_h - else - response = all(:filter => "name eq #{target_instance}").first - response = response.attributes unless response.nil? + target_instance = service.get_target_instance(target_instance, zone).to_h + return new(target_instance) + elsif identity + response = all(:filter => "name eq #{identity}", + :max_results => 1) + target_instance = response.first unless response.empty? + return target_instance end - return nil if response.nil? - new(response) rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/lib/fog/compute/google/models/target_pools.rb b/lib/fog/compute/google/models/target_pools.rb index 533fb30627..4ee132ba3e 100644 --- a/lib/fog/compute/google/models/target_pools.rb +++ b/lib/fog/compute/google/models/target_pools.rb @@ -25,17 +25,14 @@ def all(region: nil, filter: nil, max_results: nil, order_by: nil, page_token: n end def get(identity, region = nil) - response = nil - if region.nil? - data = all(:filter => "name eq #{identity}").first - unless data.nil? - response = data.attributes - end - else - response = service.get_target_pool(identity, region).to_h + if region + target_pool = service.get_target_pool(identity, region).to_h + return new(target_pool) + elsif identity + response = all(:filter => "name eq #{identity}") + target_pool = response.first unless response.empty? + return target_pool end - return nil if response.nil? - new(response) rescue ::Google::Apis::ClientError => e raise e unless e.status_code = 404 nil diff --git a/lib/fog/compute/google/models/url_maps.rb b/lib/fog/compute/google/models/url_maps.rb index 3976883eb0..6f58fb6cb9 100644 --- a/lib/fog/compute/google/models/url_maps.rb +++ b/lib/fog/compute/google/models/url_maps.rb @@ -10,8 +10,9 @@ def all end def get(identity) - if url_map = service.get_url_map(identity).to_h - new(url_map) + if identity + url_map = service.get_url_map(identity).to_h + return new(url_map) end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 diff --git a/lib/fog/compute/google/models/zones.rb b/lib/fog/compute/google/models/zones.rb index dfa681334a..5c128af34f 100644 --- a/lib/fog/compute/google/models/zones.rb +++ b/lib/fog/compute/google/models/zones.rb @@ -10,8 +10,10 @@ def all end def get(identity) - data = service.get_zone(identity).to_h - new(data) + if identity + zone = service.get_zone(identity).to_h + new(zone) + end rescue ::Google::Apis::ClientError => e raise e unless e.status_code == 404 nil diff --git a/test/helpers/test_collection.rb b/test/helpers/test_collection.rb index f866a9fd20..8df68ea646 100644 --- a/test/helpers/test_collection.rb +++ b/test/helpers/test_collection.rb @@ -32,8 +32,6 @@ def test_enumerable end def test_nil_get - # Fixture for #33 - skip if @subject.method(:get).arity <= 1 assert_nil @subject.get(nil) elsif @subject.method(:get).arity == 2 diff --git a/test/unit/compute/test_common_collections.rb b/test/unit/compute/test_common_collections.rb index 6a20b4ffa4..38ebcdb49f 100644 --- a/test/unit/compute/test_common_collections.rb +++ b/test/unit/compute/test_common_collections.rb @@ -30,8 +30,6 @@ def test_common_methods end def test_collection_get_arguments - # TODO: Fixture for #352 - skip @collections.each do |klass| obj = klass.new assert_operator(obj.method(:get).arity, :<=, 1,