Skip to content

Commit

Permalink
Increase collection item max len (#2661)
Browse files Browse the repository at this point in the history
* Increase collection item max len

* Rename table

---------

Co-authored-by: Stuart Corbishley <corbish@gmail.com>
  • Loading branch information
jyeshe and stuartc authored Nov 13, 2024
1 parent e9aff3c commit 0e84f1b
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 21 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 7 additions & 8 deletions lib/lightning/collections.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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}
Expand Down
3 changes: 2 additions & 1 deletion lib/lightning/collections/item.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
13 changes: 3 additions & 10 deletions lib/lightning_web/controllers/collections_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion test/integration/web_and_worker_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
38 changes: 37 additions & 1 deletion test/lightning/collections_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
%{
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0e84f1b

Please sign in to comment.