Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Index primary thumbnail id #304

Merged
merged 6 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,38 @@ defmodule DpulCollections.IndexingPipeline.Figgy.HydrationCacheEntry do
years_is: extract_years(data),
display_date_s: format_date(metadata),
page_count_i: page_count(metadata),
image_service_urls_ss: image_service_urls(metadata, related_data)
image_service_urls_ss: image_service_urls(metadata, related_data),
primary_thumbnail_service_url_s: primary_thumbnail_service_url(metadata, related_data)
}
end

defp primary_thumbnail_service_url(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it's more readable, but I wonder if a bunch of these conditionals are pattern matchable?

Like

defp primary_thumbnail_service_url(%{"thumbnail_id" => [%{"id" => id}], %{"member_ids" => %{id => stuff}})

for the case where there's an id key (this might not be possible, I dunno if you can match like this...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried similar approaches and could not get matches to happen.

%{"thumbnail_id" => thumbnail_id} = metadata,
%{"member_ids" => member_data} = related_data
)
when is_list(thumbnail_id) do
thumbnail_member =
thumbnail_id
|> Enum.at(0, %{})
|> Map.get("id")
|> then(fn id -> member_data[id] end)

if is_nil(thumbnail_member) do
# When thumbnail id does not correspond to a related FileSet,
# use the first image service url
image_service_urls(metadata, related_data)
|> Enum.at(0)
else
extract_service_url(thumbnail_member)
end
end

defp primary_thumbnail_service_url(metadata, related_data) do
# When the thumbnail id is not set, use first image service url
image_service_urls(metadata, related_data)
|> Enum.at(0)
end

defp image_service_urls(%{"member_ids" => member_ids}, related_data) do
member_ids
|> Enum.map(&extract_service_url(&1, related_data))
Expand Down
2 changes: 2 additions & 0 deletions lib/dpul_collections/item.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defmodule DpulCollections.Item do
:page_count,
:url,
:image_service_urls,
:primary_thumbnail_service_url,
:description
]

Expand All @@ -25,6 +26,7 @@ defmodule DpulCollections.Item do
page_count: doc["page_count_i"],
url: generate_url(id, slug),
image_service_urls: doc["image_service_urls_ss"] || [],
primary_thumbnail_service_url: doc["primary_thumbnail_service_url_s"],
description: doc["description_txtm"] || []
}
end
Expand Down
3 changes: 2 additions & 1 deletion lib/dpul_collections/solr.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ defmodule DpulCollections.Solr do
"page_count_i",
"detectlang_ss",
"slug_s",
"image_service_urls_ss"
"image_service_urls_ss",
"primary_thumbnail_service_url_s"
]

@spec query(map(), String.t()) :: map()
Expand Down
2 changes: 1 addition & 1 deletion lib/dpul_collections_web/live/item_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ defmodule DpulCollectionsWeb.ItemLive do
<div class="primary-thumbnail md:col-span-2 md:order-first">
<img
class="w-full"
src={"#{Enum.at(@item.image_service_urls, 0)}/full/525,800/0/default.jpg"}
src={"#{@item.primary_thumbnail_service_url}/full/525,800/0/default.jpg"}
alt="main image display"
style="
background-color: lightgray;"
Expand Down
26 changes: 25 additions & 1 deletion lib/dpul_collections_web/live/search_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ defmodule DpulCollectionsWeb.SearchLive do
<div id={"item-#{@item.id}"} class="item">
<div class="flex flex-wrap gap-5 md:max-h-60 max-h-[22rem] overflow-hidden justify-center md:justify-start relative">
<.thumbs
:for={{thumb, thumb_num} <- Enum.with_index(@item.image_service_urls)}
:for={{thumb, thumb_num} <- thumbnail_service_urls(5, @item)}
:if={@item.page_count}
thumb={thumb}
thumb_num={thumb_num}
Expand Down Expand Up @@ -343,4 +343,28 @@ defmodule DpulCollectionsWeb.SearchLive do
type == :post -> [{"...", false}, {page, false}]
end
end

defp thumbnail_service_urls(max_thumbnails, item) do
thumbnail_service_urls(
max_thumbnails,
item.image_service_urls,
item.primary_thumbnail_service_url
)
end

defp thumbnail_service_urls(max_thumbnails, image_service_urls, nil) do
# Truncate image service urls to max value
image_service_urls
|> Enum.slice(0, max_thumbnails)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😂 Yeah I was wondering about this, were we just putting every thumbnail on the page? I guess it was performing.

|> Enum.with_index()
end

defp thumbnail_service_urls(max_thumbnails, image_service_urls, primary_thumbnail_service_url) do
# Move thumbnail url to front of list and then truncate to max value
image_service_urls
|> Enum.filter(&(&1 != primary_thumbnail_service_url))
|> List.insert_at(0, primary_thumbnail_service_url)
|> Enum.slice(0, max_thumbnails)
|> Enum.with_index()
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ defmodule DpulCollections.IndexingPipeline.Figgy.HydrationCacheEntryTest do
"internal_resource" => "EphemeraFolder",
"metadata" => %{
"member_ids" => [%{"id" => "1"}],
"title" => ["test title 4"]
"title" => ["test title 4"],
"thumbnail_id" => [%{"id" => "1"}]
}
}
})
Expand All @@ -79,6 +80,115 @@ defmodule DpulCollections.IndexingPipeline.Figgy.HydrationCacheEntryTest do
assert doc[:image_service_urls_ss] == [
"https://iiif-cloud.princeton.edu/iiif/2/0c%2Fff%2F89%2F0cff895a01ea48959c3da8c6eaab4017%2Fintermediate_file"
]

# Has thumbnail url
assert doc[:primary_thumbnail_service_url_s] ==
"https://iiif-cloud.princeton.edu/iiif/2/0c%2Fff%2F89%2F0cff895a01ea48959c3da8c6eaab4017%2Fintermediate_file"
end

test "uses first image service url when there is no thumbnail_id property" do
{:ok, entry} =
IndexingPipeline.write_hydration_cache_entry(%{
cache_version: 0,
record_id: "0cff895a-01ea-4895-9c3d-a8c6eaab4013",
source_cache_order: ~U[2018-03-09 20:19:35.465203Z],
related_data: %{
"member_ids" => %{
"1" => %{
"internal_resource" => "FileSet",
"id" => "9ad621a7b-01ea-4895-9c3d-a8c6eaab4013",
"metadata" => %{
"file_metadata" => [
%{
"id" => %{"id" => "0cff895a-01ea-4895-9c3d-a8c6eaab4017"},
"internal_resource" => "FileMetadata",
"mime_type" => ["image/tiff"],
"use" => [%{"@id" => "http://pcdm.org/use#ServiceFile"}]
}
]
}
}
}
},
data: %{
"id" => "0cff895a-01ea-4895-9c3d-a8c6eaab4013",
"internal_resource" => "EphemeraFolder",
"metadata" => %{
"member_ids" => [%{"id" => "1"}],
"title" => ["test title 4"]
}
}
})

doc = HydrationCacheEntry.to_solr_document(entry)

assert doc[:primary_thumbnail_service_url_s] ==
"https://iiif-cloud.princeton.edu/iiif/2/0c%2Fff%2F89%2F0cff895a01ea48959c3da8c6eaab4017%2Fintermediate_file"
end

test "uses first image service url when thumbnail id does not point to related FileSet" do
{:ok, entry} =
IndexingPipeline.write_hydration_cache_entry(%{
cache_version: 0,
record_id: "0cff895a-01ea-4895-9c3d-a8c6eaab4013",
source_cache_order: ~U[2018-03-09 20:19:35.465203Z],
related_data: %{
"member_ids" => %{
"1" => %{
"internal_resource" => "FileSet",
"id" => "9ad621a7b-01ea-4895-9c3d-a8c6eaab4013",
"metadata" => %{
"file_metadata" => [
%{
"id" => %{"id" => "0cff895a-01ea-4895-9c3d-a8c6eaab4017"},
"internal_resource" => "FileMetadata",
"mime_type" => ["image/tiff"],
"use" => [%{"@id" => "http://pcdm.org/use#ServiceFile"}]
}
]
}
}
}
},
data: %{
"id" => "0cff895a-01ea-4895-9c3d-a8c6eaab4013",
"internal_resource" => "EphemeraFolder",
"metadata" => %{
"member_ids" => [%{"id" => "1"}],
"title" => ["test title 4"],
"thumbnail_id" => [%{"id" => "9"}]
}
}
})

doc = HydrationCacheEntry.to_solr_document(entry)

assert doc[:primary_thumbnail_service_url_s] ==
"https://iiif-cloud.princeton.edu/iiif/2/0c%2Fff%2F89%2F0cff895a01ea48959c3da8c6eaab4017%2Fintermediate_file"
end

test "does not add a thumbnail service url when there are no image members" do
{:ok, entry} =
IndexingPipeline.write_hydration_cache_entry(%{
cache_version: 0,
record_id: "0cff895a-01ea-4895-9c3d-a8c6eaab4013",
source_cache_order: ~U[2018-03-09 20:19:35.465203Z],
related_data: %{
"member_ids" => %{}
},
data: %{
"id" => "0cff895a-01ea-4895-9c3d-a8c6eaab4013",
"internal_resource" => "EphemeraFolder",
"metadata" => %{
"member_ids" => [],
"title" => ["test title 4"]
}
}
})

doc = HydrationCacheEntry.to_solr_document(entry)

assert doc[:primary_thumbnail_service_url_s] == nil
end

test "can handle when members do not have the correct file metadata type" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,5 +196,8 @@ defmodule DpulCollections.IndexingPipeline.FiggyFullIntegrationTest do
"https://iiif-cloud.princeton.edu/iiif/2/5e%2F24%2Faf%2F5e24aff45b2e4c9aaba3f05321d1c797%2Fintermediate_file"
| _rest
] = document["image_service_urls_ss"]

assert "https://iiif-cloud.princeton.edu/iiif/2/5e%2F24%2Faf%2F5e24aff45b2e4c9aaba3f05321d1c797%2Fintermediate_file" =
document["primary_thumbnail_service_url_s"]
end
end
11 changes: 7 additions & 4 deletions test/dpul_collections_web/live/item_live_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ defmodule DpulCollectionsWeb.ItemLiveTest do
"https://example.com/iiif/2/image1",
"https://example.com/iiif/2/image2"
],
primary_thumbnail_service_url_s: "https://example.com/iiif/2/image2",
description_txtm: ["This is a test description"]
},
%{
Expand All @@ -29,7 +30,8 @@ defmodule DpulCollectionsWeb.ItemLiveTest do
image_service_urls_ss: [
"https://example.com/iiif/2/image1",
"https://example.com/iiif/2/image2"
]
],
primary_thumbnail_service_url_s: "https://example.com/iiif/2/image1"
},
%{
id: 3,
Expand All @@ -39,7 +41,8 @@ defmodule DpulCollectionsWeb.ItemLiveTest do
image_service_urls_ss: [
"https://example.com/iiif/2/image1",
"https://example.com/iiif/2/image2"
]
],
primary_thumbnail_service_url_s: "https://example.com/iiif/2/image1"
}
],
active_collection()
Expand Down Expand Up @@ -108,10 +111,10 @@ defmodule DpulCollectionsWeb.ItemLiveTest do
"Download"
)

# Large thumbnail renders
# Large thumbnail renders using thumbnail service url
assert view
|> has_element?(
".primary-thumbnail img[src='https://example.com/iiif/2/image1/full/525,800/0/default.jpg']"
".primary-thumbnail img[src='https://example.com/iiif/2/image2/full/525,800/0/default.jpg']"
)

assert view
Expand Down
45 changes: 28 additions & 17 deletions test/dpul_collections_web/live/search_live_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -53,27 +53,38 @@ defmodule DpulCollectionsWeb.SearchLiveTest do
end

test "GET /search renders thumbnails for each resource", %{conn: conn} do
{:ok, view, _html} = live(conn, "/search?")
{:ok, _view, html} = live(conn, "/search?")

assert view
|> has_element?(
"#item-1 img[src='https://example.com/iiif/2/image1/square/350,350/0/default.jpg']"
)
{:ok, document} =
html
|> Floki.parse_document()

assert view
|> has_element?(
"#item-1 img[src='https://example.com/iiif/2/image2/square/350,350/0/default.jpg']"
)
# There should be a maximum of 5 thumbnails on the search results page
assert document |> Floki.find("#item-1 > div > img") |> Enum.count() == 5

assert view
|> has_element?(
"#item-2 img[src='https://example.com/iiif/2/image1/square/350,350/0/default.jpg']"
)
# Odd numbered documents in test data do not have a thumbnail id
# so the order of thumbnails should be the same as the image member order
assert document
|> Floki.attribute("#item-1 > div > :first-child", "src") == [
"https://example.com/iiif/2/image1/square/350,350/0/default.jpg"
]

assert view
|> has_element?(
"#item-2 img[src='https://example.com/iiif/2/image2/square/350,350/0/default.jpg']"
)
assert document
|> Floki.attribute("#item-1 > div > :nth-child(2)", "src") == [
"https://example.com/iiif/2/image2/square/350,350/0/default.jpg"
]

# Even numbered documents in test data have a thumbnail id so the order
# of thumbnails should be different from the image member order
assert document
|> Floki.attribute("#item-2 > div > :first-child", "src") == [
"https://example.com/iiif/2/image2/square/350,350/0/default.jpg"
]

assert document
|> Floki.attribute("#item-2 > div > :nth-child(2)", "src") == [
"https://example.com/iiif/2/image1/square/350,350/0/default.jpg"
]
end

test "searching filters results", %{conn: conn} do
Expand Down
22 changes: 19 additions & 3 deletions test/support/solr_test_support.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@ defmodule SolrTestSupport do
def mock_solr_documents(count \\ 100) do
for n <- 1..count do
date = 2025 - n
page_count = Enum.random(1..10)

# Equals the number of image service urls
page_count = 7

# Assign thumbnail urls to even numbered documents.
# Used for testing thumbnail rendering order
thumbnail_url =
cond do
rem(n, 2) == 0 -> "https://example.com/iiif/2/image2"
true -> nil
end

%{
id: n,
Expand All @@ -12,8 +22,14 @@ defmodule SolrTestSupport do
page_count_i: page_count,
image_service_urls_ss: [
"https://example.com/iiif/2/image1",
"https://example.com/iiif/2/image2"
]
"https://example.com/iiif/2/image2",
"https://example.com/iiif/2/image3",
"https://example.com/iiif/2/image4",
"https://example.com/iiif/2/image5",
"https://example.com/iiif/2/image6",
"https://example.com/iiif/2/image7"
],
primary_thumbnail_service_url_s: thumbnail_url
}
end
end
Expand Down