Skip to content

Commit

Permalink
Improve generators attributes
Browse files Browse the repository at this point in the history
Fixes phoenixframework#5987

Structural and general changes

- (1) Extract `Mix.Phoenix.Attribute` module. It parses cli attribute into `Attribute` struct, with validation of types and options, and prefilling some default options. Covered with tests.
- (2) Create specs for attribute types and options, with examples, to be used in generators docs and in console errors.
- (3) Extract `Mix.Phoenix.Migration` module. It prepares data, based on `Attribute` struct, to apply in migration file. Covered with tests. This eliminate migration data preparation in cases with no migration generation (e.g. for embedded schema or when flag is passed). Thoughts for later: potentially this logic can be used in new `mix phx.gen.migration`, as extension of `mix echo.gen.migration` with attributes.
- (4) Extract `Mix.Phoenix.TestData` module. It prepares data, based on `Attribute` struct, to apply in test files. Covered with tests.
- (5) Refactor only schema related data preparation in `Mix.Phoenix.Schema`, based on `Attribute` struct. Only parsing cli attributes and preparing general `sample_values` is done during creation of `Schema` struct. Specific data for generated files are created on demand based on `schema.attrs` and `schema.sample_values`.
- (6) Rely on correct type inferred from referenced schema for associations, get rid of unneeded any more general foreign type setup as schema module attribute. It original implementation there it was a bug when referenced schema had different type.
- (7) Extracted `Mix.Phoenix.Web` module. It prepares data, based on `Attribute` struct, to apply in html, live files. Covered with tests. It also postpone data generation. Thus e.g. data generation will be invoked only if conflicts check passes.
- (8) Add utility function `Mix.Phoenix.indent_text` to indent text or list of lines with spaces, and prepend and append empty lines when needed. Use it for new and old refactored cases.
- (9) Generate `context` files before `json`, `html`, `live` files, as it doesn't need specific extra bindings.
- (10) Extract fixture data generation, to be done only when fixture is going to be created.
- (11) Extract html assertion data generation, to be done only when web files is going to be created.
- (12) Correct array values rendering for index and html. Array of integers is interpolating into string with wrong representation (`"#{[1,2]}"` is `<<1, 2>>`, `"#{[42,43]}"` is `"*+"`). And it even leads to errors during run of generated tests (e.g. `"#{[1042,1043]}"` leads to error `(ArgumentError) lists in Phoenix.HTML templates only support iodata, and not chardata.`). Correct with simple default rendering logic - adjustment with `(values || []) |> List.flatten() |> Enum.join(", ")`. It works for any levels nesting arrays. And feels reasonable default representation for array values.
- (13) In generated tests files, relocate `create_attrs` and `update_attrs` from module attributes into `test` body. More practical approach, as more often we need to generate some data. E.g. with current improvement we add generation of associated records, which is pretty general case (to have associations).

Attributes related changes

- (14) Fix `[array,enum]` issue. Potentially nested arrays like `[array,[array,enum]]` should work as well (for now it is outside of this change scope).
- (15) Add `array` alias for `[array,string]`.
- (16) Fix possible collision of `enum` values with options. Extract enum values into specific options. Add support both variants: string option `[one,two]`, integer option `[[one,1],[two,2]]`. Adjust migration and other files representation.
- (17) Add option-flag `required` to use in migration, schema and input form in html and live. When no attribute marked as required, the first given attribute is set to be required, with provided notification and confirmation message in console.
- (18) Add alias `*` for option-flag `required`.
- (19) Add option-flag `virtual` to use in schema and field skipping in migration. Add support of type `any` to be used only with this flag (add validation logic).
- (20) Add option-flag `index` to use in migration.
- (21) Add `default,value` option for types `boolean`, `integer`, `decimal`, `float`.
- (22) Add `precision,value` and `scale,value` options for `decimal` type, to use in migration and test files. Add validations: `scale` can be used only with `precision`, `scale` have to be less then `precision`, minimum for `scale` is `1`, minimum for `precision` is `2`.
- (23) Add option `Context.Schema` for `references` type. This options should be used when referenced schema cannot be inferred from the attribute name (e.g. when it has different context). Referenced table and type will be properly inferred from schema reflection. It should fix issue with incorrect references type in case when schema created `--binary-id` option, but referenced schema was created with general `id`. And vice versa.
- (24) Add option `assoc,value` for `references` type. For cases when it cannot be inferred from the attribute name.
- (25) Add option `on_delete,value` for `references` type. Pretty often we want to deviate from default value `nothing`.
- (26) Add option `column,value` for `references` type. For cases when referenced column differs from default `id`.
- (27) Add option `type,value` for `references` type. For rear cases when referenced schema is not reachable (e.g. in generators' tests), or when referenced column is not default `id`. For now support values `id`, `binary_id`, `string`.
- (28) Add option `table,value` for `references` type. For rear cases when referenced schema is not reachable (e.g. in generators' tests).
- (29) Update guides about changes in `references` interface and other parts.

Notes

NOTE: (1) One drawback so far is necessity to specify `string` type if any option is provided. Which can be neglected I think. Otherwise we need more specific separation between name-type and options, something in a way of `name:type|options`. Then it would be easy and safe to parse omitted string type in `name|options`. Let me know if you want apply this approach. it should be easy enough to modify.

NOTE: (2) There is bug in current original version: live test fail in case of invalid attributes, when only boolean attributes exists (cannot find `"can&phoenixframework#39;t be blank"` text.). An this bug still exists in this PR. It is out of scope of this PR to decide approach to fix it.

NOTE: (3) Looks like there is issue with recently added `--primary-key` option (generated code has errors on execution "(Postgrex.Error) ERROR 23502 (not_null_violation) null value in column" - related to primary key column). An this bug still exists in this PR. It is out of scope of this PR to decide approach to fix it.
  • Loading branch information
ShPakvel committed Nov 28, 2024
1 parent 092605f commit ed6ef80
Show file tree
Hide file tree
Showing 56 changed files with 5,369 additions and 1,775 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ future styling).

### Enhancements
* [phx.gen.auth] Add enhanced session fixation protection.
For applications whichs previously used `phx.gen.auth`, the following line can be added to the `renew_session` function in the auth module:
For applications which's previously used `phx.gen.auth`, the following line can be added to the `renew_session` function in the auth module:

```diff
defp renew_session(conn) do
Expand Down Expand Up @@ -231,7 +231,7 @@ future styling).
* [Router] Add `Phoenix.VerifiedRoutes` for `~p`-based route generation with compile-time verification.
* [Router] Support `helpers: false` to `use Phoenix.Router` to disable helper generation
* [Router] Add `--info [url]` switch to `phx.routes` to get route information about a url/path
* [Flash] Add `Phoenix.Flash` for unfied flash access
* [Flash] Add `Phoenix.Flash` for unified flash access

### JavaScript Client Bug Fixes
* Fix heartbeat being sent after disconnect and causing abnormal disconnects
Expand Down
129 changes: 60 additions & 69 deletions guides/contexts.md

Large diffs are not rendered by default.

26 changes: 13 additions & 13 deletions guides/ecto.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ This guide assumes that we have generated our new application with Ecto integrat
Once we have Ecto and PostgreSQL installed and configured, the easiest way to use Ecto is to generate an Ecto *schema* through the `phx.gen.schema` task. Ecto schemas are a way for us to specify how Elixir data types map to and from external sources, such as database tables. Let's generate a `User` schema with `name`, `email`, `bio`, and `number_of_pets` fields.

```console
$ mix phx.gen.schema User users name:string email:string \
bio:string number_of_pets:integer
$ mix phx.gen.schema User users name:string:required email:string:* \
bio:string:* number_of_pets:integer:*

* creating ./lib/hello/user.ex
* creating priv/repo/migrations/20170523151118_create_users.exs
Expand Down Expand Up @@ -72,18 +72,18 @@ hello_dev=# \d
hello_dev=# \q
```

If we take a look at the migration generated by `phx.gen.schema` in `priv/repo/migrations/`, we'll see that it will add the columns we specified. It will also add timestamp columns for `inserted_at` and `updated_at` which come from the [`timestamps/1`] function.
If we take a look at the migration generated by `phx.gen.schema` in `priv/repo/migrations/`, we'll see that it will add the columns we specified. With `null: false` constraint for required fields. It will also add timestamp columns for `inserted_at` and `updated_at` which come from the [`timestamps/1`] function.

```elixir
defmodule Hello.Repo.Migrations.CreateUsers do
use Ecto.Migration

def change do
create table(:users) do
add :name, :string
add :email, :string
add :bio, :string
add :number_of_pets, :integer
create table("users") do
add :name, :string, null: false
add :email, :string, null: false
add :bio, :string, null: false
add :number_of_pets, :integer, null: false

timestamps()
end
Expand All @@ -100,10 +100,10 @@ Table "public.users"
Column | Type | Modifiers
---------------+-----------------------------+----------------------------------------------------
id | bigint | not null default nextval('users_id_seq'::regclass)
name | character varying(255) |
email | character varying(255) |
bio | character varying(255) |
number_of_pets | integer |
name | character varying(255) | not null
email | character varying(255) | not null
bio | character varying(255) | not null
number_of_pets | integer | not null
inserted_at | timestamp without time zone | not null
updated_at | timestamp without time zone | not null
Indexes:
Expand Down Expand Up @@ -194,7 +194,7 @@ Right now, we have two transformations in our pipeline. In the first call, we in

[`cast/3`] first takes a struct, then the parameters (the proposed updates), and then the final field is the list of columns to be updated. [`cast/3`] also will only take fields that exist in the schema.

Next, `Ecto.Changeset.validate_required/3` checks that this list of fields is present in the changeset that [`cast/3`] returns. By default with the generator, all fields are required.
Next, `Ecto.Changeset.validate_required/3` checks that this list of fields is present in the changeset that [`cast/3`] returns. In generation command we added option `required` to fields `name` and alias its `*` to rest of the fields. By default if no field is required then generator will set first given field as required, notifying us and asking for approval before generation.

We can verify this functionality in `IEx`. Let's fire up our application inside IEx by running `iex -S mix`. In order to minimize typing and make this easier to read, let's alias our `Hello.User` struct.

Expand Down
2 changes: 1 addition & 1 deletion guides/json_and_apis.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ For this guide let's create a simple JSON API to store our favourite links, that
For this guide, we will use Phoenix generators to scaffold our API infrastructure:

```console
mix phx.gen.json Urls Url urls link:string title:string
mix phx.gen.json Urls Url urls link:string:* title:string:*
* creating lib/hello_web/controllers/url_controller.ex
* creating lib/hello_web/controllers/url_json.ex
* creating lib/hello_web/controllers/changeset_json.ex
Expand Down
10 changes: 5 additions & 5 deletions guides/mix_tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ We will cover all Phoenix Mix tasks, except `phx.new`, `phx.new.ecto`, and `phx.

Phoenix offers the ability to generate all the code to stand up a complete HTML resource — Ecto migration, Ecto context, controller with all the necessary actions, view, and templates. This can be a tremendous time saver. Let's take a look at how to make this happen.

The `mix phx.gen.html` task takes the following arguments: the module name of the context, the module name of the schema, the resource name, and a list of column_name:type attributes. The module name we pass in must conform to the Elixir rules of module naming, following proper capitalization.
The `mix phx.gen.html` task takes the following arguments: the module name of the context, the module name of the schema, the resource name, and a list of `name:type:options` attributes. The module name we pass in must conform to the Elixir rules of module naming, following proper capitalization.

```console
$ mix phx.gen.html Blog Post posts body:string word_count:integer
Expand Down Expand Up @@ -132,7 +132,7 @@ It will tell us we need to add a line to our router file, but since we skipped t

Phoenix also offers the ability to generate all the code to stand up a complete JSON resource — Ecto migration, Ecto schema, controller with all the necessary actions and view. This command will not create any template for the app.

The `mix phx.gen.json` task takes the following arguments: the module name of the context, the module name of the schema, the resource name, and a list of column_name:type attributes. The module name we pass in must conform to the Elixir rules of module naming, following proper capitalization.
The `mix phx.gen.json` task takes the following arguments: the module name of the context, the module name of the schema, the resource name, and a list of `name:type:options` attributes. The module name we pass in must conform to the Elixir rules of module naming, following proper capitalization.

```console
$ mix phx.gen.json Blog Post posts title:string content:string
Expand Down Expand Up @@ -179,7 +179,7 @@ warning: no route path for HelloWeb.Router matches \"/posts\"

If we don't need a complete HTML/JSON resource and only need a context, we can use the `mix phx.gen.context` task. It will generate a context, a schema, a migration and a test case.

The `mix phx.gen.context` task takes the following arguments: the module name of the context, the module name of the schema, the resource name, and a list of column_name:type attributes.
The `mix phx.gen.context` task takes the following arguments: the module name of the context, the module name of the schema, the resource name, and a list of `name:type:options` attributes.

```console
$ mix phx.gen.context Accounts User users name:string age:integer
Expand Down Expand Up @@ -211,10 +211,10 @@ $ mix phx.gen.context Admin.Accounts User users name:string age:integer

If we don't need a complete HTML/JSON resource and are not interested in generating or altering a context we can use the `mix phx.gen.schema` task. It will generate a schema, and a migration.

The `mix phx.gen.schema` task takes the following arguments: the module name of the schema (which may be namespaced), the resource name, and a list of column_name:type attributes.
The `mix phx.gen.schema` task takes the following arguments: the module name of the schema (which may be namespaced), the resource name, and a list of `name:type:options` attributes.

```console
$ mix phx.gen.schema Accounts.Credential credentials email:string:unique user_id:references:users
$ mix phx.gen.schema Accounts.Credential credentials email:string:unique user_id:references
* creating lib/hello/accounts/credential.ex
* creating priv/repo/migrations/20170906162013_create_credentials.exs
```
Expand Down
17 changes: 10 additions & 7 deletions guides/testing/testing_contexts.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
At the end of the Introduction to Testing guide, we generated an HTML resource for posts using the following command:

```console
$ mix phx.gen.html Blog Post posts title body:text
$ mix phx.gen.html Blog Post posts title:string:* body:text:*
```

This gave us a number of modules for free, including a Blog context and a Post schema, alongside their respective test files. As we have learned in the Context guide, the Blog context is simply a module with functions to a particular area of our business domain, while Post schema maps to a particular table in our database.
Expand Down Expand Up @@ -60,7 +60,7 @@ Next, we define an alias, so we can refer to `Hello.Blog` simply as `Blog`.
Then we start a `describe "posts"` block. A `describe` block is a feature in ExUnit that allows us to group similar tests. The reason why we have grouped all post related tests together is because contexts in Phoenix are capable of grouping multiple schemas together. For example, if we ran this command:

```console
$ mix phx.gen.html Blog Comment comments post_id:references:posts body:text
$ mix phx.gen.html Blog Comment comments post_id:references body:text
```

We will get a bunch of new functions in the `Hello.Blog` context, plus a whole new `describe "comments"` block in our test file.
Expand All @@ -69,11 +69,14 @@ The tests defined for our context are very straight-forward. They call the funct

```elixir
test "create_post/1 with valid data creates a post" do
valid_attrs = %{body: "some body", title: "some title"}

assert {:ok, %Post{} = post} = Blog.create_post(valid_attrs)
assert post.body == "some body"
assert post.title == "some title"
create_attrs = %{
body: "body value",
title: "title value"
}

assert {:ok, %Post{} = post} = Blog.create_post(create_attrs)
assert post.body == "body value"
assert post.title == "title value"
end
```

Expand Down
26 changes: 17 additions & 9 deletions guides/testing/testing_controllers.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
At the end of the Introduction to Testing guide, we generated an HTML resource for posts using the following command:

```console
$ mix phx.gen.html Blog Post posts title body:text
$ mix phx.gen.html Blog Post posts title:string:* body:text:*
```

This gave us a number of modules for free, including a PostController and the associated tests. We are going to explore those tests to learn more about testing controllers in general. At the end of the guide, we will generate a JSON resource, and explore how our API tests look like.
Expand All @@ -22,10 +22,8 @@ defmodule HelloWeb.PostControllerTest do

import Hello.BlogFixtures

@create_attrs %{body: "some body", title: "some title"}
@update_attrs %{body: "some updated body", title: "some updated title"}
@invalid_attrs %{body: nil, title: nil}

describe "index" do
test "lists all posts", %{conn: conn} do
conn = get(conn, ~p"/posts")
Expand All @@ -36,7 +34,7 @@ defmodule HelloWeb.PostControllerTest do
...
```

Similar to the `PageControllerTest` that ships with our application, this controller tests uses `use HelloWeb.ConnCase` to setup the testing structure. Then, as usual, it defines some aliases, some module attributes to use throughout testing, and then it starts a series of `describe` blocks, each of them to test a different controller action.
Similar to the `PageControllerTest` that ships with our application, this controller tests uses `use HelloWeb.ConnCase` to setup the testing structure. Then, as usual, it defines some aliases, module attribute for invalid data to use throughout testing, and then it starts a series of `describe` blocks, each of them to test a different controller action.

### The index action

Expand Down Expand Up @@ -87,7 +85,12 @@ Since there are two possible outcomes for the `create`, we will have at least tw
```elixir
describe "create post" do
test "redirects to show when data is valid", %{conn: conn} do
conn = post(conn, ~p"/posts", post: @create_attrs)
create_attrs %{
body: "body value",
title: "title value"
}

conn = post(conn, ~p"/posts", post: create_attrs)

assert %{id: id} = redirected_params(conn)
assert redirected_to(conn) == ~p"/posts/#{id}"
Expand Down Expand Up @@ -321,15 +324,20 @@ This is precisely what the first test for the `create` action verifies:
```elixir
describe "create article" do
test "renders article when data is valid", %{conn: conn} do
conn = post(conn, ~p"/articles", article: @create_attrs)
create_attrs %{
body: "body value",
title: "title value"
}

conn = post(conn, ~p"/articles", article: create_attrs)
assert %{"id" => id} = json_response(conn, 201)["data"]

conn = get(conn, ~p"/api/articles/#{id}")

assert %{
"id" => ^id,
"body" => "some body",
"title" => "some title"
"body" => "body value",
"title" => "title value"
} = json_response(conn, 200)["data"]
end
```
Expand Down
30 changes: 24 additions & 6 deletions integration_test/test/code_generation/app_with_defaults_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do
with_installer_tmp("app_with_defaults", fn tmp_dir ->
{app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog")

mix_run!(~w(phx.gen.html Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), app_root_path)
mix_run!(
~w(phx.gen.html Blog Post posts title:string:*:unique body:string status:enum:[unpublished,published,deleted]),
app_root_path
)

modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file ->
inject_before_final_end(file, """
Expand All @@ -50,7 +53,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do
with_installer_tmp("app_with_defaults", fn tmp_dir ->
{app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog")

mix_run!(~w(phx.gen.html Blog Post posts title:unique body:string status:enum:unpublished:published:deleted order:integer:unique), app_root_path)
mix_run!(
~w(phx.gen.html Blog Post posts title:string:*:unique body:string status:enum:[unpublished,published,deleted] order:integer:unique),
app_root_path
)

modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file ->
inject_before_final_end(file, """
Expand All @@ -74,7 +80,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do
with_installer_tmp("app_with_defaults", fn tmp_dir ->
{app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog")

mix_run!(~w(phx.gen.json Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), app_root_path)
mix_run!(
~w(phx.gen.json Blog Post posts title:string:*:unique body:string status:enum:[unpublished,published,deleted]),
app_root_path
)

modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file ->
inject_before_final_end(file, """
Expand All @@ -97,7 +106,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do
with_installer_tmp("app_with_defaults", fn tmp_dir ->
{app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog")

mix_run!(~w(phx.gen.json Blog Post posts title:unique body:string status:enum:unpublished:published:deleted), app_root_path)
mix_run!(
~w(phx.gen.json Blog Post posts title:string:*:unique body:string status:enum:[unpublished,published,deleted]),
app_root_path
)

modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file ->
inject_before_final_end(file, """
Expand All @@ -121,7 +133,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do
with_installer_tmp("app_with_defaults", fn tmp_dir ->
{app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--live"])

mix_run!(~w(phx.gen.live Blog Post posts title:unique body:string p:boolean s:enum:a:b:c), app_root_path)
mix_run!(
~w(phx.gen.live Blog Post posts title:string:*:unique body:string p:boolean s:enum:[a,b,c]),
app_root_path
)

modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file ->
inject_before_final_end(file, """
Expand All @@ -147,7 +162,10 @@ defmodule Phoenix.Integration.CodeGeneration.AppWithDefaultsTest do
with_installer_tmp("app_with_defaults", fn tmp_dir ->
{app_root_path, _} = generate_phoenix_app(tmp_dir, "phx_blog", ["--live"])

mix_run!(~w(phx.gen.live Blog Post posts title body:string public:boolean status:enum:unpublished:published:deleted), app_root_path)
mix_run!(
~w(phx.gen.live Blog Post posts title:string:* body:string public:boolean status:enum:[unpublished,published,deleted]),
app_root_path
)

modify_file(Path.join(app_root_path, "lib/phx_blog_web/router.ex"), fn file ->
inject_before_final_end(file, """
Expand Down
Loading

0 comments on commit ed6ef80

Please sign in to comment.