Skip to content

Merge the dataloaders together #3713

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.

Sign up for GitHub

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

Draft
richardTowers wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Merge the dataloaders together #3713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions app/graphql/sources/linked_to_editions_source.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ def initialize(content_store:, locale:)
def fetch(editions_and_link_types)
link_types_map = {}
query_input = []
editions_and_link_types.each do |edition, link_type|
query_input.push({ edition_id: edition.id, content_id: edition.content_id, link_type: })
link_types_map[[edition.content_id, link_type]] = []
editions_and_link_types.each do |edition, link_type, direction|
query_input.push({ edition_id: edition.id, content_id: edition.content_id, link_type:, direction: })
link_types_map[[edition.content_id, link_type, direction.to_s]] = []
end

sql_params = {
Expand All @@ -30,8 +30,8 @@ def fetch(editions_and_link_types)
all_editions = Edition.find_by_sql([SQL, sql_params])
all_editions.each(&:strict_loading!)
all_editions.each_with_object(link_types_map) { |edition, hash|
unless hash[[edition.source_content_id, edition.link_type]].include?(edition)
hash[[edition.source_content_id, edition.link_type]] << edition
unless hash[[edition.requested_content_id, edition.link_type, edition.direction]].include?(edition)
hash[[edition.requested_content_id, edition.link_type, edition.direction]] << edition
end
}.values
end
Expand Down
107 changes: 95 additions & 12 deletions app/graphql/sources/queries/linked_to_editions.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ WITH query_input AS (
json_to_recordset(:query_input::json) AS query_input (
edition_id integer,
content_id uuid,
link_type varchar
link_type varchar,
direction varchar
)
LIMIT (:query_input_count) -- noqa: AM09
),

edition_linked_editions AS (
direct_edition_linked_editions AS (
SELECT DISTINCT ON (links.id)
editions.*,
links.link_type,
Expand All @@ -19,12 +20,18 @@ edition_linked_editions AS (
documents.content_id,
documents.locale,
documents.locale =:primary_locale AS is_primary_locale,
source_documents.content_id AS source_content_id
source_documents.content_id AS requested_content_id,
query_input.direction
FROM editions
INNER JOIN documents ON editions.document_id = documents.id
INNER JOIN links ON documents.content_id = links.target_content_id
INNER JOIN editions AS source_editions ON links.edition_id = source_editions.id
INNER JOIN query_input ON source_editions.id = query_input.edition_id AND links.link_type = query_input.link_type
INNER JOIN
query_input
ON
query_input.direction = 'direct'
AND source_editions.id = query_input.edition_id
AND links.link_type = query_input.link_type
INNER JOIN documents AS source_documents ON source_editions.document_id = source_documents.id
WHERE
editions.content_store =:content_store
Expand All @@ -37,7 +44,7 @@ edition_linked_editions AS (
ORDER BY links.id ASC, is_primary_locale DESC
),

link_set_linked_editions AS (
direct_link_set_linked_editions AS (
SELECT DISTINCT ON (links.id)
editions.*,
links.link_type,
Expand All @@ -46,13 +53,17 @@ link_set_linked_editions AS (
documents.content_id,
documents.locale,
documents.locale =:primary_locale AS is_primary_locale,
links.link_set_content_id AS source_content_id
links.link_set_content_id AS requested_content_id,
query_input.direction
FROM editions
INNER JOIN documents ON editions.document_id = documents.id
INNER JOIN links ON documents.content_id = links.target_content_id
INNER JOIN
query_input
ON links.link_set_content_id = query_input.content_id AND links.link_type = query_input.link_type
ON
query_input.direction = 'direct'
AND links.link_set_content_id = query_input.content_id
AND links.link_type = query_input.link_type
WHERE
editions.content_store =:content_store
AND documents.locale IN (:primary_locale,:secondary_locale)
Expand All @@ -63,18 +74,90 @@ link_set_linked_editions AS (
)
-- skip any links that we already found in edition_linked_editions:
AND NOT EXISTS (
SELECT FROM edition_linked_editions
SELECT FROM direct_edition_linked_editions
WHERE
edition_linked_editions.source_content_id = links.link_set_content_id
AND edition_linked_editions.link_type = links.link_type
direct_edition_linked_editions.requested_content_id = links.link_set_content_id
AND direct_edition_linked_editions.link_type = links.link_type
)
ORDER BY links.id ASC, is_primary_locale DESC
),

reverse_edition_linked_editions AS (
-- NOTE: we're not using DISTINCT ON (links.id) here because the tests check that
-- if we have multiple links of the same link_type / target_content_id pointing
-- at editions of the same document with different locales, we should only get
-- the document with the best locale (rather than all of them).
-- It's not clear if the behaviour we're testing for is correct though.
-- Reverse edition links are a niche feature.
SELECT DISTINCT ON (documents.content_id, links.link_type, links.target_content_id)
editions.*,
links.link_type,
links.position,
links.id AS link_id,
documents.content_id,
documents.locale,
documents.locale =:primary_locale AS is_primary_locale,
links.target_content_id AS requested_content_id,
query_input.direction
FROM editions
INNER JOIN documents ON editions.document_id = documents.id
INNER JOIN links ON editions.id = links.edition_id
INNER JOIN
query_input
ON
query_input.direction = 'reverse'
AND links.target_content_id = query_input.content_id
AND links.link_type = query_input.link_type
WHERE
editions.content_store =:content_store
AND documents.locale IN (:primary_locale,:secondary_locale)
AND editions.document_type NOT IN (:non_renderable_formats)
AND (
links.link_type IN (:unpublished_link_types)
OR editions.state != 'unpublished'
)
ORDER BY documents.content_id ASC, links.link_type ASC, links.target_content_id ASC, is_primary_locale DESC
),

reverse_link_set_linked_editions AS (
SELECT DISTINCT ON (links.id)
editions.*,
links.link_type,
links.position,
links.id AS link_id,
documents.content_id,
documents.locale,
documents.locale =:primary_locale AS is_primary_locale,
links.target_content_id AS requested_content_id,
query_input.direction
FROM editions
INNER JOIN documents ON editions.document_id = documents.id
INNER JOIN links ON documents.content_id = links.link_set_content_id
INNER JOIN
query_input
ON
query_input.direction = 'reverse'
AND links.target_content_id = query_input.content_id
AND links.link_type = query_input.link_type
WHERE
editions.content_store =:content_store
AND documents.locale IN (:primary_locale,:secondary_locale)
AND editions.document_type NOT IN (:non_renderable_formats)
AND (
links.link_type IN (:unpublished_link_types)
OR editions.state != 'unpublished'
)
ORDER BY links.id ASC, is_primary_locale DESC
)

SELECT editions.* FROM (
SELECT * FROM link_set_linked_editions
SELECT * FROM direct_link_set_linked_editions
UNION ALL
SELECT * FROM direct_edition_linked_editions
UNION ALL
SELECT * FROM reverse_link_set_linked_editions
UNION ALL
SELECT * FROM edition_linked_editions
SELECT * FROM reverse_edition_linked_editions
) AS editions
ORDER BY
editions.link_type ASC, editions.position ASC, editions.link_id DESC
74 changes: 0 additions & 74 deletions app/graphql/sources/queries/reverse_linked_to_editions.sql
View file
Open in desktop

This file was deleted.

40 changes: 0 additions & 40 deletions app/graphql/sources/reverse_linked_to_editions_source.rb
View file
Open in desktop

This file was deleted.

6 changes: 3 additions & 3 deletions app/graphql/types/base_object.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def self.links_field(link_type, graphql_field_type)
content_store: object.content_store,
locale: context[:root_edition].locale,
)
.load([object, link_type.to_s])
.load([object, link_type.to_s, :direct])
end
end

Expand All @@ -24,11 +24,11 @@ def self.reverse_links_field(field_name, link_type, graphql_field_type)

define_method(field_name.to_sym) do
dataloader.with(
Sources::ReverseLinkedToEditionsSource,
Sources::LinkedToEditionsSource,
content_store: object.content_store,
locale: context[:root_edition].locale,
)
.load([object, link_type.to_s])
.load([object, link_type.to_s, :reverse])
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions app/graphql/types/edition_type.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,18 @@ class EditionLinks < Types::BaseObject
def role_appointments
if %w[role ministerial_role].include?(object.document_type)
dataloader.with(
Sources::ReverseLinkedToEditionsSource,
Sources::LinkedToEditionsSource,
content_store: object.content_store,
locale: context[:root_edition].locale,
)
.load([object, "role"])
.load([object, "role", :reverse])
else
dataloader.with(
Sources::ReverseLinkedToEditionsSource,
Sources::LinkedToEditionsSource,
content_store: object.content_store,
locale: context[:root_edition].locale,
)
.load([object, "person"])
.load([object, "person", :reverse])
end
end

Expand Down
Loading