Wrap Api#attributes with indifferent key access

A number of bugs have been introduced or missed due to the way
attributes are exposed by fog. Fog transforms the top level of
attributes from the JSON sources (String) to Symbols, but does not act
recursively.

This results in accessing nested structures with a mix of Symbol and
String keys which inevitably is missed or incorrectly specced out.

This change first transforms the keys to use Symbols then adds a simple
wrapper around `Api#attributes` that takes the fog attributes and allows
indifferent access with Symbols or Strings. To allow existing code to
work even if Symbols were not used for any lookups.

Author: tokengeek

lib/brightbox-cli/api.rb

@@ -52,6 +52,16 @@ def initialize(model = nil)
       Brightbox.config.cache_id(@id) if Brightbox.config.respond_to?(:cache_id)
     end
 
+    def attributes
+      fog_attributes
+    end
+
+    # Returns the transformed attributes from the fog model with a
+    # wrapper to allow access using either String or Symbol keys.
+    def fog_attributes
+      IndifferentAccessHash.new(deep_symbolize(fog_model.attributes))
+    end
+
     def fog_model
       @fog_model ||= self.class.find(@id)
     end
@@ -192,5 +202,24 @@ def created_on
 
       fog_model.created_at.strftime("%Y-%m-%d")
     end
+
+    private
+
+    # Recursively converts all keys in a hash to symbols to ensure
+    # consistent access.
+    def deep_symbolize(hash)
+      return hash unless hash.respond_to?(:each_with_object)
+
+      hash.each_with_object({}) do |(k, v), result|
+        result[k.to_sym] = case v
+                           when Hash
+                             deep_symbolize(v)
+                           when Array
+                             v.map { |i| i.is_a?(Hash) ? deep_symbolize(i) : i }
+                           else
+                             v
+                           end
+      end
+    end
   end
 end

lib/brightbox-cli/indifferent_access_hash.rb

@@ -0,0 +1,50 @@
+# A simpler wrapper to allows either String or Symbol keys to be used
+# when accessing attributes since fog applies a change on the top
+# level resulting in a mix of both which has introduced issues.
+class IndifferentAccessHash
+  def initialize(hash)
+    @hash = hash
+  end
+
+  # @param key [String, Symbol] the key to look up
+  # @return [Object] the value of the key
+  def [](key)
+    value = @hash[key.to_s] || @hash[key.to_sym]
+    wrap(value)
+  end
+
+  # @param key [String, Symbol] the key to set
+  # @param value [Object] the value to set
+  # @return [Object] the value of the key
+  def []=(key, value)
+    @hash[key.to_s] = value
+  end
+
+  # @param other [Object] the object to compare
+  # @return [Object] the result of the comparison
+  def ==(other)
+    @hash == (other.is_a?(IndifferentAccessHash) ? other.to_h : other)
+  end
+
+  def method_missing(method, *args, &block)
+    @hash.send(method, *args, &block)
+  end
+
+  def to_h
+    @hash
+  end
+
+  private
+
+  # This is to handle nested hashes to avoid the original issue again
+  def wrap(value)
+    case value
+    when Hash
+      IndifferentAccessHash.new(value)
+    when Array
+      value.map { |v| wrap(v) }
+    else
+      value
+    end
+  end
+end

lib/brightbox_cli.rb

@@ -71,6 +71,7 @@ module Config
 require_relative "brightbox-cli/connection_manager"
 require_relative "brightbox-cli/tables"
 require_relative "brightbox-cli/logging"
+require_relative "brightbox-cli/indifferent_access_hash"
 require_relative "brightbox-cli/api"
 require_relative "brightbox-cli/config/cache"
 require_relative "brightbox-cli/config/gpg_encrypted_passwords"

spec/unit/brightbox/api/attributes_spec.rb

@@ -0,0 +1,65 @@
+require "spec_helper"
+
+RSpec.describe Brightbox::Api, "#attributes" do
+  subject(:model_attributes) { api_model.attributes }
+
+  let(:api_model) { Brightbox::Api.new(fog_model) }
+  let(:fog_model) do
+    double(
+      "Fog::Compute::Brightbox::Api",
+      id: "res-12345",
+      attributes: attributes
+    )
+  end
+
+  context "when attributes are not set" do
+    let(:attributes) { nil }
+
+    it "returns an indifferent access hash" do
+      expect(model_attributes).to be_instance_of(IndifferentAccessHash)
+    end
+  end
+
+  context "when attributes are set with a mix of string and symbols" do
+    let(:attributes) do
+      {
+        id: "res-12345",
+        name: "test",
+        acme: acme_details
+      }
+    end
+    let(:acme_details) do
+      {
+        "domains" => [
+          {
+            "identifier" => "example.test",
+            "status" => "pending"
+          }
+        ]
+      }
+    end
+
+    it "transforms the keys with deep symbolize" do
+      expect(model_attributes).to eq(
+        id: "res-12345",
+        name: "test",
+        acme: {
+          domains: [
+            {
+              identifier: "example.test",
+              status: "pending"
+            }
+          ]
+        }
+      )
+    end
+
+    it "can accessed by string keys" do
+      expect(model_attributes["acme"]["domains"].first["identifier"]).to eq("example.test")
+    end
+
+    it "can accessed by symbol keys" do
+      expect(model_attributes[:acme][:domains].first[:identifier]).to eq("example.test")
+    end
+  end
+end

spec/unit/brightbox/api/fog_attributes_spec.rb

@@ -0,0 +1,65 @@
+require "spec_helper"
+
+RSpec.describe Brightbox::Api, "#fog_attributes" do
+  subject(:model_attributes) { api_model.fog_attributes }
+
+  let(:api_model) { Brightbox::Api.new(fog_model) }
+  let(:fog_model) do
+    double(
+      "Fog::Compute::Brightbox::Api",
+      id: "res-12345",
+      attributes: attributes
+    )
+  end
+
+  context "when attributes are not set" do
+    let(:attributes) { nil }
+
+    it "returns an indifferent access hash" do
+      expect(model_attributes).to be_instance_of(IndifferentAccessHash)
+    end
+  end
+
+  context "when attributes are set with a mix of string and symbols" do
+    let(:attributes) do
+      {
+        id: "res-12345",
+        name: "test",
+        acme: acme_details
+      }
+    end
+    let(:acme_details) do
+      {
+        "domains" => [
+          {
+            "identifier" => "example.test",
+            "status" => "pending"
+          }
+        ]
+      }
+    end
+
+    it "transforms the keys with deep symbolize" do
+      expect(model_attributes).to eq(
+        id: "res-12345",
+        name: "test",
+        acme: {
+          domains: [
+            {
+              identifier: "example.test",
+              status: "pending"
+            }
+          ]
+        }
+      )
+    end
+
+    it "can not accessed by string keys" do
+      expect(model_attributes["acme"]["domains"].first["identifier"]).to eq("example.test")
+    end
+
+    it "can accessed by symbol keys" do
+      expect(model_attributes[:acme][:domains].first[:identifier]).to eq("example.test")
+    end
+  end
+end