Skip to content
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.

Sign up for GitHub

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

Change :read_only field option to :writable #4472

Merged
merged 3 commits into from
Aug 5, 2024
Merged

Change :read_only field option to :writable #4472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e4a94e
read_only to writable
greg-rychlewski Aug 5, 2024
028fae1
more test
greg-rychlewski Aug 5, 2024
2187d20
fix sorting
greg-rychlewski Aug 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 0 additions & 106 deletions integration_test/cases/repo.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1312,112 +1312,6 @@ defmodule Ecto.Integration.RepoTest do
assert TestRepo.get(Post, id).temp == "temp"
end

describe "read only fields" do
test "select with read only field" do
{1, _} = TestRepo.insert_all("posts", [%{title: "1", read_only: "readonly"}])
query = from p in Post, where: p.read_only == ^"readonly", select: p.read_only

assert "readonly" == TestRepo.one(query)
assert %{read_only: "readonly"} = TestRepo.one(Post)
end

test "update with read only field" do
%Post{id: id} = post = TestRepo.insert!(%Post{title: "1"})
cs = Ecto.Changeset.change(post, %{title: "2", read_only: "nope"})
TestRepo.update!(cs)
assert is_nil(TestRepo.get(Post, id).read_only)
end

@tag :returning
test "update with read only field and returning" do
post = TestRepo.insert!(%Post{title: "1"})
cs = Ecto.Changeset.change(post, %{title: "2", read_only: "nope"})
updated_post = TestRepo.update!(cs, returning: true)
assert is_nil(updated_post.read_only)
end

test "update_all with read only field" do
TestRepo.insert!(%Post{title: "1"})
update_query = from p in Post, where: p.title == "1", update: [set: [read_only: "nope"]]

assert_raise Ecto.QueryError, ~r/cannot update unwritable field `read_only` in query/, fn ->
TestRepo.update_all(update_query, [])
end
end

test "insert with read only field" do
%Post{id: id} = TestRepo.insert!(%Post{title: "1", read_only: "nope"})
assert is_nil(TestRepo.get(Post, id).read_only)
end

@tag :returning
test "insert with read only field and returning" do
post = TestRepo.insert!(%Post{title: "1", read_only: "nope"}, returning: true)
assert is_nil(post.read_only)
end

test "insert with read only field and conflict query" do
on_conflict = from Post, update: [set: [read_only: "nope"]]

assert_raise Ecto.QueryError, ~r/cannot update unwritable field `read_only` in query/, fn ->
TestRepo.insert!(%Post{title: "1"}, on_conflict: on_conflict)
end

assert_raise Ecto.QueryError, ~r/cannot update unwritable field `read_only` in query/, fn ->
TestRepo.insert!(%Post{title: "1"}, on_conflict: [set: [read_only: "nope"]])
end
end

test "insert with read only field and conflict replace" do
msg = "cannot replace unwritable field `:read_only` in :on_conflict option"

assert_raise ArgumentError, msg, fn ->
TestRepo.insert!(%Post{title: "1"}, on_conflict: {:replace, [:read_only]})
end
end

@tag :with_conflict_target
@tag :upsert
test "insert with read only field and conflict replace_all" do
uuid = Ecto.UUID.generate()
TestRepo.insert!(%Post{uuid: uuid, title: "1"})
%Post{id: id} = TestRepo.insert!(%Post{uuid: uuid, title: "2"}, conflict_target: [:uuid], on_conflict: :replace_all)
assert %{title: "2", read_only: nil} = TestRepo.get(Post, id)
end

@tag :returning
@tag :with_conflict_target
@tag :upsert
test "insert with read only field and conflict replace_all and returning" do
uuid = Ecto.UUID.generate()
TestRepo.insert!(%Post{uuid: uuid, title: "1"})

post =
TestRepo.insert!(%Post{uuid: uuid, title: "2", read_only: "nope"},
conflict_target: [:uuid],
on_conflict: :replace_all,
returning: true
)

assert %{title: "2", read_only: nil} = post
end

test "insert_all with read only field" do
msg = ~r/Unwritable fields, such as virtual and read only fields are not supported./

assert_raise ArgumentError, msg, fn ->
TestRepo.insert_all(Post, [%{title: "1", read_only: "nope"}])
end

msg = "cannot select unwritable field `read_only` for insert_all"

assert_raise ArgumentError, msg, fn ->
query = from p in Post, select: %{read_only: p.read_only}
TestRepo.insert_all(Post, query)
end
end
end

## Query syntax

defmodule Foo do
Expand Down
1 change: 0 additions & 1 deletion integration_test/support/schemas.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ defmodule Ecto.Integration.Post do
field :links, {:map, :string}
field :intensities, {:map, :float}
field :posted, :date
field :read_only, :string, read_only: true
has_many :comments, Ecto.Integration.Comment, on_delete: :delete_all, on_replace: :delete
has_many :force_comments, Ecto.Integration.Comment, on_replace: :delete_if_exists
has_many :ordered_comments, Ecto.Integration.Comment, preload_order: [:text]
Expand Down
18 changes: 9 additions & 9 deletions lib/ecto/query/planner.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2197,7 +2197,7 @@ defmodule Ecto.Query.Planner do
{{:source, {source, schema}, prefix || query.prefix, types}, fields}

{{:ok, {_, fields}}, _} ->
{{:map, Enum.map(fields, &{&1, {:value, :any}})}, Enum.map(fields, &select_field(&1, ix, false))}
{{:map, Enum.map(fields, &{&1, {:value, :any}})}, Enum.map(fields, &select_field(&1, ix, :always))}

{:error, {:fragment, _, _}} ->
{{:value, :map}, [{:&, [], [ix]}]}
Expand All @@ -2207,7 +2207,7 @@ defmodule Ecto.Query.Planner do

dumper =
types
|> Enum.map(fn {field, type} -> {field, {field, type, false}} end)
|> Enum.map(fn {field, type} -> {field, {field, type, :always}} end)
|> Enum.into(%{})

{types, fields} = select_dump(fields, dumper, ix, drop)
Expand All @@ -2224,7 +2224,7 @@ defmodule Ecto.Query.Planner do

{:error, %Ecto.SubQuery{select: select}} ->
fields = subquery_source_fields(select)
{select, Enum.map(fields, &select_field(&1, ix, false))}
{select, Enum.map(fields, &select_field(&1, ix, :always))}
end
end

Expand All @@ -2233,16 +2233,16 @@ defmodule Ecto.Query.Planner do
|> Enum.reverse()
|> Enum.reduce({[], []}, fn
field, {types, exprs} when is_atom(field) and not is_map_key(drop, field) ->
{source, type, read_only?} = Map.get(dumper, field, {field, :any, false})
{[{field, type} | types], [select_field(source, ix, read_only?) | exprs]}
{source, type, writable} = Map.get(dumper, field, {field, :any, :always})
{[{field, type} | types], [select_field(source, ix, writable) | exprs]}

_field, acc ->
acc
end)
end

defp select_field(field, ix, read_only?) do
{{:., [read_only: read_only?], [{:&, [], [ix]}, field]}, [], []}
defp select_field(field, ix, writable) do
{{:., [writable: writable], [{:&, [], [ix]}, field]}, [], []}
end

defp get_ix!({:&, _, [ix]} = expr, _kind, query) do
Expand Down Expand Up @@ -2609,8 +2609,8 @@ defmodule Ecto.Query.Planner do
end

case dumper do
%{^k => {_, _, false}} -> :ok
%{} -> error!(query, "cannot update unwritable field `#{k}`")
%{^k => {_, _, :always}} -> :ok
%{} -> error!(query, "cannot update non-updatable field `#{inspect(k)}`")
nil -> :ok
end

Expand Down
30 changes: 15 additions & 15 deletions lib/ecto/repo/schema.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ defmodule Ecto.Repo.Schema do
defp init_mapper(schema, dumper, adapter, placeholder_map) do
fn {field, value}, acc ->
case dumper do
%{^field => {source, type, false}} ->
%{^field => {source, type, writable}} when writable != :never ->
extract_value(source, value, type, placeholder_map, acc, fn val ->
dump_field!(:insert_all, schema, field, type, val, adapter)
end)
Expand Down Expand Up @@ -274,17 +274,17 @@ defmodule Ecto.Repo.Schema do
end

defp insert_all_select_dump!({{:., dot_meta, [{:&, _, [_]}, field]}, [], []}) do
if dot_meta[:read_only] == true do
raise ArgumentError, "cannot select unwritable field `#{field}` for insert_all"
if dot_meta[:writable] == :never do
raise ArgumentError, "cannot select unwritable field `#{inspect(field)}` for insert_all"
else
field
end
end

defp insert_all_select_dump!(field, dumper) when is_atom(field) do
case dumper do
%{^field => {source, _, false}} -> source
%{} -> raise ArgumentError, "cannot select unwritable field `#{field}` for insert_all"
%{^field => {source, _, writable}} when writable != :never -> source
%{} -> raise ArgumentError, "cannot select unwritable field `#{inspect(field)}` for insert_all"
nil -> field
end
end
Expand Down Expand Up @@ -366,7 +366,7 @@ defmodule Ecto.Repo.Schema do
struct = struct_from_changeset!(:insert, changeset)
schema = struct.__struct__
dumper = schema.__schema__(:dump)
writable_fields = schema.__schema__(:writable_fields)
insertable_fields = schema.__schema__(:insertable_fields)
assocs = schema.__schema__(:associations)
embeds = schema.__schema__(:embeds)

Expand All @@ -383,7 +383,7 @@ defmodule Ecto.Repo.Schema do
# On insert, we always merge the whole struct into the
# changeset as changes, except the primary key if it is nil.
changeset = put_repo_and_action(changeset, :insert, repo, tuplet)
changeset = Relation.surface_changes(changeset, struct, writable_fields ++ assocs)
changeset = Relation.surface_changes(changeset, struct, insertable_fields ++ assocs)

wrap_in_transaction(adapter, adapter_meta, opts, changeset, assocs, embeds, prepare, fn ->
assoc_opts = assoc_opts(assocs, opts)
Expand All @@ -402,7 +402,7 @@ defmodule Ecto.Repo.Schema do
{changes, cast_extra, dump_extra, return_types, return_sources} =
autogenerate_id(autogen_id, changes, return_types, return_sources, adapter)

changes = Map.take(changes, writable_fields)
changes = Map.take(changes, insertable_fields)
autogen = autogenerate_changes(schema, :insert, changes)

dump_changes =
Expand Down Expand Up @@ -458,7 +458,7 @@ defmodule Ecto.Repo.Schema do
struct = struct_from_changeset!(:update, changeset)
schema = struct.__struct__
dumper = schema.__schema__(:dump)
writable_fields = schema.__schema__(:writable_fields)
updatable_fields = schema.__schema__(:updatable_fields)
assocs = schema.__schema__(:associations)
embeds = schema.__schema__(:embeds)

Expand Down Expand Up @@ -487,7 +487,7 @@ defmodule Ecto.Repo.Schema do
if changeset.valid? do
embeds = Ecto.Embedded.prepare(changeset, embeds, adapter, :update)

changes = changeset.changes |> Map.merge(embeds) |> Map.take(writable_fields)
changes = changeset.changes |> Map.merge(embeds) |> Map.take(updatable_fields)
autogen = autogenerate_changes(schema, :update, changes)
dump_changes = dump_changes!(:update, changes, autogen, schema, [], dumper, adapter)

Expand Down Expand Up @@ -669,7 +669,7 @@ defmodule Ecto.Repo.Schema do
end
defp fields_to_sources(fields, dumper) do
Enum.reduce(fields, {[], []}, fn field, {types, sources} ->
{source, type, _read_only?} = Map.fetch!(dumper, field)
{source, type, _writable} = Map.fetch!(dumper, field)
{[{field, type} | types], [source | sources]}
end)
end
Expand Down Expand Up @@ -786,12 +786,12 @@ defmodule Ecto.Repo.Schema do
defp replace_fields!(dumper, fields) do
Enum.map(fields, fn field ->
case dumper do
%{^field => {source, _type, false}} ->
%{^field => {source, _type, :always}} ->
source

_ ->
raise ArgumentError,
"cannot replace unwritable field `#{inspect(field)}` in :on_conflict option"
"cannot replace non-updatable field `#{inspect(field)}` in :on_conflict option"
end
end)
end
Expand All @@ -801,7 +801,7 @@ defmodule Ecto.Repo.Schema do
end

defp replace_all_fields!(_kind, schema, to_remove) do
Enum.map(schema.__schema__(:writable_fields) -- to_remove, &field_source!(schema, &1))
Enum.map(schema.__schema__(:updatable_fields) -- to_remove, &field_source!(schema, &1))
end

defp field_source!(nil, field) do
Expand Down Expand Up @@ -1117,7 +1117,7 @@ defmodule Ecto.Repo.Schema do

defp dump_fields!(action, schema, kw, dumper, adapter) do
for {field, value} <- kw do
{alias, type, _read_only?} = Map.fetch!(dumper, field)
{alias, type, _writable} = Map.fetch!(dumper, field)
{alias, dump_field!(action, schema, field, type, value, adapter)}
end
end
Expand Down
Loading
Loading