From 88c27a52745988ac3002b966ba9f7063372af7aa Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Thu, 25 Apr 2024 22:57:07 -0400 Subject: [PATCH] improvement: update to bulk operations now that we can test them in ets --- .../getting-started-with-json-api.md | 5 +- lib/ash_json_api/controllers/delete.ex | 1 - .../controllers/delete_from_relationship.ex | 1 - lib/ash_json_api/controllers/helpers.ex | 153 ++++++++++++++---- lib/ash_json_api/controllers/patch.ex | 1 - .../controllers/patch_relationship.ex | 1 - .../controllers/post_to_relationship.ex | 1 - test/acceptance/patch_test.exs | 1 + 8 files changed, 129 insertions(+), 35 deletions(-) diff --git a/documentation/tutorials/getting-started-with-json-api.md b/documentation/tutorials/getting-started-with-json-api.md index 58a15aef..6cfd57ec 100644 --- a/documentation/tutorials/getting-started-with-json-api.md +++ b/documentation/tutorials/getting-started-with-json-api.md @@ -1,7 +1,6 @@ # Getting started with JSON:API -The easiest set up involves using Phoenix, but it should be roughly the same to set up an -application using only Plug. We are showing examples using Phoenix Routers. +The easiest set up involves using Phoenix, but it should be roughly the same to set up an application using only Plug. We are showing examples using Phoenix Routers. The resulting JSON APIs follow the specifications from https://jsonapi.org/. @@ -110,7 +109,7 @@ end > ### Whats up with `Module.concat/1`? {: .info} > -> This `Module.concat/1` prevents a [compile-time dependency](https://dashbit.co/blog/speeding-up-re-compilation-of-elixir-projects) from this router module to the domain modules. It is an implementation detail of how `forward/2` works that you end up with a compile-time dependency on the schema, but there is no need for this dependency, and that dependency can have *drastic* impacts on your compile times in certain scenarios. +> This `Module.concat/1` prevents a [compile-time dependency](https://dashbit.co/blog/speeding-up-re-compilation-of-elixir-projects) from this router module to the domain modules. It is an implementation detail of how `forward/2` works that you end up with a compile-time dependency on the schema, but there is no need for this dependency, and that dependency can have _drastic_ impacts on your compile times in certain scenarios. Additionally, your Resource requires a type, a base route and a set of allowed HTTP methods and what action they will trigger. diff --git a/lib/ash_json_api/controllers/delete.ex b/lib/ash_json_api/controllers/delete.ex index bf2efd01..d0f86c4c 100644 --- a/lib/ash_json_api/controllers/delete.ex +++ b/lib/ash_json_api/controllers/delete.ex @@ -16,7 +16,6 @@ defmodule AshJsonApi.Controllers.Delete do conn |> Request.from(resource, action, domain, route) - |> Helpers.fetch_record_from_path() |> Helpers.destroy_record() |> Helpers.fetch_includes() |> Helpers.render_or_render_errors(conn, fn request -> diff --git a/lib/ash_json_api/controllers/delete_from_relationship.ex b/lib/ash_json_api/controllers/delete_from_relationship.ex index b4d56828..376ce2fa 100644 --- a/lib/ash_json_api/controllers/delete_from_relationship.ex +++ b/lib/ash_json_api/controllers/delete_from_relationship.ex @@ -41,7 +41,6 @@ defmodule AshJsonApi.Controllers.DeleteFromRelationship do conn |> Request.from(options[:resource], action, domain, route) - |> Helpers.fetch_record_from_path(options[:resource]) |> Helpers.update_record(&Helpers.resource_identifiers(&1, argument)) |> Helpers.render_or_render_errors(conn, fn request -> Response.render_many_relationship(conn, request, 200, relationship) diff --git a/lib/ash_json_api/controllers/helpers.ex b/lib/ash_json_api/controllers/helpers.ex index 1f8348db..ff976860 100644 --- a/lib/ash_json_api/controllers/helpers.ex +++ b/lib/ash_json_api/controllers/helpers.ex @@ -103,22 +103,39 @@ defmodule AshJsonApi.Controllers.Helpers do end def update_record(request, attributes \\ &Map.merge(&1.attributes, &1.arguments)) do - chain(request, fn %{assigns: %{result: result}} -> - result - |> Ash.Changeset.for_update( - request.action.name, - attributes.(request), - Request.opts(request) - ) - |> Ash.Changeset.set_context(request.context) - |> Ash.Changeset.load(fields(request, request.resource) ++ (request.includes_keyword || [])) - |> Ash.update(Request.opts(request)) + chain(request, fn request -> + request + |> fetch_query() |> case do - {:ok, record} -> - Request.assign(request, :result, record) - {:error, error} -> - Request.add_error(request, error, :update) + Request.add_error(request, error, :fetch_from_path) + + {:ok, filter, query} -> + query + |> Ash.bulk_update( + request.action.name, + attributes.(request), + Request.opts(request, + return_errors?: true, + notify?: true, + strategy: [:atomic, :stream, :atomic_batches], + allow_stream_with: :full_read, + return_records?: true, + context: request.context || %{}, + load: fields(request, request.resource) ++ (request.includes_keyword || []) + ) + ) + |> case do + %Ash.BulkResult{status: :success, records: [result | _]} -> + request |> Request.assign(:result, result) |> Request.assign(:record_from_path, result) + + %Ash.BulkResult{status: :success, records: []} -> + error = Error.NotFound.exception(filter: filter, resource: request.resource) + Request.add_error(request, error, :fetch_from_path) + + %Ash.BulkResult{status: :error, errors: errors} -> + Request.add_error(request, errors, :update) + end end end) end @@ -139,7 +156,6 @@ defmodule AshJsonApi.Controllers.Helpers do |> case do {:ok, updated} -> request - |> Request.assign(:record_from_path, updated) |> Request.assign(:result, Map.get(updated, relationship_name)) {:error, error} -> @@ -164,7 +180,6 @@ defmodule AshJsonApi.Controllers.Helpers do |> case do {:ok, updated} -> request - |> Request.assign(:record_from_path, updated) |> Request.assign(:result, Map.get(updated, relationship_name)) {:error, error} -> @@ -192,7 +207,6 @@ defmodule AshJsonApi.Controllers.Helpers do |> case do {:ok, updated} -> request - |> Request.assign(:record_from_path, updated) |> Request.assign(:result, Map.get(updated, relationship_name)) {:error, error} -> @@ -206,17 +220,40 @@ defmodule AshJsonApi.Controllers.Helpers do end def destroy_record(request) do - chain(request, fn %{assigns: %{result: result}} = request -> - result - |> Ash.Changeset.for_destroy(request.action.name, %{}, Request.opts(request)) - |> Ash.Changeset.set_context(request.context) - |> Ash.destroy(Request.opts(request)) + chain(request, fn request -> + request + |> fetch_query() |> case do - :ok -> - Request.assign(request, :result, nil) - {:error, error} -> - Request.add_error(request, error, :destroy) + Request.add_error(request, error, :fetch_from_path) + + {:ok, filter, query} -> + + query + |> Ash.bulk_destroy( + request.action.name, + %{}, + Request.opts(request, + return_errors?: true, + notify?: true, + strategy: [:atomic, :stream, :atomic_batches], + allow_stream_with: :full_read, + return_records?: true, + context: request.context || %{}, + load: fields(request, request.resource) ++ (request.includes_keyword || []) + ) + ) + |> case do + %Ash.BulkResult{status: :success, records: [result | _]} -> + request |> Request.assign(:result, result) |> Request.assign(:record_from_path, result) + + %Ash.BulkResult{status: :success, records: []} -> + error = Error.NotFound.exception(filter: filter, resource: request.resource) + Request.add_error(request, error, :fetch_from_path) + + %Ash.BulkResult{status: :error, errors: errors} -> + Request.add_error(request, errors, :update) + end end end) end @@ -239,6 +276,69 @@ defmodule AshJsonApi.Controllers.Helpers do end) end + def fetch_query(%{resource: request_resource} = request, through_resource \\ nil, load \\ nil) do + resource = through_resource || request_resource + + action = + if through_resource || request.action.type != :read do + if request.route.read_action do + Ash.Resource.Info.action(request.resource, request.route.read_action) + else + Ash.Resource.Info.primary_action!(resource, :read) + end + else + request.action + end + + {params, filter} = path_args_and_filter(request.path_params, resource, action) + + action = action.name + + fields_to_load = + if through_resource do + [] + else + fields(request, request.resource) + end + + query = + if filter do + case Ash.Filter.parse_input(resource, filter) do + {:ok, parsed} -> + {:ok, Ash.Query.filter(resource, ^parsed)} + + {:error, error} -> + {:error, error} + end + else + {:ok, resource} + end + + case query do + {:error, error} -> + {:error, Request.add_error(request, error, :filter)} + + {:ok, query} -> + query = + if load do + Ash.Query.load(query, load) + else + query + end + + {:ok, filter, + query + |> Ash.Query.load(fields_to_load ++ (request.includes_keyword || [])) + |> Ash.Query.set_context(request.context) + |> Ash.Query.for_read( + action, + Map.merge(request.arguments, params), + Keyword.put(Request.opts(request), :page, false) + ) + |> Ash.Query.limit(1)} + end + end + def fetch_record_from_path(request, through_resource \\ nil, load \\ nil) do chain(request, fn %{resource: request_resource} = request -> resource = through_resource || request_resource @@ -356,7 +456,6 @@ defmodule AshJsonApi.Controllers.Helpers do |> paginator_or_list() request - |> Request.assign(:record_from_path, record) |> Request.assign(:result, paginated_result) end) end diff --git a/lib/ash_json_api/controllers/patch.ex b/lib/ash_json_api/controllers/patch.ex index da883fb0..0d146a0b 100644 --- a/lib/ash_json_api/controllers/patch.ex +++ b/lib/ash_json_api/controllers/patch.ex @@ -16,7 +16,6 @@ defmodule AshJsonApi.Controllers.Patch do conn |> Request.from(resource, action, domain, route) - |> Helpers.fetch_record_from_path() |> Helpers.update_record() |> Helpers.fetch_includes() |> Helpers.render_or_render_errors(conn, fn request -> diff --git a/lib/ash_json_api/controllers/patch_relationship.ex b/lib/ash_json_api/controllers/patch_relationship.ex index cbaf1396..83909a79 100644 --- a/lib/ash_json_api/controllers/patch_relationship.ex +++ b/lib/ash_json_api/controllers/patch_relationship.ex @@ -41,7 +41,6 @@ defmodule AshJsonApi.Controllers.PatchRelationship do conn |> Request.from(options[:resource], action, domain, route) - |> Helpers.fetch_record_from_path(options[:resource]) |> Helpers.update_record(&Helpers.resource_identifiers(&1, argument)) |> Helpers.render_or_render_errors(conn, fn request -> Response.render_many_relationship(conn, request, 200, relationship) diff --git a/lib/ash_json_api/controllers/post_to_relationship.ex b/lib/ash_json_api/controllers/post_to_relationship.ex index 7293173c..c2669c0d 100644 --- a/lib/ash_json_api/controllers/post_to_relationship.ex +++ b/lib/ash_json_api/controllers/post_to_relationship.ex @@ -41,7 +41,6 @@ defmodule AshJsonApi.Controllers.PostToRelationship do conn |> Request.from(options[:resource], action, domain, route) - |> Helpers.fetch_record_from_path(options[:resource]) |> Helpers.update_record(&Helpers.resource_identifiers(&1, argument)) |> Helpers.render_or_render_errors(conn, fn request -> Response.render_many_relationship(conn, request, 200, relationship) diff --git a/test/acceptance/patch_test.exs b/test/acceptance/patch_test.exs index cc923993..5404103d 100644 --- a/test/acceptance/patch_test.exs +++ b/test/acceptance/patch_test.exs @@ -86,6 +86,7 @@ defmodule Test.Acceptance.PatchTest do primary? true accept([:id, :email]) argument(:author, :map) + require_atomic?(false) change(manage_relationship(:author, type: :append_and_remove)) end