Skip to content

improvement: include context in inspect consistently #2441

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+41 −1
Draft

improvement: include context in inspect consistently #2441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lib/ash/changeset/changeset.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ defmodule Ash.Changeset do
if context == %{} do
empty()
else
concat("context: ", to_doc(context, opts))
concat("context: ", to_doc(Ash.Helpers.redact(context), opts))
Copy link
Contributor

@zachdaniel zachdaniel Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There will pretty much always be a context. Could we perhaps to some work to check which keys are commonly included in the context (mostly just private) and only show it if it has any keys aside from those?

Copy link
Contributor

@zachdaniel zachdaniel Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps only show the keys of the context? Context<keys: [:foo, :bar]>?

Copy link
Author

@felixbr felixbr Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how I would know ahead of time which keys are considered "common" and which are considered "additional" (aside from :private).

But showing the keys without values sounds like a great idea. I'll rework it soon™, possibly with a redact_context/redact_struct function so it's easier to have consistent behaviour 👍

Do you have any opinion if :private should be omitted from the output? When showing the full struct it makes sense to hide that internal implementation detail, but then again this is about debugging where internal state might be something a dev wants to know about.

Copy link
Contributor

@zachdaniel zachdaniel Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:private should be yeah. Let's just set it up with that list, and make the list be :private, i.e Map.keys(context) -- @hide_keys etc. And if I spot more later then we can add them to that list 😆

end

tenant =
Expand Down
4 changes: 4 additions & 0 deletions lib/ash/query/query.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ defmodule Ash.Query do
| calculations: Ash.Actions.Read.Calculations.map_without_calc_deps(query.calculations)
}

context = Map.delete(query.context, :private)

sort? = query.sort != []
distinct_sort? = query.distinct_sort != []
load? = query.load != []
Expand All @@ -323,6 +325,7 @@ defmodule Ash.Query do
filter? = not is_nil(query.filter)
errors? = not Enum.empty?(query.errors)
tenant? = not is_nil(query.tenant)
context? = query.context != %{}
select? = query.select not in [[], nil]
distinct? = query.distinct not in [[], nil]
lock? = not is_nil(query.lock)
Expand All @@ -338,6 +341,7 @@ defmodule Ash.Query do
not is_nil(query.action)
),
or_empty(concat("tenant: ", to_doc(query.to_tenant, opts)), tenant?),
or_empty(concat("context: ", to_doc(Ash.Helpers.redact(context), opts)), context?),
arguments(query, opts),
# TODO: inspect these specially
or_empty(
Expand Down
20 changes: 20 additions & 0 deletions test/changeset/changeset_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,26 @@ defmodule Ash.Test.Changeset.ChangesetTest do
assert post.slug == "foo-bar"
end

describe "inspect" do
test "empty map context is omitted" do
changeset =
Category
|> Ash.Changeset.for_create(:create, %{name: "foo"})
|> Ash.Changeset.set_context(%{})

refute inspect(changeset) =~ "context:"
end

test "nonempty map context is redacted" do
changeset =
Category
|> Ash.Changeset.for_create(:create, %{name: "foo"})
|> Ash.Changeset.set_context(%{secret: "value"})

assert inspect(changeset) =~ "context: \"**redacted**\""
end
end

describe "new" do
test "it wraps a new resource in a `create` changeset" do
assert %Ash.Changeset{
Expand Down
16 changes: 16 additions & 0 deletions test/query_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,22 @@ defmodule Ash.Test.QueryTest do
assert inspect(Ash.Query.new(User)) == "#Ash.Query<resource: Ash.Test.QueryTest.User>"
assert inspect(Ash.Query.for_read(User, :by_id, %{id: "foobar"})) =~ "action: :by_id"
end

test "empty map context is omitted" do
query = Ash.Query.new(User) |> Ash.Query.set_context(%{})

assert inspect(query) == "#Ash.Query<resource: Ash.Test.QueryTest.User>"
end

test "nonempty map context is redacted" do
query = Ash.Query.new(User) |> Ash.Query.set_context(%{secret: "value"})

# We show that there is a context present but redact its contents as it might be sensitive.
# During development the context can be inspected either by inspecting it directly or by
# enabling :show_sensitive? in the dev config.
assert inspect(query) ==
"#Ash.Query<resource: Ash.Test.QueryTest.User, context: \"**redacted**\">"
end
end

describe "read argument validation" do
Expand Down