Merge pull request #400 from Temikus/nonexistent_und_nil_checks

Fixing empty get() behavior

Author: Temikus

.rubocop.yml

@@ -6,6 +6,9 @@ Style/ConditionalAssignment:
   SingleLineConditionsOnly: true
   EnforcedStyle: assign_inside_condition
 
+Style/RedundantReturn:
+  Enabled: false
+
 Style/FormatString:
   Enabled: false
 

.travis.yml

@@ -12,9 +12,6 @@ matrix:
     - rvm: 2.5
     - rvm: jruby-head
   allow_failures:
-  # allow 2.5 to fail until Travis CI is fixed.
-  # See travis-ci/travis-ci#8969
-    - rvm: 2.5
     - rvm: jruby-head
 notifications:
   email: false

CHANGELOG.md

@@ -9,7 +9,34 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 #### Fixed
 
 - \#419 Locked down fog upstream dependencies to alleviate deprecation warnings
-  until they can be properly dealt with.
+  until they can be properly dealt with. [temikus]
+- \#400 Small `%Collection%.get` and `%Collection%.all` behaviour fixes [temikus]
+  - `Fog::Google::SQL::Instances.get(nil)` no longer returns an invalid 
+    `sql#instancesList` object.
+  - `Fog::Compute::Google::InstanceGroups.get` and `.all` methods now support more than
+    just `:filter` option, fixed `.all` output if zone wasn't provided.
+  - Fix a typo causing `Operations.get(region:REGION)` to fail.
+  - `Fog::Compute::Google::Images.get(IMAGE, PROJECT)`, now returns `nil` if image is not
+    found rather than throwing `Google::Apis::ClientError`.
+
+### Development changes
+
+#### Added
+
+- \#400 Additional test coverage [temikus]
+  - Expanded tests for `%Collection%.get` behavior - scoped requests (e.g. `get(zone:ZONE)`)
+    and their corresponding code paths are now also tested.
+  - Increase `Fog::Compute::Google::Images` integration test coverage.
+  - Unit tests now work without a `~/.fog` config file set up.
+  - Expanded unit test coverage.
+
+#### Changed
+
+- \#400 Refactored most compute `get()` and `all()` methods to common format. [temikus]
+
+#### Fixed
+
+- \#400 Removed the Travis Ruby 2.5 workaround. [temikus]
 
 ## 1.7.1
 

CONTRIBUTING.md

@@ -101,20 +101,26 @@ test:
 Then you can run all the live tests:
 
 ```shell
-$ rake test
+$ bundle exec rake test
 ```
 
 or just one API:
 
 ```
-$ rake test:compute
+$ bundle exec rake test:compute
 ```
 (See `rake -T` for all available tasks)
 
-or just one file:
+or just one test:
 
 ```shell
-$ rake test TEST=test/integration/compute/test_servers.rb TESTOPTS="--name=TestServers#test_bootstrap_ssh_destroy"
+$ bundle exec rake test TESTOPTS="--name=TestServers#test_bootstrap_ssh_destroy"
+```
+
+or a series of tests by name:
+
+```
+$ bundle exec rake test TESTOPTS="--name=/test_nil_get/"
 ```
 
 #### Unit tests
@@ -133,7 +139,7 @@ We're in progress of extending the library with more unit tests and contribution
 #### The transition from `shindo` to Minitest
 
 Previously, [shindo](https://github.com/geemus/shindo) was the primary testing framework. 
-We've started moving away from it, and to Minitest, but some artifacts may remain.
+We've moved away from it, and to Minitest, but some artifacts may remain.
 
 For more information on transition, read [#50](https://github.com/fog/fog-google/issues/50).
 

README.md

@@ -42,8 +42,6 @@ We are always looking for people to improve our code and test coverage, so pleas
 
 ## Pubsub
 
-Note: You **must** have a version of google-api-client > 0.8.5 to use the Pub/Sub API; previous versions will not work.
-
 Fog mostly implements [v1](https://cloud.google.com/pubsub/docs/reference/rest/) of the Google Cloud Pub/Sub API; however some less common API methods are missing. Pull requests for additions would be greatly appreciated.
 
 ## Installation

lib/fog/bin/google.rb

@@ -15,6 +15,8 @@ def class_for(key)
         Fog::Storage::Google
       when :sql
         Fog::Google::SQL
+      when :pubsub
+        Fog::Google::Pubsub
       else
         raise ArgumentError, "Unsupported #{self} service: #{key}"
       end
@@ -33,6 +35,8 @@ def [](service)
                       Fog::Google::Monitoring.new
                     when :sql
                       Fog::Google::SQL.new
+                    when :pubsub
+                      Fog::Google::Pubsub.new
                     when :storage
                       Fog::Logger.warning("Google[:storage] is not recommended, use Storage[:google] for portability")
                       Fog::Storage.new(:provider => "Google")

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

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

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

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