diff --git a/.formatter.exs b/.formatter.exs index e808b92..ce2b786 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -3,9 +3,9 @@ # SPDX-License-Identifier: MIT spark_locals_without_parens = [ + guard: 1, label: 1, relate: 1, - guard: 1, skip: 1 ] diff --git a/CHANGELOG.md b/CHANGELOG.md index 6565208..c4bc704 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,18 @@ See [Conventional Commits](Https://conventionalcommits.org) for commit guideline +## [v0.5.1](https://github.com/diffo-dev/ash_neo4j/compare/v0.5.0...v0.5.1) (2026-05-10) + +### Improvements + +* **Documentation** (#249) — ex_doc configuration overhauled: extras reorganised with titled entries, module groups defined for AshNeo4j, Introspection, Cypher, Utilities and Internals, Livebook added to How To, CHANGELOG included in About AshNeo4j, maintainer contact updated. + +### Bug Fixes + +* **Aggregate filters honoured** (#252) — filters declared via `filter expr(...)` on aggregate definitions were silently dropped. Filtered aggregates now load full destination records in Elixir and apply `Ash.Filter.Runtime.filter_matches/3` per source group before reducing. The fast Cypher push-down path is preserved for unfiltered aggregates. + +* **Aggregate names with `?` suffix** (#251) — aggregate names following the Elixir predicate convention (e.g. `exists :cvc_defined?, :characteristics`) caused Neo4j to reject the generated Cypher with an invalid identifier error. Column aliases are now backtick-quoted, allowing any valid Elixir atom as an aggregate name. + ## [v0.5.0](https://github.com/diffo-dev/ash_neo4j/compare/v0.4.1...v0.5.0) (2026-05-08) ### Features @@ -28,7 +40,7 @@ See [Conventional Commits](Https://conventionalcommits.org) for commit guideline ## [v0.4.1](https://github.com/diffo-dev/ash_neo4j/compare/v0.4.0...v0.4.1) (2026-05-06) -## What's Changed +### What's Changed * fix in_transaction? by @matt-beanland in https://github.com/diffo-dev/ash_neo4j/pull/226 * fixed sandbox and non-sandbox paths by @matt-beanland in https://github.com/diffo-dev/ash_neo4j/pull/227 * fix unhandled throws by @matt-beanland in https://github.com/diffo-dev/ash_neo4j/pull/228 diff --git a/lib/cypher.ex b/lib/cypher.ex index b2db8f3..da3c485 100644 --- a/lib/cypher.ex +++ b/lib/cypher.ex @@ -13,8 +13,21 @@ defmodule AshNeo4j.Cypher do require Logger alias AshNeo4j.Cypher.{ - Query, Match, OptionalMatch, Create, Merge, Where, With, - Set, Remove, Delete, DetachDelete, Return, OrderBy, Skip, Limit + Query, + Match, + OptionalMatch, + Create, + Merge, + Where, + With, + Set, + Remove, + Delete, + DetachDelete, + Return, + OrderBy, + Skip, + Limit } @spec remove_properties(atom(), maybe_improper_list()) :: binary() diff --git a/lib/cypher/query.ex b/lib/cypher/query.ex index 2e38dd9..532f78e 100644 --- a/lib/cypher/query.ex +++ b/lib/cypher/query.ex @@ -104,16 +104,39 @@ defmodule AshNeo4j.Cypher.Query do """ alias AshNeo4j.Cypher + alias AshNeo4j.Cypher.{ - Match, OptionalMatch, Create, Merge, Where, With, - Set, Remove, Delete, DetachDelete, Return, OrderBy, Skip, Limit + Match, + OptionalMatch, + Create, + Merge, + Where, + With, + Set, + Remove, + Delete, + DetachDelete, + Return, + OrderBy, + Skip, + Limit } @type clause :: - Match.t() | OptionalMatch.t() | Create.t() | Merge.t() - | Where.t() | With.t() | Set.t() | Remove.t() - | Delete.t() | DetachDelete.t() | Return.t() - | OrderBy.t() | Skip.t() | Limit.t() + Match.t() + | OptionalMatch.t() + | Create.t() + | Merge.t() + | Where.t() + | With.t() + | Set.t() + | Remove.t() + | Delete.t() + | DetachDelete.t() + | Return.t() + | OrderBy.t() + | Skip.t() + | Limit.t() @type t :: %__MODULE__{clauses: [clause()], params: map()} @@ -273,7 +296,16 @@ defmodule AshNeo4j.Cypher.Query do `path_segments` is a list of `{edge_label, direction, dest_label}` tuples describing the traversal from source to the node being aggregated. """ - @spec aggregate_per_record(atom(), atom(), [any()], [{atom(), atom(), atom()}], atom(), atom() | nil, atom(), boolean()) :: t() + @spec aggregate_per_record( + atom(), + atom(), + [any()], + [{atom(), atom(), atom()}], + atom(), + atom() | nil, + atom(), + boolean() + ) :: t() def aggregate_per_record(source_label, pk_field, ids, path_segments, kind, field, name, uniq? \\ false) when is_atom(source_label) and is_atom(pk_field) and is_list(ids) and is_list(path_segments) and is_atom(kind) do path = build_agg_path(path_segments) @@ -295,7 +327,8 @@ defmodule AshNeo4j.Cypher.Query do `MATCH (s:Label) WHERE s.pk IN $agg_ids OPTIONAL MATCH (s)(d) RETURN agg_fn AS name` """ - @spec aggregate_total(atom(), atom(), [any()], [{atom(), atom(), atom()}], atom(), atom() | nil, atom(), boolean()) :: t() + @spec aggregate_total(atom(), atom(), [any()], [{atom(), atom(), atom()}], atom(), atom() | nil, atom(), boolean()) :: + t() def aggregate_total(source_label, pk_field, ids, path_segments, kind, field, name, uniq? \\ false) when is_atom(source_label) and is_atom(pk_field) and is_list(ids) and is_list(path_segments) and is_atom(kind) do path = build_agg_path(path_segments) @@ -375,7 +408,11 @@ defmodule AshNeo4j.Cypher.Query do def delete_nodes_guarded(label, properties, guards) when is_atom(label) and is_map(properties) and is_list(guards) do {pattern, params} = Cypher.parameterized_node(:n, [label], properties) - conditions = Enum.map(guards, fn {edge_label, direction, dest_label} -> guard_condition(:n, edge_label, direction, dest_label) end) + + conditions = + Enum.map(guards, fn {edge_label, direction, dest_label} -> + guard_condition(:n, edge_label, direction, dest_label) + end) %__MODULE__{ clauses: [%Match{pattern: pattern}, %Where{conditions: conditions}, %DetachDelete{items: ["n"]}], @@ -421,7 +458,9 @@ defmodule AshNeo4j.Cypher.Query do clauses: [ %Match{pattern: src_pattern}, %With{items: ["s"]}, - %OptionalMatch{pattern: "(s)" <> Cypher.relationship(:r0, edge_label, direction) <> Cypher.node(:d0, [dest_label])}, + %OptionalMatch{ + pattern: "(s)" <> Cypher.relationship(:r0, edge_label, direction) <> Cypher.node(:d0, [dest_label]) + }, %Delete{items: ["r0"]}, %With{items: ["s"]}, %Match{pattern: dest_pattern}, @@ -450,7 +489,9 @@ defmodule AshNeo4j.Cypher.Query do %Match{pattern: src_pattern}, %OptionalMatch{pattern: dest_pattern}, %With{items: ["s", "d"]}, - %OptionalMatch{pattern: Cypher.node(:s0, [src_label]) <> Cypher.relationship(:r0, edge_label, direction) <> "(d)"}, + %OptionalMatch{ + pattern: Cypher.node(:s0, [src_label]) <> Cypher.relationship(:r0, edge_label, direction) <> "(d)" + }, %Where{conditions: ["s0 <> s"]}, %Delete{items: ["r0"]}, %With{items: ["s", "d"]}, @@ -485,7 +526,9 @@ defmodule AshNeo4j.Cypher.Query do %With{items: ["s"]}, %OptionalMatch{pattern: dest_pattern}, %With{items: ["s", "d"]}, - %OptionalMatch{pattern: Cypher.node(:s0, [src_label]) <> Cypher.relationship(:r0, edge_label, direction) <> "(d)"}, + %OptionalMatch{ + pattern: Cypher.node(:s0, [src_label]) <> Cypher.relationship(:r0, edge_label, direction) <> "(d)" + }, %Where{conditions: ["s0 <> s"]}, %Delete{items: ["r0"]}, %With{items: ["s", "d"]}, @@ -566,11 +609,14 @@ defmodule AshNeo4j.Cypher.Query do |> Enum.with_index() |> Enum.reduce("", fn {{edge_label, direction, dest_label}, i}, acc -> node_var = if i == last_idx, do: "d", else: "h#{i}" - rel = case direction do - :outgoing -> "-[:#{edge_label}]->" - :incoming -> "<-[:#{edge_label}]-" - _ -> "-[:#{edge_label}]-" - end + + rel = + case direction do + :outgoing -> "-[:#{edge_label}]->" + :incoming -> "<-[:#{edge_label}]-" + _ -> "-[:#{edge_label}]-" + end + acc <> rel <> "(#{node_var}:#{dest_label})" end) end @@ -579,17 +625,18 @@ defmodule AshNeo4j.Cypher.Query do distinct = if uniq?, do: "DISTINCT ", else: "" field_ref = if field, do: "d.#{field}", else: "d" - fn_str = case kind do - :count -> "COUNT(#{distinct}d)" - :exists -> "COUNT(d) > 0" - :sum -> "sum(#{distinct}#{field_ref})" - :avg -> "avg(#{distinct}#{field_ref})" - :min -> "min(#{field_ref})" - :max -> "max(#{field_ref})" - :list -> "collect(#{distinct}#{field_ref})" - :first -> "head(collect(#{field_ref}))" - end - - "#{fn_str} AS #{name}" + fn_str = + case kind do + :count -> "COUNT(#{distinct}d)" + :exists -> "COUNT(d) > 0" + :sum -> "sum(#{distinct}#{field_ref})" + :avg -> "avg(#{distinct}#{field_ref})" + :min -> "min(#{field_ref})" + :max -> "max(#{field_ref})" + :list -> "collect(#{distinct}#{field_ref})" + :first -> "head(collect(#{field_ref}))" + end + + "#{fn_str} AS `#{name}`" end end diff --git a/lib/data_layer.ex b/lib/data_layer.ex index ec75f29..04dde5a 100644 --- a/lib/data_layer.ex +++ b/lib/data_layer.ex @@ -171,7 +171,12 @@ defmodule AshNeo4j.DataLayer do case Enum.find(all_results, &match?({:error, _}, &1)) do nil -> - records = Enum.map(all_results, fn {:ok, r} -> r; r -> r end) + records = + Enum.map(all_results, fn + {:ok, r} -> r + r -> r + end) + aggregates = Map.values(Map.get(query, :aggregates) || %{}) calculations = Map.values(Map.get(query, :calculations) || %{}) @@ -1012,11 +1017,13 @@ defmodule AshNeo4j.DataLayer do Enum.reduce_while(aggregates, {:ok, records}, fn aggregate, {:ok, acc_records} -> case run_aggregate_for_ids(mapping, neo4j_pk, ids, aggregate, :per_record) do {:ok, agg_map} -> - updated = Enum.map(acc_records, fn record -> - id = Map.get(record, pk_field) - value = Map.get(agg_map, id, aggregate.default_value) - Map.put(record, aggregate.name, value) - end) + updated = + Enum.map(acc_records, fn record -> + id = Map.get(record, pk_field) + value = Map.get(agg_map, id, aggregate.default_value) + Map.put(record, aggregate.name, value) + end) + {:cont, {:ok, updated}} {:error, e} -> @@ -1072,26 +1079,57 @@ defmodule AshNeo4j.DataLayer do embedded = embedded_field_type(dest_mapping.module, aggregate.field) cond do + # Expression-based aggregates always load full records in Elixir; + # run_expr_agg handles aggregate.filter internally via apply_record_filter. is_struct(aggregate.field, Ash.Query.Calculation) -> run_expr_agg(mapping, neo4j_pk, ids, aggregate, mode, path_segments, dest_mapping) + # When a filter is present on a plain or embedded aggregate, load full + # destination records in Elixir so Ash.Filter.Runtime can evaluate it. + # Honouring the filter is a contract implied by can?({:aggregate, kind}). + aggregate_has_filter?(aggregate) -> + run_filtered_aggregate(mapping, neo4j_pk, ids, aggregate, mode, path_segments, dest_mapping) + embedded -> {field_type, field_constraints} = embedded - run_embedded_agg(mapping, neo4j_pk, ids, aggregate, mode, path_segments, neo4j_field, field_type, field_constraints) + + run_embedded_agg( + mapping, + neo4j_pk, + ids, + aggregate, + mode, + path_segments, + neo4j_field, + field_type, + field_constraints + ) true -> query = case mode do :per_record -> CypherQuery.aggregate_per_record( - mapping.label, neo4j_pk, ids, path_segments, - aggregate.kind, neo4j_field, aggregate.name, aggregate.uniq? + mapping.label, + neo4j_pk, + ids, + path_segments, + aggregate.kind, + neo4j_field, + aggregate.name, + aggregate.uniq? ) :total -> CypherQuery.aggregate_total( - mapping.label, neo4j_pk, ids, path_segments, - aggregate.kind, neo4j_field, aggregate.name, aggregate.uniq? + mapping.label, + neo4j_pk, + ids, + path_segments, + aggregate.kind, + neo4j_field, + aggregate.name, + aggregate.uniq? ) end @@ -1099,9 +1137,10 @@ defmodule AshNeo4j.DataLayer do {:ok, %Bolty.Response{results: rows}} -> case mode do :per_record -> - {:ok, Map.new(rows, fn row -> - {Map.get(row, "source_id"), Map.get(row, to_string(aggregate.name))} - end)} + {:ok, + Map.new(rows, fn row -> + {Map.get(row, "source_id"), Map.get(row, to_string(aggregate.name))} + end)} :total -> value = rows |> List.first(%{}) |> Map.get(to_string(aggregate.name), aggregate.default_value) @@ -1118,32 +1157,121 @@ defmodule AshNeo4j.DataLayer do end end + # Handles any aggregate that carries a filter expression. Loads all destination + # records for the given source IDs via Elixir, applies the Ash runtime filter, + # then computes the aggregate in Elixir. + # + # This path is also used for expression-based aggregates (Ash.Query.Calculation + # field) when a filter is present, because we already load full records there. + defp run_filtered_aggregate(mapping, neo4j_pk, ids, aggregate, mode, path_segments, dest_mapping) do + query = CypherQuery.related_nodes(mapping.label, neo4j_pk, ids, path_segments) + dest_resource = dest_mapping.module + domain = Ash.Resource.Info.domain(dest_resource) + + with {:ok, %Bolty.Response{results: rows}} <- Cypher.run(query) do + pairs = + Enum.flat_map(rows, fn row -> + source_id = Map.get(row, "source_id") + dest_node = Map.get(row, "dest_node") + + if dest_node do + case convert_node_to_resource(dest_resource, dest_node) do + {:ok, record} -> [{source_id, record}] + _ -> [] + end + else + [] + end + end) + + case mode do + :per_record -> + grouped = Enum.group_by(pairs, &elem(&1, 0), &elem(&1, 1)) + + result = + Map.new(grouped, fn {source_id, records} -> + {:ok, filtered} = Ash.Filter.Runtime.filter_matches(domain, records, aggregate.query.filter) + values = extract_aggregate_field_values(filtered, aggregate) + {source_id, apply_elixir_aggregate(aggregate.kind, values, aggregate.default_value)} + end) + + {:ok, result} + + :total -> + all_records = Enum.map(pairs, &elem(&1, 1)) + {:ok, filtered} = Ash.Filter.Runtime.filter_matches(domain, all_records, aggregate.query.filter) + values = extract_aggregate_field_values(filtered, aggregate) + {:ok, apply_elixir_aggregate(aggregate.kind, values, aggregate.default_value)} + end + end + end + + # Extracts the aggregate's target field value from each record, respecting uniq?. + defp extract_aggregate_field_values(records, aggregate) do + values = + Enum.map(records, fn record -> + case aggregate.field do + nil -> record + field when is_atom(field) -> Map.get(record, field) + _ -> record + end + end) + + if aggregate.uniq?, do: Enum.uniq(values), else: values + end + defp embedded_field_type(resource_module, field_name) when is_atom(field_name) do case Ash.Resource.Info.attribute(resource_module, field_name) do - nil -> nil + nil -> + nil + attr -> type = Ash.Type.get_type(attr.type) + case TypeClassifier.classify(type) do {:ok, :ash_json, _} -> {type, attr.constraints} _ -> nil end end end + defp embedded_field_type(_, _), do: nil - defp run_embedded_agg(mapping, neo4j_pk, ids, aggregate, mode, path_segments, neo4j_field, field_type, field_constraints) do + defp run_embedded_agg( + mapping, + neo4j_pk, + ids, + aggregate, + mode, + path_segments, + neo4j_field, + field_type, + field_constraints + ) do query = case mode do :per_record -> CypherQuery.aggregate_per_record( - mapping.label, neo4j_pk, ids, path_segments, - :list, neo4j_field, aggregate.name, aggregate.uniq? + mapping.label, + neo4j_pk, + ids, + path_segments, + :list, + neo4j_field, + aggregate.name, + aggregate.uniq? ) :total -> CypherQuery.aggregate_total( - mapping.label, neo4j_pk, ids, path_segments, - :list, neo4j_field, aggregate.name, aggregate.uniq? + mapping.label, + neo4j_pk, + ids, + path_segments, + :list, + neo4j_field, + aggregate.name, + aggregate.uniq? ) end @@ -1153,12 +1281,13 @@ defmodule AshNeo4j.DataLayer do case mode do :per_record -> - {:ok, Map.new(rows, fn row -> - source_id = Map.get(row, "source_id") - raw_list = Map.get(row, agg_key, []) - cast_list = cast_raw_list(raw_list, field_type, field_constraints) - {source_id, apply_elixir_aggregate(aggregate.kind, cast_list, aggregate.default_value)} - end)} + {:ok, + Map.new(rows, fn row -> + source_id = Map.get(row, "source_id") + raw_list = Map.get(row, agg_key, []) + cast_list = cast_raw_list(raw_list, field_type, field_constraints) + {source_id, apply_elixir_aggregate(aggregate.kind, cast_list, aggregate.default_value)} + end)} :total -> raw_list = rows |> List.first(%{}) |> Map.get(agg_key, []) @@ -1174,6 +1303,7 @@ defmodule AshNeo4j.DataLayer do defp run_expr_agg(mapping, neo4j_pk, ids, aggregate, mode, path_segments, dest_mapping) do query = CypherQuery.related_nodes(mapping.label, neo4j_pk, ids, path_segments) dest_resource = dest_mapping.module + domain = Ash.Resource.Info.domain(dest_resource) calc = aggregate.field expr = calc.opts[:expr] @@ -1181,18 +1311,14 @@ defmodule AshNeo4j.DataLayer do {:ok, hydrated} -> case Cypher.run(query) do {:ok, %Bolty.Response{results: rows}} -> - pairs = + record_pairs = Enum.flat_map(rows, fn row -> source_id = Map.get(row, "source_id") dest_node = Map.get(row, "dest_node") if dest_node do case convert_node_to_resource(dest_resource, dest_node) do - {:ok, record} -> - case Ash.Expr.eval_hydrated(hydrated, record: record, resource: dest_resource, unknown_on_unknown_refs?: true) do - {:ok, value} when not is_nil(value) -> [{source_id, value}] - _ -> [] - end + {:ok, record} -> [{source_id, record}] _ -> [] end else @@ -1200,31 +1326,84 @@ defmodule AshNeo4j.DataLayer do end end) + # Apply aggregate filter if present, then evaluate the expression. + pairs = + apply_record_filter(record_pairs, aggregate_query_filter(aggregate), domain) + |> Enum.flat_map(fn {source_id, record} -> + case Ash.Expr.eval_hydrated(hydrated, + record: record, + resource: dest_resource, + unknown_on_unknown_refs?: true + ) do + {:ok, value} when not is_nil(value) -> [{source_id, value}] + _ -> [] + end + end) + case mode do :per_record -> grouped = Enum.group_by(pairs, &elem(&1, 0), &elem(&1, 1)) - {:ok, Map.new(grouped, fn {source_id, values} -> - {source_id, apply_elixir_aggregate(aggregate.kind, values, aggregate.default_value)} - end)} + + {:ok, + Map.new(grouped, fn {source_id, values} -> + {source_id, apply_elixir_aggregate(aggregate.kind, values, aggregate.default_value)} + end)} :total -> values = Enum.map(pairs, &elem(&1, 1)) {:ok, apply_elixir_aggregate(aggregate.kind, values, aggregate.default_value)} end - {:error, e} -> {:error, e} + {:error, e} -> + {:error, e} end - {:error, e} -> {:error, e} + {:error, e} -> + {:error, e} + end + end + + # Returns true when the aggregate carries a real (non-trivial) filter in its + # query. Ash always provides an Ash.Query on the aggregate; unfiltered aggregates + # have %Ash.Filter{expression: true}. We only route through the Elixir-side + # path when there is an actual user-defined filter to honour. + defp aggregate_has_filter?(aggregate) do + case aggregate_query_filter(aggregate) do + %Ash.Filter{expression: true} -> false + %Ash.Filter{} -> true + _ -> false + end + end + + # Extracts the filter from aggregate.query, returning nil if absent. + defp aggregate_query_filter(aggregate) do + case Map.get(aggregate, :query) do + %Ash.Query{filter: filter} -> filter + _ -> nil end end + # Applies an Ash filter (if any) to a list of {source_id, record} pairs, + # keeping per-source grouping so filter predicates referencing destination + # attributes are evaluated correctly. + defp apply_record_filter(pairs, nil, _domain), do: pairs + + defp apply_record_filter(pairs, filter, domain) do + grouped = Enum.group_by(pairs, &elem(&1, 0), &elem(&1, 1)) + + Enum.flat_map(grouped, fn {source_id, records} -> + {:ok, filtered} = Ash.Filter.Runtime.filter_matches(domain, records, filter) + Enum.map(filtered, &{source_id, &1}) + end) + end + defp cast_raw_list(raw_list, field_type, field_constraints) when is_list(raw_list) do case Cast.cast({:array, field_type}, raw_list, field_constraints) do {:ok, values} -> values {:error, _} -> [] end end + defp cast_raw_list(_, _, _), do: [] defp apply_elixir_aggregate(:list, values, _default), do: values diff --git a/lib/mix/tasks/ash_neo4j.install.ex b/lib/mix/tasks/ash_neo4j.install.ex index a50a11f..e2e15bd 100644 --- a/lib/mix/tasks/ash_neo4j.install.ex +++ b/lib/mix/tasks/ash_neo4j.install.ex @@ -52,7 +52,8 @@ if Code.ensure_loaded?(Igniter) do "runtime.exs", :bolty, [Bolt, :auth], - [username: "neo4j", password: "password"] + username: "neo4j", + password: "password" ) |> Igniter.Project.Config.configure( "runtime.exs", @@ -66,9 +67,7 @@ if Code.ensure_loaded?(Igniter) do [Bolt, :name], Bolt ) - |> Igniter.Project.Application.add_new_child( - {Bolty, {:code, quote(do: Application.get_env(:bolty, Bolt))}} - ) + |> Igniter.Project.Application.add_new_child({Bolty, {:code, quote(do: Application.get_env(:bolty, Bolt))}}) end end else diff --git a/lib/neo4j_helper.ex b/lib/neo4j_helper.ex index 762101d..f153e7a 100644 --- a/lib/neo4j_helper.ex +++ b/lib/neo4j_helper.ex @@ -186,10 +186,24 @@ defmodule AshNeo4j.Neo4jHelper do :ok ``` """ - def relate_nodes_unrelating_source(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + def relate_nodes_unrelating_source( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) when is_atom(source_label) and is_map(source_properties) and is_atom(dest_label) and is_map(dest_properties) and is_atom(edge_label) and is_atom(edge_direction) do - Query.relate_unrelating_source(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + Query.relate_unrelating_source( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) |> Cypher.run() end @@ -209,10 +223,24 @@ defmodule AshNeo4j.Neo4jHelper do :ok ``` """ - def relate_nodes_unrelating_destination(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + def relate_nodes_unrelating_destination( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) when is_atom(source_label) and is_map(source_properties) and is_atom(dest_label) and is_map(dest_properties) and is_atom(edge_label) and is_atom(edge_direction) do - Query.relate_unrelating_destination(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + Query.relate_unrelating_destination( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) |> Cypher.run() end @@ -234,10 +262,24 @@ defmodule AshNeo4j.Neo4jHelper do :ok ``` """ - def relate_nodes_unrelating_source_and_destination(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + def relate_nodes_unrelating_source_and_destination( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) when is_atom(source_label) and is_map(source_properties) and is_atom(dest_label) and is_map(dest_properties) and is_atom(edge_label) and is_atom(edge_direction) do - Query.relate_unrelating_both(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + Query.relate_unrelating_both( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) |> Cypher.run() end @@ -250,13 +292,34 @@ defmodule AshNeo4j.Neo4jHelper do relate_nodes(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) {true, false} -> - relate_nodes_unrelating_source(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + relate_nodes_unrelating_source( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) {false, true} -> - relate_nodes_unrelating_destination(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + relate_nodes_unrelating_destination( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) {true, true} -> - relate_nodes_unrelating_source_and_destination(source_label, source_properties, dest_label, dest_properties, edge_label, edge_direction) + relate_nodes_unrelating_source_and_destination( + source_label, + source_properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) end end @@ -281,9 +344,17 @@ defmodule AshNeo4j.Neo4jHelper do def relate_nodes(label, properties, relationships) when is_atom(label) and is_map(properties) and is_list(relationships) do results = - Enum.reduce_while(relationships, [], fn {dest_label, dest_properties, edge_label, edge_direction, exclusive}, acc -> + Enum.reduce_while(relationships, [], fn {dest_label, dest_properties, edge_label, edge_direction, exclusive}, + acc -> if exclusive do - case relate_nodes_unrelating_destination(label, properties, dest_label, dest_properties, edge_label, edge_direction) do + case relate_nodes_unrelating_destination( + label, + properties, + dest_label, + dest_properties, + edge_label, + edge_direction + ) do {:ok, result} -> {:cont, [result | acc]} {:error, _} -> {:halt, :error} end @@ -324,7 +395,9 @@ defmodule AshNeo4j.Neo4jHelper do cypher = "MATCH #{src_pattern}#{Cypher.relationship(:r, edge_label, edge_direction)}#{dest_pattern} RETURN s, r, d" case Cypher.run(cypher, Map.merge(src_params, dest_params)) do - {:ok, %{records: records}} -> length(records) > 0 + {:ok, %{records: records}} -> + length(records) > 0 + {:error, error} -> Logger.error("AshNeo4j.Neo4jHelper.Error running query: #{inspect(error)}") :error @@ -355,14 +428,18 @@ defmodule AshNeo4j.Neo4jHelper do path = Enum.reduce(edges, "", fn {edge_label, edge_direction}, acc -> variable = String.to_atom("r#{String.length(acc)}") - if acc == "", do: Cypher.relationship(variable, edge_label, edge_direction), - else: acc <> "()" <> Cypher.relationship(variable, edge_label, edge_direction) + + if acc == "", + do: Cypher.relationship(variable, edge_label, edge_direction), + else: acc <> "()" <> Cypher.relationship(variable, edge_label, edge_direction) end) cypher = "MATCH #{src_pattern}#{path}#{dest_pattern} RETURN s, d" case Cypher.run(cypher, Map.merge(src_params, dest_params)) do - {:ok, %{records: records}} -> length(records) > 0 + {:ok, %{records: records}} -> + length(records) > 0 + {:error, error} -> Logger.error("AshNeo4j.Neo4jHelper.Error running query: #{inspect(error)}") :error diff --git a/lib/persisters/persist_mapping.ex b/lib/persisters/persist_mapping.ex index 8dcf287..148edf3 100644 --- a/lib/persisters/persist_mapping.ex +++ b/lib/persisters/persist_mapping.ex @@ -43,9 +43,13 @@ defmodule AshNeo4j.Persisters.PersistMapping do } {:ok, - Transformer.eval(dsl, [], quote do - @doc false - def __ash_neo4j_mapping__, do: unquote(Macro.escape(mapping)) - end)} + Transformer.eval( + dsl, + [], + quote do + @doc false + def __ash_neo4j_mapping__, do: unquote(Macro.escape(mapping)) + end + )} end end diff --git a/lib/query_helper.ex b/lib/query_helper.ex index 801eaba..f1e5934 100644 --- a/lib/query_helper.ex +++ b/lib/query_helper.ex @@ -160,5 +160,4 @@ defmodule AshNeo4j.QueryHelper do ResourceInfo.attribute_type(mapping.module, predicate_left) in [Ash.Type.CiString, :ci_string] or match?(%Ash.CiString{}, predicate_right) end - end diff --git a/lib/resource/info.ex b/lib/resource/info.ex index 28f2fe7..670664f 100644 --- a/lib/resource/info.ex +++ b/lib/resource/info.ex @@ -45,7 +45,7 @@ defmodule AshNeo4j.Resource.Info do @spec labels(Ash.Resource.t()) :: list(atom()) | nil def labels(resource) do Extension.get_persisted(resource, :labels, nil) || - ([domain_label(resource), label(resource)] |> Enum.uniq() |> Enum.filter(& &1)) + [domain_label(resource), label(resource)] |> Enum.uniq() |> Enum.filter(& &1) end @doc """ diff --git a/mix.exs b/mix.exs index 5d86280..ff137d6 100644 --- a/mix.exs +++ b/mix.exs @@ -6,7 +6,7 @@ defmodule AshNeo4j.MixProject do @moduledoc false use Mix.Project - @version "0.5.0" + @version "0.5.1" @name "AshNeo4j" @description "Ash DataLayer for Neo4j" @github_url "https://github.com/diffo-dev/ash_neo4j" @@ -30,23 +30,10 @@ defmodule AshNeo4j.MixProject do ], consolidate_protocols: Mix.env() == :prod, aliases: aliases(), - # ex_doc name: @name, source_url: @github_url, homepage_url: "https://diffo.dev/diffo/ash_neo4j", - docs: [main: "readme", extras: ["README.md"]], - # hex.pm stuff - description: @description, - package: [ - name: "ash_neo4j", - licenses: ["MIT"], - files: ["lib", "mix.exs", "README*", "VERSION*"], - maintainers: ["Matt Beanland"], - links: %{ - "GitHub" => @github_url, - "Author's home page" => "https://www.diffo.dev" - } - ] + description: @description ] end @@ -68,25 +55,60 @@ defmodule AshNeo4j.MixProject do source_url: @github_url, source_ref: "v#{@version}", main: "readme", - logo: "logos/diffo.jpg", extras: [ - "README.md": [title: "Guide"], - "LICENSES/MIT.md": [title: "License"], - "documentation/dsls/DSL-AshNeo4j.DataLayer.md": [ - title: "DSL: AshNeo4j.DataLayer", - search_data: Spark.Docs.search_data_for(AshNeo4j.DataLayer) + {"README.md", title: "Home"}, + {"LICENSES/MIT.md", title: "License"}, + {"ash_neo4j_datalayer.livemd", title: "AshNeo4j Livebook"}, + {"documentation/dsls/DSL-AshNeo4j.DataLayer.md", search_data: Spark.Docs.search_data_for(AshNeo4j.DataLayer)}, + "CHANGELOG.md" + ], + groups_for_extras: [ + Tutorials: ~r'documentation/tutorials', + "How To": [~r'documentation/how_to', "ash_neo4j_datalayer.livemd"], + Topics: ~r'documentation/topics', + DSLs: ~r'documentation/dsls', + "About AshNeo4j": [ + "CHANGELOG.md" ] + ], + logo: "logos/diffo.jpg", + groups_for_modules: [ + AshNeo4j: [ + AshNeo4j, + AshNeo4j.DataLayer, + AshNeo4j.Sandbox + ], + Introspection: [ + AshNeo4j.DataLayer.Info, + AshNeo4j.Resource.Info, + AshNeo4j.EdgeDescriptor, + AshNeo4j.ResourceMapping + ], + Cypher: ~r/AshNeo4j\.Cypher/, + Utilities: [ + AshNeo4j.BoltyHelper, + AshNeo4j.Neo4jHelper, + AshNeo4j.QueryHelper, + AshNeo4j.Util + ], + Internals: ~r/.*/ ] ] end defp package do [ - name: :ash_neo4j, + maintainers: [ + "Matt Beanland " + ], licenses: ["MIT"], - files: ~w(lib .formatter.exs mix.exs README* LICENSE* documentation usage-rules.md usage-rules), + files: + ~w(lib .formatter.exs mix.exs README* LICENSE* CHANGELOG* documentation usage-rules.md usage-rules ash_neo4j_datalayer.livemd), links: %{ - GitHub: "https://github.com/diffo-dev/ash_neo4j" + "GitHub" => "https://github.com/diffo-dev/ash_neo4j", + "Changelog" => "https://github.com/diffo-dev/ash_neo4j/blob/main/CHANGELOG.md", + "Website" => "https://diffo.dev", + "REUSE Compliance" => "https://api.reuse.software/info/github.com/diffo-dev/ash_neo4j" } ] end diff --git a/test/aggregate_test.exs b/test/aggregate_test.exs index f39f81b..5958d00 100644 --- a/test/aggregate_test.exs +++ b/test/aggregate_test.exs @@ -93,7 +93,7 @@ defmodule AshNeo4j.AggregateTest do posts = Post |> Ash.read!() |> Ash.load!([:has_comments]) |> Enum.sort_by(& &1.title) without = Enum.find(posts, &(&1.title == "without comments")) - with_c = Enum.find(posts, &(&1.title == "with comments")) + with_c = Enum.find(posts, &(&1.title == "with comments")) assert with_c.has_comments == true assert without.has_comments == false @@ -159,12 +159,113 @@ defmodule AshNeo4j.AggregateTest do create_comment_with_dog(post, "b", %DogTypedStruct{name: "Spot", age: 7}) {:ok, %{total_dog_age: total}} = - Ash.aggregate(Post, {:total_dog_age, :sum, [path: [:comments], expr: Ash.Expr.expr(get_path(dog, [:age])), expr_type: :integer]}) + Ash.aggregate( + Post, + {:total_dog_age, :sum, [path: [:comments], expr: Ash.Expr.expr(get_path(dog, [:age])), expr_type: :integer]} + ) assert total == 10 end end + describe "aggregate names with ? suffix (#251 — must not produce invalid Cypher)" do + test "exists aggregate named with ? suffix returns correct boolean" do + author = create_author() + post_with = create_post(author, "with comments") + _post_without = create_post(author, "without comments") + create_comment(post_with, "a comment") + + [with_c, without_c] = Post |> Ash.read!() |> Ash.load!([:has_comments?]) |> Enum.sort_by(& &1.title) + + assert with_c.has_comments? == true + assert without_c.has_comments? == false + end + end + + describe "filtered aggregates (#252 — filter must not be silently dropped)" do + test "first aggregate with filter returns the matching record's field, not whichever comes first" do + author = create_author() + post = create_post(author, "post") + # Create beta first so Neo4j is likely to return it first without a filter + create_comment(post, "beta") + create_comment(post, "alpha") + + [loaded] = Post |> Ash.read!() |> Ash.load!([:first_alpha_comment_title]) + assert loaded.first_alpha_comment_title == "alpha" + end + + test "count aggregate with filter counts only matching records" do + author = create_author() + post1 = create_post(author, "post1") + post2 = create_post(author, "post2") + create_comment(post1, "alpha") + create_comment(post1, "beta") + create_comment(post1, "alpha") + create_comment(post2, "beta") + + [p1, p2] = Post |> Ash.read!() |> Ash.load!([:alpha_comment_count]) |> Enum.sort_by(& &1.title) + assert p1.alpha_comment_count == 2 + assert p2.alpha_comment_count == 0 + end + + test "exists aggregate with filter is false when only non-matching records exist" do + author = create_author() + post = create_post(author, "post") + create_comment(post, "beta") + + [loaded] = Post |> Ash.read!() |> Ash.load!([:has_alpha_comment]) + assert loaded.has_alpha_comment == false + end + + test "exists aggregate with filter is true when a matching record exists" do + author = create_author() + post = create_post(author, "post") + create_comment(post, "beta") + create_comment(post, "alpha") + + [loaded] = Post |> Ash.read!() |> Ash.load!([:has_alpha_comment]) + assert loaded.has_alpha_comment == true + end + + test "list aggregate with filter returns only matching field values" do + author = create_author() + post = create_post(author, "post") + create_comment(post, "alpha") + create_comment(post, "beta") + create_comment(post, "alpha") + + [loaded] = Post |> Ash.read!() |> Ash.load!([:alpha_comment_titles]) + assert Enum.sort(loaded.alpha_comment_titles) == ["alpha", "alpha"] + end + + test "count with filter returns 0 for post with no comments" do + author = create_author() + create_post(author, "empty post") + + [loaded] = Post |> Ash.read!() |> Ash.load!([:alpha_comment_count]) + assert loaded.alpha_comment_count == 0 + end + + test "multiple posts each see only their own filtered count" do + author = create_author() + post1 = create_post(author, "aaa") + post2 = create_post(author, "bbb") + create_comment(post1, "alpha") + create_comment(post1, "alpha") + create_comment(post1, "beta") + create_comment(post2, "beta") + create_comment(post2, "beta") + + [p1, p2] = + Post |> Ash.read!() |> Ash.load!([:alpha_comment_count, :has_alpha_comment]) |> Enum.sort_by(& &1.title) + + assert p1.alpha_comment_count == 2 + assert p1.has_alpha_comment == true + assert p2.alpha_comment_count == 0 + assert p2.has_alpha_comment == false + end + end + describe "aggregates on embedded struct fields" do test "list aggregate returns deserialized typed structs" do author = create_author() diff --git a/test/blog_test.exs b/test/blog_test.exs index d263c91..d454983 100644 --- a/test/blog_test.exs +++ b/test/blog_test.exs @@ -692,7 +692,14 @@ defmodule AshNeo4j.BlogTest do uuid: post_uuid = Ash.UUID.generate() }) - Neo4jHelper.relate_nodes([:SRM, :Author], %{uuid: author_uuid}, [:SRM, :Post], %{uuid: post_uuid}, :WROTE, :outgoing) + Neo4jHelper.relate_nodes( + [:SRM, :Author], + %{uuid: author_uuid}, + [:SRM, :Post], + %{uuid: post_uuid}, + :WROTE, + :outgoing + ) end end @@ -707,7 +714,15 @@ defmodule AshNeo4j.BlogTest do for post <- 1..posts do Neo4jHelper.create_node([:SRM, :Post], %{title: "post#{post}", uuid: post_uuid = Ash.UUID.generate()}) - Neo4jHelper.relate_nodes([:SRM, :Author], %{uuid: author_uuid}, [:SRM, :Post], %{uuid: post_uuid}, :WROTE, :outgoing) + + Neo4jHelper.relate_nodes( + [:SRM, :Author], + %{uuid: author_uuid}, + [:SRM, :Post], + %{uuid: post_uuid}, + :WROTE, + :outgoing + ) for comment <- 1..comments do Neo4jHelper.create_node([:SRM, :Comment], %{ @@ -715,7 +730,14 @@ defmodule AshNeo4j.BlogTest do uuid: comment_uuid = Ash.UUID.generate() }) - Neo4jHelper.relate_nodes([:SRM, :Comment], %{uuid: comment_uuid}, [:SRM, :Post], %{uuid: post_uuid}, :BELONGS_TO, :outgoing) + Neo4jHelper.relate_nodes( + [:SRM, :Comment], + %{uuid: comment_uuid}, + [:SRM, :Post], + %{uuid: post_uuid}, + :BELONGS_TO, + :outgoing + ) end end end diff --git a/test/calculation_test.exs b/test/calculation_test.exs index aa0b2cb..caabd9f 100644 --- a/test/calculation_test.exs +++ b/test/calculation_test.exs @@ -23,7 +23,10 @@ defmodule AshNeo4j.CalculationTest do end defp create_author, do: Author |> Ash.Changeset.for_create(:create, %{name: "Author"}) |> Ash.create!() - defp create_post(author), do: Post |> Ash.Changeset.for_create(:create, %{title: "post", written_by: author.id}) |> Ash.create!() + + defp create_post(author), + do: Post |> Ash.Changeset.for_create(:create, %{title: "post", written_by: author.id}) |> Ash.create!() + defp create_comment(post, title, score) do Comment |> Ash.Changeset.for_create(:create, %{title: title, score: score, post_id: post.id}) |> Ash.create!() end diff --git a/test/support/resource/post.ex b/test/support/resource/post.ex index a8aa85c..349b78d 100644 --- a/test/support/resource/post.ex +++ b/test/support/resource/post.ex @@ -79,10 +79,28 @@ defmodule AshNeo4j.Test.Resource.Post do aggregates do count :comment_count, :comments exists :has_comments, :comments + exists :has_comments?, :comments first :first_comment_title, :comments, field: :title list :comment_titles, :comments, field: :title list :comment_dogs, :comments, field: :dog first :first_comment_dog, :comments, field: :dog + + # Filtered aggregates — used to verify #252 (filters must not be silently dropped). + first :first_alpha_comment_title, :comments, :title do + filter expr(title == "alpha") + end + + count :alpha_comment_count, :comments do + filter expr(title == "alpha") + end + + exists :has_alpha_comment, :comments do + filter expr(title == "alpha") + end + + list :alpha_comment_titles, :comments, :title do + filter expr(title == "alpha") + end end preparations do