From 0e84f1b246b74d9ba92fd7eb5b634f640779b3f6 Mon Sep 17 00:00:00 2001 From: Rogerio Pontual <44991200+jyeshe@users.noreply.github.com> Date: Wed, 13 Nov 2024 12:50:42 +0100 Subject: [PATCH] Increase collection item max len (#2661) * Increase collection item max len * Rename table --------- Co-authored-by: Stuart Corbishley --- CHANGELOG.md | 3 ++ lib/lightning/collections.ex | 15 ++++---- lib/lightning/collections/item.ex | 3 +- .../controllers/collections_controller.ex | 13 ++----- ...41112150001_increase_collections_items.exs | 9 +++++ ...13111034_rename_collection_items_table.exs | 37 ++++++++++++++++++ test/integration/web_and_worker_test.exs | 2 +- test/lightning/collections_test.exs | 38 ++++++++++++++++++- 8 files changed, 99 insertions(+), 21 deletions(-) create mode 100644 priv/repo/migrations/20241112150001_increase_collections_items.exs create mode 100644 priv/repo/migrations/20241113111034_rename_collection_items_table.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 2867d13882..38363ec4e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ and this project adheres to ### Changed +- Increase collection items value limit to 1M characters + [#2661](https://github.com/OpenFn/lightning/pull/2661) + ### Fixed - Fix issues loading suggestions for code-assist diff --git a/lib/lightning/collections.ex b/lib/lightning/collections.ex index 04b88c93bc..d549980e09 100644 --- a/lib/lightning/collections.ex +++ b/lib/lightning/collections.ex @@ -161,7 +161,7 @@ defmodule Lightning.Collections do end @spec put_all(Collection.t(), [{String.t(), String.t()}]) :: - {:ok, non_neg_integer()} | :error + {:ok, non_neg_integer()} def put_all(%{id: collection_id}, kv_list) do item_list = Enum.with_index(kv_list, fn %{"key" => key, "value" => value}, @@ -177,13 +177,12 @@ defmodule Lightning.Collections do } end) - case Repo.insert_all(Item, item_list, - conflict_target: [:collection_id, :key], - on_conflict: {:replace, [:value, :updated_at]} - ) do - {n, nil} when n > 0 -> {:ok, n} - _error -> :error - end + with {count, _nil} <- + Repo.insert_all(Item, item_list, + conflict_target: [:collection_id, :key], + on_conflict: {:replace, [:value, :updated_at]} + ), + do: {:ok, count} end @spec delete(Collection.t(), String.t()) :: :ok | {:error, :not_found} diff --git a/lib/lightning/collections/item.ex b/lib/lightning/collections/item.ex index 1ec5f6c755..ec4cfd8751 100644 --- a/lib/lightning/collections/item.ex +++ b/lib/lightning/collections/item.ex @@ -15,7 +15,7 @@ defmodule Lightning.Collections.Item do } @primary_key false - schema "collections_items" do + schema "collection_items" do belongs_to :collection, Lightning.Collections.Collection, primary_key: true field :key, :string, primary_key: true field :value, :string @@ -28,6 +28,7 @@ defmodule Lightning.Collections.Item do entry |> cast(attrs, [:collection_id, :key, :value]) |> validate_required([:collection_id, :key, :value]) + |> validate_length(:value, max: 1_000_000) |> unique_constraint([:collection_id, :key]) |> foreign_key_constraint(:collection_id) end diff --git a/lib/lightning_web/controllers/collections_controller.ex b/lib/lightning_web/controllers/collections_controller.ex index 52774cf50f..6bb373f1c3 100644 --- a/lib/lightning_web/controllers/collections_controller.ex +++ b/lib/lightning_web/controllers/collections_controller.ex @@ -47,16 +47,9 @@ defmodule LightningWeb.CollectionsController do def put_all(conn, %{"name" => col_name, "items" => items}) do with {:ok, collection} <- Collections.get_collection(col_name), - :ok <- authorize(conn, collection) do - case Collections.put_all(collection, items) do - {:ok, count} -> - json(conn, %{upserted: count, error: nil}) - - :error -> - conn - |> put_status(:internal_server_error) - |> json(%{upserted: 0, error: "Database Error"}) - end + :ok <- authorize(conn, collection), + {:ok, count} <- Collections.put_all(collection, items) do + json(conn, %{upserted: count, error: nil}) end end diff --git a/priv/repo/migrations/20241112150001_increase_collections_items.exs b/priv/repo/migrations/20241112150001_increase_collections_items.exs new file mode 100644 index 0000000000..8cf8dfcda0 --- /dev/null +++ b/priv/repo/migrations/20241112150001_increase_collections_items.exs @@ -0,0 +1,9 @@ +defmodule Lightning.Repo.Migrations.IncreaseCollectionsItems do + use Ecto.Migration + + def change do + alter table(:collections_items, primary_key: false) do + modify :value, :string, size: 1_000_000, from: {:string, size: 255} + end + end +end diff --git a/priv/repo/migrations/20241113111034_rename_collection_items_table.exs b/priv/repo/migrations/20241113111034_rename_collection_items_table.exs new file mode 100644 index 0000000000..22f9a20322 --- /dev/null +++ b/priv/repo/migrations/20241113111034_rename_collection_items_table.exs @@ -0,0 +1,37 @@ +defmodule Lightning.Repo.Migrations.RenameCollectionItemsTable do + use Ecto.Migration + + def change do + rename table(:collections_items), to: table(:collection_items) + + execute """ + ALTER INDEX collections_items_key_trgm_idx RENAME TO collection_items_key_trgm_idx + """, + """ + ALTER INDEX collection_items_key_trgm_idx RENAME TO collections_items_key_trgm_idx + """ + + execute """ + ALTER INDEX collections_items_collection_id_key_index RENAME TO collection_items_collection_id_key_index + """, + """ + ALTER INDEX collection_items_collection_id_key_index RENAME TO collections_items_collection_id_key_index + """ + + execute """ + ALTER INDEX collections_items_updated_at_index RENAME TO collection_items_updated_at_index + """, + """ + ALTER INDEX collection_items_updated_at_index RENAME TO collections_items_updated_at_index + """ + + execute """ + ALTER TABLE collection_items DROP CONSTRAINT collections_items_collection_id_fkey, + ADD CONSTRAINT collection_items_collection_id_fkey FOREIGN KEY (collection_id) REFERENCES collections(id) ON DELETE CASCADE + """, + """ + ALTER TABLE collection_item DROP CONSTRAINT collection_items_collection_id_fkey, + ADD CONSTRAINT collections_items_collection_id_fkey FOREIGN KEY (collection_id) REFERENCES collections(id) ON DELETE CASCADE + """ + end +end diff --git a/test/integration/web_and_worker_test.exs b/test/integration/web_and_worker_test.exs index 8e0c9f572b..cec5e401bf 100644 --- a/test/integration/web_and_worker_test.exs +++ b/test/integration/web_and_worker_test.exs @@ -58,7 +58,7 @@ defmodule Lightning.WebAndWorkerTest do conn = post(conn, "/i/#{webhook_trigger_id}", webhook_body) assert %{"work_order_id" => wo_id} = - json_response(conn, 200) |> IO.inspect() + json_response(conn, 200) assert %{runs: [run]} = WorkOrders.get(wo_id, include: [:runs]) diff --git a/test/lightning/collections_test.exs b/test/lightning/collections_test.exs index ec22bd9b4f..37c624ae78 100644 --- a/test/lightning/collections_test.exs +++ b/test/lightning/collections_test.exs @@ -270,6 +270,29 @@ defmodule Lightning.CollectionsTest do Repo.get_by!(Item, key: "some-key") end + test "returns an :error if the value is bigger than max len" do + collection = insert(:collection) + + assert {:error, + %{ + errors: [ + value: + {"should be at most %{count} character(s)", + [ + count: 1_000_000, + validation: :length, + kind: :max, + type: :string + ]} + ] + }} = + Collections.put( + collection, + "key", + String.duplicate("a", 1_000_001) + ) + end + test "returns an :error if the collection does not exist" do assert {:error, %{ @@ -278,7 +301,7 @@ defmodule Lightning.CollectionsTest do {"does not exist", [ constraint: :foreign, - constraint_name: "collections_items_collection_id_fkey" + constraint_name: "collection_items_collection_id_fkey" ]} ] }} = Collections.put(%{id: Ecto.UUID.generate()}, "key", "value") @@ -330,6 +353,19 @@ defmodule Lightning.CollectionsTest do assert %{value: "value5", updated_at: ^updated_at5} = Repo.get_by(Item, key: "key5") end + + test "raises Postgrex error when an item is bigger than allowed max len" do + collection = insert(:collection) + + items = [ + %{"key" => "key1", "value" => "adsf"}, + %{"key" => "key1", "value" => String.duplicate("a", 1_000_001)} + ] + + assert_raise Postgrex.Error, fn -> + Collections.put_all(collection, items) + end + end end describe "delete/2" do