From febbada6f7c9b7878014bd9aec648ea73f7bb091 Mon Sep 17 00:00:00 2001
From: Rick Moynihan <rick.m@swirrl.com>
Date: Thu, 9 Jun 2016 14:26:12 +0100
Subject: [PATCH] Fix #54 subclasses & field inheritance override properly

---
 lib/tripod/fields.rb       | 37 ++++++++++++++-----
 spec/tripod/fields_spec.rb | 73 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/lib/tripod/fields.rb b/lib/tripod/fields.rb
index 31be6df..fbf7543 100644
--- a/lib/tripod/fields.rb
+++ b/lib/tripod/fields.rb
@@ -5,9 +5,15 @@
 module Tripod::Fields
   extend ActiveSupport::Concern
 
-  included do
-    class_attribute :fields
-    self.fields = {}
+  def self.included(base)
+    base.instance_eval do
+      @fields ||= {}
+    end
+    base.extend(ClassMethods)
+  end
+
+  def fields
+    self.class.fields
   end
 
   module ClassMethods
@@ -37,7 +43,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 +54,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 +86,8 @@ 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 ||= {}
+      @fields[name] = field
 
       # set up the accessors for the fields
       create_accessors(name, name, options)
@@ -90,7 +111,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 +169,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
diff --git a/spec/tripod/fields_spec.rb b/spec/tripod/fields_spec.rb
index 1bc838d..7007ef6 100644
--- a/spec/tripod/fields_spec.rb
+++ b/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