Fix up polymorphism for core model types (#5)

* Fix up polymorphism for core model types

Previously the manuscript, holding, and record classes weren't doing much.
They were added with an eye toward polymorphism, but the ExportRepresenter
was converting all item hashes unconditionally to a generic DsItem object
with methods applicable to all three.

This change gets ExportRepresenter working as intended so that Holding,
Manuscript, and Record objects are created by ExportRepresenter where
appropriate. DsItem is removed as no longer needed, and all unused methods
of the core model types are removed as well.

The spec helper export hash is also updated to accommodate these changes.

Addresses comment on #3

* Apply suggestions from code review

Co-authored-by: emeryr-upenn <[email protected]>

* Revive DsItem, add instance_of method

* cleanup

---------

Co-authored-by: emeryr-upenn <[email protected]>

digital_scriptorium.gemspec

@@ -30,6 +30,7 @@ Gem::Specification.new do |spec|
 
   spec.add_dependency 'wikibase_representable', '~> 0.1'
   spec.add_development_dependency 'bundler', '~> 2.5'
+  spec.add_development_dependency 'pry', '~> 0.14'
   spec.add_development_dependency 'rake', '~> 13.2'
   spec.add_development_dependency 'rspec', '~> 3.13'
   spec.add_development_dependency 'upennlib-rubocop', '~> 1.2'

lib/digital_scriptorium/ds_item.rb

@@ -3,46 +3,12 @@
 require 'wikibase_representable'
 
 module DigitalScriptorium
-  # Represents a Digital Scriptorium item
+  # Represents a generic Digital Scriptorium item
   class DsItem < WikibaseRepresentable::Model::Item
-    def instance_of_claims
-      claims_by_property_id PropertyId::INSTANCE_OF # P16
-    end
-
-    def ds_id
-      claims_by_property_id(PropertyId::DS_ID)&.first&.data_value # P1
-    end
-
-    def holding_ids
-      claims_by_property_id(PropertyId::MANUSCRIPT_HOLDING)&.map(&:entity_id_value) # P2
-    end
-
-    def described_manuscript_id
-      claims_by_property_id(PropertyId::DESCRIBED_MANUSCRIPT)&.first&.entity_id_value # P3
-    end
-
-    def holding_status
-      claims_by_property_id(PropertyId::HOLDING_STATUS)&.first&.entity_id_value # P6
-    end
-
-    def iiif_manifest
-      claims_by_property_id(PropertyId::IIIF_MANIFEST)&.first&.entity_id_value # P41
-    end
-
-    def core_model_item?
-      instance_of_claims.any? { |claim| ItemId::CORE_MODEL_ITEMS.include? claim.entity_id_value }
-    end
-
-    def manuscript?
-      instance_of_claims.any? { |claim| claim.entity_id_value == ItemId::MANUSCRIPT }
-    end
-
-    def holding?
-      instance_of_claims.any? { |claim| claim.entity_id_value == ItemId::HOLDING }
-    end
+    include PropertyId
 
-    def record?
-      instance_of_claims.any? { |claim| claim.entity_id_value == ItemId::RECORD }
+    def instance_of
+      claims_by_property_id(INSTANCE_OF)&.first&.entity_id_value
     end
   end
 end

lib/digital_scriptorium/ds_meta.rb

@@ -21,12 +21,8 @@ def initialize(record, export_hash)
       @record = record
     end
 
-    def current?(holding)
-      holding.holding_status == HOLDING_STATUS_CURRENT
-    end
-
     def current_holdings(manuscript, export_hash)
-      manuscript.holding_ids.filter_map { |id| export_hash[id] if current?(export_hash[id]) }
+      manuscript.holding_ids.filter_map { |id| export_hash[id] if export_hash[id]&.current? }
     end
   end
 end

lib/digital_scriptorium/export.rb

@@ -11,5 +11,12 @@ def to_hash
       end
       hash
     end
+
+    def instance_of_id_from(item_hash)
+      claims = item_hash['claims']
+      return nil unless claims&.any?
+
+      claims.dig('P16', 0, 'mainsnak', 'datavalue', 'value', 'id')
+    end
   end
 end

lib/digital_scriptorium/export_representer.rb

@@ -6,14 +6,26 @@
 module DigitalScriptorium
   # Representer class for deserializing Wikibase data exports from JSON.
   class ExportRepresenter < Representable::Decorator
+    include ItemId
     include Representable::JSON::Collection
     include WikibaseRepresentable::Model
     include WikibaseRepresentable::Representers
 
     items decorator: lambda { |input:, **|
       input.type == Item::ENTITY_TYPE ? ItemRepresenter : PropertyRepresenter
     }, class: lambda { |input:, **|
-      input['type'] == Item::ENTITY_TYPE ? DsItem : Property
+      return Property unless input['type'] == Item::ENTITY_TYPE
+
+      case instance_of_id_from input
+      when MANUSCRIPT
+        Manuscript
+      when HOLDING
+        Holding
+      when DS_20_RECORD
+        Record
+      else
+        DsItem
+      end
     }
   end
 end

lib/digital_scriptorium/holding.rb

@@ -3,32 +3,21 @@
 module DigitalScriptorium
   # An item representing a Digital Scriptorium holding (instance of Q2)
   class Holding < DsItem
-    def institution_as_recorded_claims
-      claims_by_property_id HOLDING_INSTITUTION_AS_RECORDED # P5
-    end
+    include ItemId
+    include PropertyId
 
     def status_claims
       claims_by_property_id HOLDING_STATUS # P6
     end
 
-    def institutional_id_claims
-      claims_by_property_id INSTITUTIONAL_ID # P7
-    end
-
-    def shelfmark_claims
-      claims_by_property_id SHELFMARK # P8
-    end
-
-    def link_to_institutional_record_claims
-      claims_by_property_id LINK_TO_INSTITUTIONAL_RECORD # P9
-    end
+    def status
+      return unless status_claims&.any?
 
-    def start_time_claims
-      claims_by_property_id START_TIME # P38
+      status_claims&.first&.entity_id_value
     end
 
-    def end_time_claims
-      claims_by_property_id END_TIME # P39
+    def current?
+      status == HOLDING_STATUS_CURRENT
     end
   end
 end

lib/digital_scriptorium/item_id.rb

@@ -7,7 +7,7 @@ module DigitalScriptorium
   module ItemId
     MANUSCRIPT                 = 'Q1'
     HOLDING                    = 'Q2'
-    RECORD                     = 'Q3'
+    DS_20_RECORD               = 'Q3'
     HOLDING_STATUS_CURRENT     = 'Q4'
     HOLDING_STATUS_NON_CURRENT = 'Q5'
     STANDARD_TITLE             = 'Q6'
@@ -23,6 +23,6 @@ module ItemId
     PLACE                      = 'Q16'
     MATERIAL                   = 'Q17'
 
-    CORE_MODEL_ITEMS = Set[MANUSCRIPT, HOLDING, RECORD]
+    CORE_MODEL_ITEMS = Set[MANUSCRIPT, HOLDING, DS_20_RECORD]
   end
 end

lib/digital_scriptorium/record.rb

@@ -8,65 +8,5 @@ class Record < DsItem
     def described_manuscript_id
       claims_by_property_id(DESCRIBED_MANUSCRIPT)&.first&.entity_id_value # P3
     end
-
-    def title_as_recorded_claims
-      claims_by_property_id TITLE_AS_RECORDED # P10
-    end
-
-    def uniform_title_as_recorded_claims
-      claims_by_property_id UNIFORM_TITLE_AS_RECORDED # P12
-    end
-
-    def associated_name_as_recorded_claims
-      claims_by_property_id ASSOCIATED_NAME_AS_RECORDED # P14
-    end
-
-    def genre_as_recorded_claims
-      claims_by_property_id GENRE_AS_RECORDED # P18
-    end
-
-    def language_as_recorded_claims
-      claims_by_property_id LANGUAGE_AS_RECORDED # P21
-    end
-
-    def production_date_as_recorded_claims
-      claims_by_property_id PRODUCTION_DATE_AS_RECORDED # P23
-    end
-
-    def dated_claims
-      claims_by_property_id DATED # P26
-    end
-
-    def production_place_as_recorded_claims
-      claims_by_property_id PRODUCTION_PLACE_AS_RECORDED # P27
-    end
-
-    def physical_description_claims
-      claims_by_property_id PHYSICAL_DESCRIPTION # P29
-    end
-
-    def material_as_recorded_claims
-      claims_by_property_id MATERIAL_AS_RECORDED # P30
-    end
-
-    def note_claims
-      claims_by_property_id NOTE # P32
-    end
-
-    def acknowledgements_claims
-      claims_by_property_id ACKNOWLEDGEMENTS # P33
-    end
-
-    def date_added_claims
-      claims_by_property_id DATE_ADDED # P34
-    end
-
-    def date_last_updated_claims
-      claims_by_property_id DATE_LAST_UPDATED # P35
-    end
-
-    def iiif_manifest_claims
-      claims_by_property_id IIIF_MANIFEST # P41
-    end
   end
 end

spec/digital_scriptorium/ds_item_spec.rb

@@ -3,32 +3,24 @@
 require 'wikibase_representable'
 
 module DigitalScriptorium
+  include ItemId
   include WikibaseRepresentable::Representers
 
   RSpec.describe DsItem do
-    let(:holding_json) { read_fixture('items/Q542_holding_example.json') }
-    let(:manuscript_json) { read_fixture('items/Q543_manuscript_example.json') }
-    let(:record_json) { read_fixture('items/Q544_record_example.json') }
+    let(:holding) { item_from_fixture('items/Q542_holding_example.json') }
+    let(:manuscript) { item_from_fixture('items/Q543_manuscript_example.json') }
+    let(:record) { item_from_fixture('items/Q544_record_example.json') }
 
-    it 'correctly reports if it is a holding' do
-      item = ItemRepresenter.new(described_class.new).from_json(holding_json)
-      expect(item.holding?).to be true
+    it 'correctly reports holding instance-of item ID' do
+      expect(holding.instance_of).to eq HOLDING
     end
 
-    it 'correctly reports if it is a manuscript' do
-      item = ItemRepresenter.new(described_class.new).from_json(manuscript_json)
-      expect(item.manuscript?).to be true
+    it 'correctly reports manuscript instance-of item ID' do
+      expect(manuscript.instance_of).to eq MANUSCRIPT
     end
 
-    it 'correctly reports if it is a record' do
-      item = ItemRepresenter.new(described_class.new).from_json(record_json)
-      expect(item.record?).to be true
-    end
-
-    it 'returns instance_of (P16) claims' do
-      item = ItemRepresenter.new(described_class.new).from_json(record_json)
-      expect(item.instance_of_claims).not_to be_empty
-      expect(item.instance_of_claims.first.entity_id_value).to eq 'Q3'
+    it 'correctly reports record instance-of item ID' do
+      expect(record.instance_of).to eq DS_20_RECORD
     end
   end
 end

spec/digital_scriptorium/ds_meta_spec.rb

@@ -6,14 +6,11 @@ module DigitalScriptorium
   include WikibaseRepresentable::Representers
 
   RSpec.describe DsMeta do
-    let(:holding) { item_from_fixture('items/Q542_holding_example.json') }
-    let(:manuscript) { item_from_fixture('items/Q543_manuscript_example.json') }
-    let(:record) { item_from_fixture('items/Q544_record_example.json') }
-
     context 'with a record concerning a manuscript with a single current holding' do
       it 'correctly sets the holding' do
+        record = export_hash.fetch('Q544')
         meta = described_class.new(record, export_hash)
-        expect(meta.holding).to eq holding
+        expect(meta.holding).to eq export_hash.fetch('Q542')
       end
     end
 

spec/digital_scriptorium/export_representer_spec.rb

@@ -1,34 +1,27 @@
 # frozen_string_literal: true
 
-require 'wikibase_representable'
-
 module DigitalScriptorium
   include WikibaseRepresentable::Model
-  include WikibaseRepresentable::Representers
 
   RSpec.describe ExportRepresenter do
-    let(:holding_json) { read_fixture('items/Q542_holding_example.json') }
-    let(:manuscript_json) { read_fixture('items/Q543_manuscript_example.json') }
-    let(:record_json) { read_fixture('items/Q544_record_example.json') }
-    let(:property_json) { read_fixture('properties/P16_instance_of.json') }
-    let(:export) do
-      described_class.new(Export.new).from_json("[#{holding_json},#{manuscript_json},#{record_json},#{property_json}]")
+    it 'deserializes holding as a Holding' do
+      expect(export_hash.fetch('Q542')).to be_instance_of Holding
     end
 
-    it 'deserializes holding as a DsItem' do
-      expect(export[0]).to be_instance_of DsItem
+    it 'deserializes manuscript as a Manuscript' do
+      expect(export_hash.fetch('Q543')).to be_instance_of Manuscript
     end
 
-    it 'deserializes manuscript as a DsItem' do
-      expect(export[1]).to be_instance_of DsItem
+    it 'deserializes record as a Record' do
+      expect(export_hash.fetch('Q544')).to be_instance_of Record
     end
 
-    it 'deserializes record as a DsItem' do
-      expect(export[2]).to be_instance_of DsItem
+    it 'deserializes property as a Property' do
+      expect(export_hash.fetch('P16')).to be_instance_of Property
     end
 
-    it 'deserializes property as a Property' do
-      expect(export[3]).to be_instance_of Property
+    it 'deserializes non-core item as a DsItem' do
+      expect(export_hash.fetch('Q129')).to be_instance_of DsItem
     end
   end
 end