-
-
Notifications
You must be signed in to change notification settings - Fork 296
feat: add function and module-based topic generation to PubSub #2139
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
Draft
barnabasJ
wants to merge
2
commits into
main
Choose a base branch
from
feature/relationship-based-pubsub-topics
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
defmodule Ash.Notifier.PubSub.Helpers do | ||
@moduledoc """ | ||
Helper functions for PubSub notifier integration. | ||
""" | ||
|
||
alias Ash.Notifier.PubSub | ||
|
||
@doc """ | ||
Adds required loads from topic generators to a changeset or query. | ||
|
||
This function inspects all PubSub publications configured for the resource | ||
and action, extracts load requirements from topic generators that implement | ||
the `required_loads/0` callback, and adds them to the changeset/query. | ||
|
||
This ensures that when notifications are created, the data already has | ||
the required relationships loaded, preventing N+1 queries in bulk operations. | ||
""" | ||
def add_topic_generator_loads(%Ash.Changeset{} = changeset) do | ||
add_loads_for_action(changeset, changeset.resource, changeset.action) | ||
end | ||
|
||
def add_topic_generator_loads(%Ash.Query{} = query) do | ||
# For queries, we might need to handle this differently | ||
# For now, return unchanged | ||
query | ||
end | ||
|
||
defp add_loads_for_action(changeset_or_query, resource, action) when not is_nil(action) do | ||
# Get all PubSub notifiers for this resource | ||
resource | ||
|> Ash.Resource.Info.notifiers() | ||
|> Enum.filter(&match?(Ash.Notifier.PubSub, &1)) | ||
|> Enum.reduce(changeset_or_query, fn _notifier, acc -> | ||
# Get publications from the notifier | ||
resource | ||
|> PubSub.Info.publications() | ||
|> Enum.filter(&publication_matches?(&1, action)) | ||
|> Enum.reduce(acc, &add_publication_loads/2) | ||
end) | ||
end | ||
|
||
defp add_loads_for_action(changeset_or_query, _resource, _action) do | ||
changeset_or_query | ||
end | ||
|
||
defp publication_matches?(%{action: pub_action}, %{name: action_name}) | ||
when is_atom(pub_action) do | ||
pub_action == action_name | ||
end | ||
|
||
defp publication_matches?(%{type: pub_type}, %{type: action_type}) do | ||
pub_type == action_type | ||
end | ||
|
||
defp publication_matches?(%{type: pub_type, except: except}, %{ | ||
type: action_type, | ||
name: action_name | ||
}) do | ||
pub_type == action_type && action_name not in except | ||
end | ||
|
||
defp publication_matches?(_, _), do: false | ||
|
||
defp add_publication_loads(publication, acc) do | ||
case publication.topic do | ||
{topic_generator, _opts} when is_atom(topic_generator) -> | ||
add_topic_generator_loads_from_module(acc, topic_generator) | ||
|
||
topic_generator when is_atom(topic_generator) -> | ||
add_topic_generator_loads_from_module(acc, topic_generator) | ||
|
||
_ -> | ||
# String, list, or function topics don't have load requirements | ||
acc | ||
end | ||
end | ||
|
||
defp add_topic_generator_loads_from_module(changeset_or_query, topic_generator) do | ||
if function_exported?(topic_generator, :required_loads, 0) do | ||
loads = topic_generator.required_loads() | ||
|
||
# Handle both map and list formats | ||
load_list = | ||
case loads do | ||
%{} = load_map -> | ||
load_map | ||
|> Map.values() | ||
|> List.flatten() | ||
|> Enum.uniq() | ||
|
||
load_list when is_list(load_list) -> | ||
List.flatten([load_list]) | ||
|> Enum.uniq() | ||
|
||
single_load -> | ||
[single_load] | ||
end | ||
|
||
case changeset_or_query do | ||
%Ash.Changeset{} = changeset -> | ||
Ash.Changeset.load(changeset, load_list) | ||
|
||
%Ash.Query{} = query -> | ||
Ash.Query.load(query, load_list) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't just load the things here, because the caller of the action could request the same load key but with different parameters which can lead to non deterministic behavior. This is what I was getting at with needing to add a custom calculation. |
||
end | ||
else | ||
changeset_or_query | ||
end | ||
end | ||
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't have behavior for a specific notifier here, it should be calling a callback on all of the notifiers for this resource and action.