From e9ecd799742569b283848ba0424bc91f1afbd876 Mon Sep 17 00:00:00 2001 From: Kirk Wang Date: Mon, 17 Oct 2022 23:00:46 -0700 Subject: [PATCH 1/4] remove apply thumbnail to manifest and fix specs --- .../manifest_builder/canvas_builder.rb | 5 +-- .../canvas_builder_factory.rb | 4 +-- .../manifest_builder/structure_builder.rb | 2 +- .../v3/manifest_builder/canvas_builder.rb | 10 ++++-- .../v3/manifest_builder/iiif_service.rb | 4 +++ .../record_property_builder.rb | 31 ++----------------- .../v3/manifest_builder/structure_builder.rb | 4 +-- .../manifest_builder/canvas_builder_spec.rb | 10 ++++++ .../iiif_manifest/v3/manifest_factory_spec.rb | 11 ------- 9 files changed, 32 insertions(+), 49 deletions(-) diff --git a/lib/iiif_manifest/manifest_builder/canvas_builder.rb b/lib/iiif_manifest/manifest_builder/canvas_builder.rb index d8ebbc4..ffc7cf6 100644 --- a/lib/iiif_manifest/manifest_builder/canvas_builder.rb +++ b/lib/iiif_manifest/manifest_builder/canvas_builder.rb @@ -1,11 +1,12 @@ module IIIFManifest class ManifestBuilder class CanvasBuilder - attr_reader :record, :parent, :iiif_canvas_factory, :image_builder + attr_reader :record, :parent, :manifest, :iiif_canvas_factory, :image_builder - def initialize(record, parent, iiif_canvas_factory:, image_builder:) + def initialize(record, parent, manifest, iiif_canvas_factory:, image_builder:) @record = record @parent = parent + @manifest = manifest @iiif_canvas_factory = iiif_canvas_factory @image_builder = image_builder apply_record_properties diff --git a/lib/iiif_manifest/manifest_builder/canvas_builder_factory.rb b/lib/iiif_manifest/manifest_builder/canvas_builder_factory.rb index ae736e6..b8200d8 100644 --- a/lib/iiif_manifest/manifest_builder/canvas_builder_factory.rb +++ b/lib/iiif_manifest/manifest_builder/canvas_builder_factory.rb @@ -7,10 +7,10 @@ def initialize(composite_builder:, canvas_builder_factory:) @canvas_builder_factory = canvas_builder_factory end - def from(work) + def from(work, *manifest) composite_builder.new( *file_set_presenters(work).map do |presenter| - canvas_builder_factory.new(presenter, work) + canvas_builder_factory.new(presenter, work, manifest.first) end ) end diff --git a/lib/iiif_manifest/manifest_builder/structure_builder.rb b/lib/iiif_manifest/manifest_builder/structure_builder.rb index dff3b7e..7d6e605 100644 --- a/lib/iiif_manifest/manifest_builder/structure_builder.rb +++ b/lib/iiif_manifest/manifest_builder/structure_builder.rb @@ -78,7 +78,7 @@ def sub_ranges def canvas_builders @canvas_builders ||= file_set_presenters.map do |file_set_presenter| - canvas_builder_factory.new(file_set_presenter, parent) + canvas_builder_factory.new(file_set_presenter, parent, nil) end end diff --git a/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb b/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb index ba97f81..41ee9d7 100644 --- a/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb +++ b/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb @@ -2,11 +2,12 @@ module IIIFManifest module V3 class ManifestBuilder class CanvasBuilder - attr_reader :record, :parent, :iiif_canvas_factory, :content_builder, + attr_reader :record, :parent, :manifest, :iiif_canvas_factory, :content_builder, :choice_builder, :iiif_annotation_page_factory, :thumbnail_builder_factory def initialize(record, parent, + manifest, iiif_canvas_factory:, content_builder:, choice_builder:, @@ -14,6 +15,7 @@ def initialize(record, thumbnail_builder_factory:) @record = record @parent = parent + @manifest = manifest @iiif_canvas_factory = iiif_canvas_factory @content_builder = content_builder @choice_builder = choice_builder @@ -58,15 +60,17 @@ def apply_record_properties canvas.label = ManifestBuilder.language_map(record.to_s) if record.to_s.present? annotation_page['id'] = "#{path}/annotation_page/#{annotation_page.index}" canvas.items = [annotation_page] - apply_thumbnail_to(canvas) + apply_thumbnail_to(manifest, canvas) end - def apply_thumbnail_to(canvas) + def apply_thumbnail_to(manifest, canvas) return unless iiif_endpoint if display_image + manifest&.thumbnail ||= Array(thumbnail_builder_factory.new(display_image).build) canvas.thumbnail = Array(thumbnail_builder_factory.new(display_image).build) elsif display_content.try(:first) + manifest&.thumbnail ||= Array(thumbnail_builder_factory.new(display_content.first).build) canvas.thumbnail = Array(thumbnail_builder_factory.new(display_content.first).build) end end diff --git a/lib/iiif_manifest/v3/manifest_builder/iiif_service.rb b/lib/iiif_manifest/v3/manifest_builder/iiif_service.rb index 6585fb5..325b77e 100644 --- a/lib/iiif_manifest/v3/manifest_builder/iiif_service.rb +++ b/lib/iiif_manifest/v3/manifest_builder/iiif_service.rb @@ -72,6 +72,10 @@ def homepage=(homepage) inner_hash['homepage'] = homepage end + def thumbnail + inner_hash['thumbnail'] + end + def thumbnail=(thumbnail) inner_hash['thumbnail'] = thumbnail end diff --git a/lib/iiif_manifest/v3/manifest_builder/record_property_builder.rb b/lib/iiif_manifest/v3/manifest_builder/record_property_builder.rb index 1cd361f..27f424c 100644 --- a/lib/iiif_manifest/v3/manifest_builder/record_property_builder.rb +++ b/lib/iiif_manifest/v3/manifest_builder/record_property_builder.rb @@ -18,7 +18,7 @@ def initialize(record, def apply(manifest) setup_manifest_from_record(manifest, record) # Build the items array - canvas_builder.apply(manifest.items) + canvas_builder(manifest).apply(manifest.items) manifest end @@ -36,8 +36,8 @@ def populate_rendering private - def canvas_builder - canvas_builder_factory.from(record) + def canvas_builder(manifest) + canvas_builder_factory.from(record, manifest) end # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize, Metrics/MethodLength @@ -56,7 +56,6 @@ def setup_manifest_from_record(manifest, record) manifest.rendering = populate_rendering if populate_rendering.present? homepage = ::IIIFManifest.config.manifest_value_for(record, property: :homepage) manifest.homepage = homepage if homepage.present? - apply_thumbnail_to(manifest) end # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity, Metrics/AbcSize, Metrics/MethodLength @@ -99,30 +98,6 @@ def transform_field(field) metadata_field['value'] = ManifestBuilder.language_map(field['value']) metadata_field end - - def apply_thumbnail_to(manifest) - return unless iiif_endpoint - - if display_image - manifest.thumbnail = Array(thumbnail_builder_factory.new(display_image).build) - elsif display_content - manifest.thumbnail = Array(thumbnail_builder_factory.new(display_content).build) - end - end - - def display_image - return @display_image if defined?(@display_image) - @display_image = record.try(:member_presenters)&.first&.display_image - end - - def display_content - return @display_content if defined?(@display_content) - @display_content = record.try(:member_presenters)&.first&.display_content - end - - def iiif_endpoint - display_image.try(:iiif_endpoint) || Array(display_content).first.try(:iiif_endpoint) - end end end end diff --git a/lib/iiif_manifest/v3/manifest_builder/structure_builder.rb b/lib/iiif_manifest/v3/manifest_builder/structure_builder.rb index ba7bb5a..f6713bd 100644 --- a/lib/iiif_manifest/v3/manifest_builder/structure_builder.rb +++ b/lib/iiif_manifest/v3/manifest_builder/structure_builder.rb @@ -38,7 +38,7 @@ def build_range def canvas_builders @canvas_builders ||= [] unless record.respond_to?(:file_set_presenters) @canvas_builders ||= file_set_presenters.map do |file_set_presenter| - canvas_builder_factory.new(file_set_presenter, parent) + canvas_builder_factory.new(file_set_presenter, parent, nil) end @canvas_builders end @@ -69,7 +69,7 @@ def range_items end def canvas_range_item(range_item) - canvas_builder = canvas_builder_factory.new(range_item, parent) + canvas_builder = canvas_builder_factory.new(range_item, parent, nil) { 'type' => 'Canvas', 'id' => canvas_builder.path } end diff --git a/spec/lib/iiif_manifest/v3/manifest_builder/canvas_builder_spec.rb b/spec/lib/iiif_manifest/v3/manifest_builder/canvas_builder_spec.rb index 6b9fa3f..240e929 100644 --- a/spec/lib/iiif_manifest/v3/manifest_builder/canvas_builder_spec.rb +++ b/spec/lib/iiif_manifest/v3/manifest_builder/canvas_builder_spec.rb @@ -9,6 +9,7 @@ described_class.new( record, parent, + manifest, iiif_canvas_factory: IIIFManifest::V3::ManifestBuilder::IIIFManifest::Canvas, content_builder: content_builder, choice_builder: IIIFManifest::V3::ManifestBuilder::ChoiceBuilder, @@ -57,6 +58,7 @@ def id allow(content_builder).to receive(:new).and_return(built_content) allow(thumbnail_builder).to receive(:build).and_return(iiif_thumbnail) allow(thumbnail_builder_factory).to receive(:new).and_return(thumbnail_builder) + allow(manifest).to receive(:thumbnail).and_return(iiif_thumbnail) end let(:iiif_body) do @@ -102,6 +104,10 @@ def id double('Thumbnail Builder') end + let(:manifest) do + instance_double(IIIFManifest::V3::ManifestBuilder::IIIFManifest) + end + let(:built_content) do IIIFManifest::V3::ManifestBuilder::ContentBuilder.new( record.display_content, @@ -149,6 +155,10 @@ def id expect(values).to include "width" => "100px" expect(values).to include "height" => "100px" expect(values).to include "duration" => nil + expect(manifest.thumbnail['type']).to eq 'Image' + expect(manifest.thumbnail['width']).to eq 200 + expect(manifest.thumbnail['height']).to eq 150 + expect(manifest.thumbnail['duration']).to be_nil end end diff --git a/spec/lib/iiif_manifest/v3/manifest_factory_spec.rb b/spec/lib/iiif_manifest/v3/manifest_factory_spec.rb index d7671dc..fb47e58 100644 --- a/spec/lib/iiif_manifest/v3/manifest_factory_spec.rb +++ b/spec/lib/iiif_manifest/v3/manifest_factory_spec.rb @@ -156,17 +156,6 @@ def display_content expect(result['items'].first['items'].first['items'].first['body']['format']).to eq 'image/jpeg' end - it 'has a thumbnail property' do - allow(book_presenter).to receive(:member_presenters).and_return([file_presenter]) - - thumbnail = result['thumbnail'].first - expect(thumbnail['id']).to eq 'test.host/images/image-77/full/!200,200/0/default.jpg' - expect(thumbnail['height']).to eq 150 - expect(thumbnail['width']).to eq 200 - expect(thumbnail['format']).to eq 'image/jpeg' - expect(thumbnail['service'].first).to be_kind_of IIIFManifest::V3::ManifestBuilder::IIIFService - end - it 'builds a structure if it can' do allow(book_presenter).to receive(:file_set_presenters).and_return([file_presenter]) allow(book_presenter.ranges[0].ranges[0]).to receive(:file_set_presenters).and_return([file_presenter]) From bfca0b0f05e23d7fbd9e6ae251e089142510858d Mon Sep 17 00:00:00 2001 From: Kirk Wang Date: Thu, 20 Oct 2022 19:26:52 -0700 Subject: [PATCH 2/4] allow thumbnails to have more than one thumbnail on manifest level --- .../v3/manifest_builder/canvas_builder.rb | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb b/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb index 41ee9d7..32eee81 100644 --- a/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb +++ b/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb @@ -63,17 +63,33 @@ def apply_record_properties apply_thumbnail_to(manifest, canvas) end + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity def apply_thumbnail_to(manifest, canvas) return unless iiif_endpoint if display_image - manifest&.thumbnail ||= Array(thumbnail_builder_factory.new(display_image).build) + if collection? + manifest&.thumbnail = [] if manifest&.thumbnail.nil? + manifest.thumbnail << Array(thumbnail_builder_factory.new(display_image).build) + else + manifest&.thumbnail ||= Array(thumbnail_builder_factory.new(display_image).build) + end canvas.thumbnail = Array(thumbnail_builder_factory.new(display_image).build) elsif display_content.try(:first) - manifest&.thumbnail ||= Array(thumbnail_builder_factory.new(display_content.first).build) + if collection? + manifest&.thumbnail = [] if manifest&.thumbnail.nil? + manifest.thumbnail << Array(thumbnail_builder_factory.new(display_content.first).build) + else + manifest&.thumbnail ||= Array(thumbnail_builder_factory.new(display_content.first).build) + end canvas.thumbnail = Array(thumbnail_builder_factory.new(display_content.first).build) end end + # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity + + def collection? + manifest.class.inspect == 'IIIFManifest::V3::ManifestBuilder::IIIFManifest::Collection' + end def annotation_page @annotation_page ||= iiif_annotation_page_factory.new From dc8ce0611f58e0cbae840a5acbb06dc892666fc8 Mon Sep 17 00:00:00 2001 From: Kirk Wang Date: Thu, 20 Oct 2022 20:45:54 -0700 Subject: [PATCH 3/4] refactor --- .../v3/manifest_builder/canvas_builder.rb | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb b/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb index 32eee81..5a8abcd 100644 --- a/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb +++ b/lib/iiif_manifest/v3/manifest_builder/canvas_builder.rb @@ -55,6 +55,10 @@ def display_content Array.wrap(record.display_content) if record.respond_to?(:display_content) && record.display_content.present? end + def manifest_can_have_thumbnail + manifest.respond_to?(:thumbnail) + end + def apply_record_properties canvas['id'] = path canvas.label = ManifestBuilder.language_map(record.to_s) if record.to_s.present? @@ -63,32 +67,31 @@ def apply_record_properties apply_thumbnail_to(manifest, canvas) end - # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity def apply_thumbnail_to(manifest, canvas) return unless iiif_endpoint if display_image - if collection? - manifest&.thumbnail = [] if manifest&.thumbnail.nil? - manifest.thumbnail << Array(thumbnail_builder_factory.new(display_image).build) - else - manifest&.thumbnail ||= Array(thumbnail_builder_factory.new(display_image).build) - end + apply_manifest_thumbnail(manifest, display_image) canvas.thumbnail = Array(thumbnail_builder_factory.new(display_image).build) elsif display_content.try(:first) - if collection? - manifest&.thumbnail = [] if manifest&.thumbnail.nil? - manifest.thumbnail << Array(thumbnail_builder_factory.new(display_content.first).build) - else - manifest&.thumbnail ||= Array(thumbnail_builder_factory.new(display_content.first).build) - end + apply_manifest_thumbnail(manifest, display_content.first) canvas.thumbnail = Array(thumbnail_builder_factory.new(display_content.first).build) end end - # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity def collection? - manifest.class.inspect == 'IIIFManifest::V3::ManifestBuilder::IIIFManifest::Collection' + manifest.is_a? IIIFManifest::Collection + end + + def apply_manifest_thumbnail(manifest, display_type) + return unless manifest_can_have_thumbnail + + if collection? + # if manifest.thumbnail is nil, make it an Array, then add more thumbnails into it + (manifest.thumbnail ||= []) << Array(thumbnail_builder_factory.new(display_type).build) + else + manifest.thumbnail ||= Array(thumbnail_builder_factory.new(display_type).build) + end end def annotation_page From 113a1d962ad0d3f93abfdae8621f0e0e7f12d53a Mon Sep 17 00:00:00 2001 From: Kirk Wang Date: Thu, 20 Oct 2022 22:40:58 -0700 Subject: [PATCH 4/4] adjust spec --- .../manifest_builder/canvas_builder_spec.rb | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/spec/lib/iiif_manifest/v3/manifest_builder/canvas_builder_spec.rb b/spec/lib/iiif_manifest/v3/manifest_builder/canvas_builder_spec.rb index 240e929..0d72427 100644 --- a/spec/lib/iiif_manifest/v3/manifest_builder/canvas_builder_spec.rb +++ b/spec/lib/iiif_manifest/v3/manifest_builder/canvas_builder_spec.rb @@ -58,7 +58,7 @@ def id allow(content_builder).to receive(:new).and_return(built_content) allow(thumbnail_builder).to receive(:build).and_return(iiif_thumbnail) allow(thumbnail_builder_factory).to receive(:new).and_return(thumbnail_builder) - allow(manifest).to receive(:thumbnail).and_return(iiif_thumbnail) + # allow(manifest).to receive(:thumbnail).and_return(iiif_thumbnail) end let(:iiif_body) do @@ -105,7 +105,7 @@ def id end let(:manifest) do - instance_double(IIIFManifest::V3::ManifestBuilder::IIIFManifest) + IIIFManifest::V3::ManifestBuilder::IIIFManifest.new end let(:built_content) do @@ -155,10 +155,17 @@ def id expect(values).to include "width" => "100px" expect(values).to include "height" => "100px" expect(values).to include "duration" => nil - expect(manifest.thumbnail['type']).to eq 'Image' - expect(manifest.thumbnail['width']).to eq 200 - expect(manifest.thumbnail['height']).to eq 150 - expect(manifest.thumbnail['duration']).to be_nil + end + + it 'sets the manifest thumbnail as well' do + builder.canvas + expect(manifest.thumbnail).to be_an Array + manifest_thumbnail = manifest.thumbnail.first + expect(manifest_thumbnail).to be_a IIIFManifest::V3::ManifestBuilder::IIIFManifest::Thumbnail + expect(manifest_thumbnail['type']).to eq 'Image' + expect(manifest_thumbnail['width']).to eq 200 + expect(manifest_thumbnail['height']).to eq 150 + expect(manifest_thumbnail['duration']).to be_nil end end