Fix #54 subclasses & field inheritance override properly

lib/tripod/fields.rb

@@ -5,9 +5,35 @@
 module Tripod::Fields
   extend ActiveSupport::Concern
 
-  included do
-    class_attribute :fields
-    self.fields = {}
+  class FieldsAccessor < Hash
+    # An adapter class to support Hash-like access to fields, but in a
+    # way that overrides based on the inheritance hierarchy
+
+    attr_accessor :fields, :klass
+    def initialize(klass, fields)
+      @fields = fields
+      @klass = klass
+    end
+
+    def [](arg)
+
+    end
+
+    def ==(other)
+      @fields == other.fields && klass == other.klass
+    end
+
+  end
+
+  def self.included(base)
+    base.instance_eval do
+      @fields ||= {}
+    end
+    base.extend(ClassMethods)
+  end
+
+  def fields
+    self.class.fields
   end
 
   module ClassMethods
@@ -37,7 +63,7 @@ module ClassMethods
     #
     # @return [ Field ] The generated field
     def field(name, predicate, options = {})
-      # TODO: validate the field params/options here..
+      @fields ||= {}
       add_field(name, predicate, options)
     end
 
@@ -48,13 +74,27 @@ def field(name, predicate, options = {})
     #
     # @param [ Symbol ] name The name of the field.
     def get_field(name)
-      field = self.fields[name]
+      @fields ||= {}
+      field = fields[name]
       raise Tripod::Errors::FieldNotPresent.new unless field
       field
     end
 
+    # Return all of the fields on a +Resource+ in a manner that
+    # respects Ruby's inheritance rules.  i.e. subclass fields should
+    # override superclass fields with the same
+    def fields
+      tripod_superclasses.map { |c| c.instance_variable_get(:@fields) }.reduce do |acc,class_fields|
+        class_fields.merge(acc)
+      end
+    end
+
     protected
 
+    def tripod_superclasses
+      self.ancestors.select { |a| a.class == Class && a.respond_to?(:fields)}
+    end
+
     # Define a field attribute for the +Resource+.
     #
     # @example Set the field.
@@ -66,7 +106,7 @@ def get_field(name)
     def add_field(name, predicate, options = {})
       # create a field object and store it in our hash
       field = field_for(name, predicate, options)
-      fields[name] = field
+      @fields[name] = field
 
       # set up the accessors for the fields
       create_accessors(name, name, options)
@@ -90,7 +130,7 @@ def add_field(name, predicate, options = {})
     # @param [ Symbol ] meth The name of the accessor.
     # @param [ Hash ] options The options.
     def create_accessors(name, meth, options = {})
-      field = fields[name]
+      field = @fields[name]
 
       create_field_getter(name, meth, field)
       create_field_setter(name, meth, field)
@@ -148,7 +188,7 @@ def create_field_check(name, meth, field)
       end
     end
 
-     # Include the field methods as a module, so they can be overridden.
+    # Include the field methods as a module, so they can be overridden.
     #
     # @example Include the fields.
     #   Person.generated_methods

spec/tripod/fields_spec.rb

@@ -1,7 +1,6 @@
 require "spec_helper"
 
 describe Tripod::Fields do
-
   describe ".field" do
 
     let(:barry) do
@@ -50,3 +49,75 @@
     end
   end
 end
+
+module Spec
+  module Tripod
+    module Inheritance
+      BASE_PREDICATE = RDF::URI.new("http://base/predicate/overriden/from/SubSub/up")
+      BASE_PREDICATE_OVERIDE = RDF::URI.new("http://overide/base/predicate")
+
+      ANOTHER_PREDICATE = RDF::RDFS::label
+
+      class Base
+        include ::Tripod::Resource
+        field :inherited, BASE_PREDICATE
+      end
+
+      class Sub < Base
+        field :bar, ANOTHER_PREDICATE
+        # expects inerited to be ANOTHER_PREDICATE
+      end
+
+      class SubSub < Sub
+        field :inherited, BASE_PREDICATE_OVERIDE
+      end
+
+      class SubSubSub < SubSub
+        # defines no new fields, used to test no NullPointerExceptions
+        # etc on classes that don't define fields.
+      end
+
+      describe 'inheritance' do
+        describe Base do
+          subject(:base) { Base }
+
+          it "does not inhert fields from subclasses" do
+            expect(base.fields[:bar]).to be_nil
+          end
+
+          it "defines the :inherited field" do
+            inherited_field = base.fields[:inherited]
+            expect(inherited_field.predicate).to eq(BASE_PREDICATE)
+          end
+        end
+
+        describe Sub do
+          subject(:inherited) { Sub.get_field(:inherited) }
+          it "does not redefine :inherited field" do
+            expect(inherited.predicate).to eq(BASE_PREDICATE)
+          end
+        end
+
+        describe SubSub do
+          subject(:inherited) { SubSub.get_field(:inherited) }
+
+          it "overrides the :inherited field" do
+            expect(inherited.predicate).to eq(BASE_PREDICATE_OVERIDE)
+          end
+        end
+
+        describe SubSubSub do
+          it "inherits the :bar field from Sub" do
+            bar = SubSubSub.get_field(:bar)
+            expect(bar.predicate).to eq(ANOTHER_PREDICATE)
+          end
+
+          it "overrides the :inherited field in Base with the value from SubSub" do
+            inherited = SubSubSub.get_field(:inherited)
+            expect(inherited.predicate).to eq(BASE_PREDICATE_OVERIDE)
+          end
+        end
+      end
+    end
+  end
+end