Deeply symbolize keys in messages

- Use HashUtils.dig instead of try(:[]) [#141054863] Signed-off-by: Greg Cobb <gcobb@pivotal.io>
cloudfoundry · Mar 17, 2017 · 068c652 · 068c652
1 parent 0ddd51f
commit 068c652
Show file tree

Hide file tree

Showing 30 changed files with 148 additions and 157 deletions.
diff --git a/app/controllers/v3/organizations_controller.rb b/app/controllers/v3/organizations_controller.rb
@@ -1,7 +1,7 @@
 require 'presenters/v3/paginated_list_presenter'
 require 'messages/orgs/orgs_list_message'
 require 'presenters/v3/to_one_relationship_presenter'
-require 'messages/orgs_default_iso_seg_update_message'
+require 'messages/orgs/orgs_default_iso_seg_update_message'
 require 'fetchers/org_list_fetcher'
 require 'actions/set_default_isolation_segment'
 require 'controllers/v3/mixins/sub_resource'

diff --git a/app/jobs/diego/sync.rb b/app/jobs/diego/sync.rb
@@ -7,7 +7,7 @@ module Diego
       class Sync < VCAP::CloudController::Jobs::CCJob
         def perform
           config = CloudController::DependencyLocator.instance.config
-          if config.dig(:diego, :temporary_local_sync)
+          if HashUtils.dig(config, :diego, :temporary_local_sync)
             VCAP::CloudController::Diego::ProcessesSync.new(config).sync
             VCAP::CloudController::Diego::TasksSync.new(config).sync
           else

diff --git a/app/messages/apps/app_create_message.rb b/app/messages/apps/app_create_message.rb
@@ -32,15 +32,15 @@ def self.lifecycle_requested?
       if: lifecycle_requested?
 
     def space_guid
-      relationships.try(:[], :space).try(:[], :data).try(:[], :guid)
+      HashUtils.dig(relationships, :space, :data, :guid)
     end
 
     def lifecycle_type
-      lifecycle.try(:[], :type)
+      HashUtils.dig(lifecycle, :type)
     end
 
     def lifecycle_data
-      lifecycle.try(:[], :data)
+      HashUtils.dig(lifecycle, :data)
     end
 
     def buildpack_data

diff --git a/app/messages/apps/app_update_message.rb b/app/messages/apps/app_update_message.rb
@@ -9,7 +9,7 @@ class AppUpdateMessage < BaseMessage
     attr_reader :app
 
     def self.create_from_http_request(body)
-      AppUpdateMessage.new(body.symbolize_keys)
+      AppUpdateMessage.new(body.deep_symbolize_keys)
     end
 
     def self.lifecycle_requested?
@@ -33,11 +33,11 @@ def self.lifecycle_requested?
       if: lifecycle_requested?
 
     def lifecycle_data
-      lifecycle.try(:[], 'data') || lifecycle.try(:[], :data)
+      HashUtils.dig(lifecycle, :data)
     end
 
     def lifecycle_type
-      lifecycle.try(:[], 'type') || lifecycle.try(:[], :type)
+      HashUtils.dig(lifecycle, :type)
     end
 
     def buildpack_data

diff --git a/app/messages/apps/apps_list_message.rb b/app/messages/apps/apps_list_message.rb
@@ -6,17 +6,17 @@ class AppsListMessage < ListMessage
 
     attr_accessor(*ALLOWED_KEYS)
 
+    def initialize(params={})
+      super(params.symbolize_keys)
+    end
+
     validates_with NoAdditionalParamsValidator
 
     validates :names, array: true, allow_nil: true
     validates :guids, array: true, allow_nil: true
     validates :organization_guids, array: true, allow_nil: true
     validates :space_guids, array: true, allow_nil: true
 
-    def initialize(params={})
-      super(params.symbolize_keys)
-    end
-
     def to_param_hash
       super(exclude: [:page, :per_page, :order_by])
     end

diff --git a/app/messages/droplets/droplet_copy_message.rb b/app/messages/droplets/droplet_copy_message.rb
@@ -12,15 +12,15 @@ class DropletCopyMessage < BaseMessage
     validates :app_guid, guid: true
 
     def self.create_from_http_request(body)
-      DropletCopyMessage.new(body.symbolize_keys)
+      DropletCopyMessage.new(body.deep_symbolize_keys)
     end
 
     def app
-      HashUtils.dig(relationships, :app) || HashUtils.dig(relationships, 'app')
+      HashUtils.dig(relationships, :app)
     end
 
     def app_guid
-      HashUtils.dig(app, :guid) || HashUtils.dig(app, 'guid')
+      HashUtils.dig(app, :guid)
     end
 
     private

diff --git a/app/messages/droplets/droplet_create_message.rb b/app/messages/droplets/droplet_create_message.rb
@@ -7,6 +7,9 @@ class DropletCreateMessage < BaseMessage
     ALLOWED_KEYS = [:staging_memory_in_mb, :staging_disk_in_mb, :environment_variables, :lifecycle].freeze
 
     attr_accessor(*ALLOWED_KEYS)
+    def self.create_from_http_request(body)
+      DropletCreateMessage.new(body.deep_symbolize_keys)
+    end
 
     def self.lifecycle_requested?
       @lifecycle_requested ||= proc { |a| a.requested?(:lifecycle) }
@@ -20,29 +23,25 @@ def self.lifecycle_requested?
     validates :environment_variables, environment_variables: true, allow_nil: true
 
     validates :lifecycle_type,
-      string: true,
-      allow_nil: false,
-      if: lifecycle_requested?
+    string: true,
+    allow_nil: false,
+    if: lifecycle_requested?
 
     validates :lifecycle_data,
-      hash: true,
-      allow_nil: false,
-      if: lifecycle_requested?
-
-    def self.create_from_http_request(body)
-      DropletCreateMessage.new(body.symbolize_keys)
-    end
+    hash: true,
+    allow_nil: false,
+    if: lifecycle_requested?
 
     def buildpack_data
       @buildpack_data ||= VCAP::CloudController::BuildpackLifecycleDataMessage.create_from_http_request(lifecycle_data)
     end
 
     def lifecycle_data
-      lifecycle.try(:[], 'data') || lifecycle.try(:[], :data)
+      HashUtils.dig(lifecycle, :data)
     end
 
     def lifecycle_type
-      lifecycle.try(:[], 'type') || lifecycle.try(:[], :type)
+      HashUtils.dig(lifecycle, :type)
     end
 
     private

diff --git a/app/messages/isolation_segments/isolation_segment_create_message.rb b/app/messages/isolation_segments/isolation_segment_create_message.rb
@@ -7,7 +7,7 @@ class IsolationSegmentCreateMessage < BaseMessage
     attr_accessor(*ALLOWED_KEYS)
 
     def self.create_from_http_request(body)
-      IsolationSegmentCreateMessage.new(body.symbolize_keys)
+      IsolationSegmentCreateMessage.new(body.deep_symbolize_keys)
     end
 
     validates_with NoAdditionalKeysValidator

diff --git a/app/messages/isolation_segments/isolation_segment_relationship_org_message.rb b/app/messages/isolation_segments/isolation_segment_relationship_org_message.rb
@@ -7,22 +7,22 @@ class IsolationSegmentRelationshipOrgMessage < BaseMessage
     attr_accessor(*ALLOWED_KEYS)
 
     def self.create_from_http_request(body)
-      IsolationSegmentRelationshipOrgMessage.new(body.symbolize_keys)
+      IsolationSegmentRelationshipOrgMessage.new(body.deep_symbolize_keys)
     end
 
     validates_with NoAdditionalKeysValidator
     validates :data, presence: true, array: true, allow_nil: false, allow_blank: false
     validates_each :data do |record, attr, values|
       if values.is_a? Array
         values.each do |value|
-          guid = value['guid']
+          guid = value[:guid]
           record.errors.add attr, "#{guid} not a string" if !guid.is_a? String
         end
       end
     end
 
     def guids
-      data.map { |val| val['guid'] }
+      data.map { |val| val[:guid] }
     end
 
     private

diff --git a/app/messages/isolation_segments/isolation_segment_update_message.rb b/app/messages/isolation_segments/isolation_segment_update_message.rb
@@ -7,7 +7,7 @@ class IsolationSegmentUpdateMessage < BaseMessage
     attr_accessor(*ALLOWED_KEYS)
 
     def self.create_from_http_request(body)
-      IsolationSegmentUpdateMessage.new(body.symbolize_keys)
+      IsolationSegmentUpdateMessage.new(body.deep_symbolize_keys)
     end
 
     validates_with NoAdditionalKeysValidator

diff --git a/app/messages/lifecycles/buildpack_lifecycle_data_message.rb b/app/messages/lifecycles/buildpack_lifecycle_data_message.rb
@@ -3,7 +3,7 @@ class BuildpackLifecycleDataMessage < BaseMessage
     ALLOWED_KEYS = [:buildpacks, :stack].freeze
 
     def self.create_from_http_request(body)
-      BuildpackLifecycleDataMessage.new((body || {}).symbolize_keys)
+      BuildpackLifecycleDataMessage.new((body || {}).deep_symbolize_keys)
     end
 
     attr_accessor(*ALLOWED_KEYS)

diff --git a/app/messages/lifecycles/docker_lifecycle_data_message.rb b/app/messages/lifecycles/docker_lifecycle_data_message.rb
@@ -10,7 +10,7 @@ def allowed_keys
     validates_with NoAdditionalKeysValidator
 
     def self.create_from_http_request(body)
-      DockerLifecycleDataMessage.new((body || {}).symbolize_keys)
+      DockerLifecycleDataMessage.new((body || {}).deep_symbolize_keys)
     end
   end
 end
diff --git a/...es/orgs_default_iso_seg_update_message.rb → ...gs/orgs_default_iso_seg_update_message.rb b/...es/orgs_default_iso_seg_update_message.rb → ...gs/orgs_default_iso_seg_update_message.rb
@@ -6,6 +6,10 @@ class OrgDefaultIsoSegUpdateMessage < BaseMessage
 
     attr_accessor(*ALLOWED_KEYS)
 
+    def self.create_from_http_request(body)
+      OrgDefaultIsoSegUpdateMessage.new(body.deep_symbolize_keys)
+    end
+
     def self.data_requested?
       @data_requested ||= proc { |a| a.requested?(:data) }
     end
@@ -15,19 +19,16 @@ def self.data_requested?
     validate :data_content, if: data_requested?
 
     def default_isolation_segment_guid
-      return data['guid'] if data
-      nil
+      HashUtils.dig(data, :guid)
     end
 
     def data_content
       return if data.nil?
       errors.add(:data, 'can only accept one key') unless data.keys.length == 1
-      errors.add(:data, "can only accept key 'guid'") unless data.keys.include?('guid')
-      errors.add(:data, "#{data['guid']} must be a string") if data['guid'] && !data['guid'].is_a?(String)
-    end
-
-    def self.create_from_http_request(body)
-      OrgDefaultIsoSegUpdateMessage.new(body.symbolize_keys)
+      errors.add(:data, "can only accept key 'guid'") unless data.keys.include?(:guid)
+      if default_isolation_segment_guid && !default_isolation_segment_guid.is_a?(String)
+        errors.add(:data, "#{default_isolation_segment_guid} must be a string")
+      end
     end
 
     private

diff --git a/app/messages/packages/package_create_message.rb b/app/messages/packages/package_create_message.rb
@@ -24,7 +24,7 @@ def bits_type?
     end
 
     def app_guid
-      relationships.dig(:app, :data, :guid)
+      HashUtils.dig(relationships, :app, :data, :guid)
     end
 
     def docker_type?

diff --git a/app/messages/processes/process_scale_message.rb b/app/messages/processes/process_scale_message.rb
@@ -13,7 +13,7 @@ class ProcessScaleMessage < BaseMessage
     validates :disk_in_mb, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true
 
     def self.create_from_http_request(body)
-      ProcessScaleMessage.new(body.symbolize_keys)
+      ProcessScaleMessage.new(body.deep_symbolize_keys)
     end
 
     private

diff --git a/app/messages/processes/process_update_message.rb b/app/messages/processes/process_update_message.rb
@@ -6,10 +6,14 @@ class ProcessUpdateMessage < BaseMessage
 
     attr_accessor(*ALLOWED_KEYS)
 
+    def self.create_from_http_request(body)
+      ProcessUpdateMessage.new(body.deep_symbolize_keys)
+    end
+
     def initialize(params={})
       super(params)
-      @requested_keys << :health_check_type if params[:health_check] && params[:health_check].key?('type')
-      @requested_keys << :health_check_timeout if params[:health_check] && params[:health_check]['data'] && params[:health_check]['data'].key?('timeout')
+      @requested_keys << :health_check_type if HashUtils.dig(params, :health_check, :type)
+      @requested_keys << :health_check_timeout if HashUtils.dig(params, :health_check, :data, :timeout)
     end
 
     def self.health_check_requested?
@@ -23,18 +27,18 @@ def self.ports_requested?
     validates_with NoAdditionalKeysValidator
 
     validates :command,
-      string: true,
-      length: { in: 1..4096, message: 'must be between 1 and 4096 characters' },
-      if:     proc { |a| a.requested?(:command) }
+    string: true,
+    length: { in: 1..4096, message: 'must be between 1 and 4096 characters' },
+    if:     proc { |a| a.requested?(:command) }
 
     validates :health_check_type,
-      inclusion: { in: %w(port process), message: 'must be "port" or "process"' },
-      if: health_check_requested?
+    inclusion: { in: %w(port process), message: 'must be "port" or "process"' },
+    if: health_check_requested?
 
     validates :health_check_timeout,
-      allow_nil: true,
-      numericality: { only_integer: true, greater_than_or_equal_to: 0 },
-      if: health_check_requested?
+    allow_nil: true,
+    numericality: { only_integer: true, greater_than_or_equal_to: 0 },
+    if: health_check_requested?
 
     validate :port_validations, if: ports_requested?
 
@@ -69,21 +73,17 @@ def port_validations
     end
 
     def health_check_type
-      HashUtils.dig(health_check, 'type') || HashUtils.dig(health_check, :type)
+      HashUtils.dig(health_check, :type)
     end
 
     def health_check_timeout
-      HashUtils.dig(health_check, 'data', 'timeout') || HashUtils.dig(health_check, :data, :timeout)
+      HashUtils.dig(health_check, :data, :timeout)
     end
 
     def audit_hash
       super(exclude: [:health_check_type, :health_check_timeout])
     end
 
-    def self.create_from_http_request(body)
-      ProcessUpdateMessage.new(body.symbolize_keys)
-    end
-
     private
 
     def allowed_keys

diff --git a/app/messages/route_mappings/route_mappings_create_message.rb b/app/messages/route_mappings/route_mappings_create_message.rb
@@ -16,31 +16,31 @@ class RouteMappingsCreateMessage < BaseMessage
     validates :process_type, string: true, allow_nil: true
 
     def self.create_from_http_request(body)
-      RouteMappingsCreateMessage.new(body.symbolize_keys)
+      RouteMappingsCreateMessage.new(body.deep_symbolize_keys)
     end
 
     def app
-      HashUtils.dig(relationships, :app) || HashUtils.dig(relationships, 'app')
+      HashUtils.dig(relationships, :app)
     end
 
     def app_guid
-      HashUtils.dig(app, :guid) || HashUtils.dig(app, 'guid')
+      HashUtils.dig(app, :guid)
     end
 
     def process
-      HashUtils.dig(relationships, :process) || HashUtils.dig(relationships, 'process')
+      HashUtils.dig(relationships, :process)
     end
 
     def process_type
-      HashUtils.dig(process, :type) || HashUtils.dig(process, 'type') || DEFAULT_PROCESS_TYPE
+      HashUtils.dig(process, :type) || DEFAULT_PROCESS_TYPE
     end
 
     def route
-      HashUtils.dig(relationships, :route) || HashUtils.dig(relationships, 'route')
+      HashUtils.dig(relationships, :route)
     end
 
     def route_guid
-      HashUtils.dig(route, :guid) || HashUtils.dig(route, 'guid')
+      HashUtils.dig(route, :guid)
     end
 
     private