Improve generators attributes

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) 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. - (7) 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. - (8) Generate `context` files before `json`, `html`, `live` files, as it doesn't need specific extra bindings. - (9) Extract fixture data generation, to be done only when fixture is going to be created. - (10) Extract html assertion data generation, to be done only when web files is going to be created. - (11) 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. - (12) 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 - (13) Fix `[array,enum]` issue. Potentially nested arrays like `[array,[array,enum]]` should work as well (for now it is outside of this change scope). - (14) Add `array` alias for `[array,string]`. - (15) 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. - (16) 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. - (17) Add alias `*` for option-flag `required`. - (18) 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). - (19) Add option-flag `index` to use in migration. - (20) Add `default,value` option for types `boolean`, `integer`, `decimal`, `float`. - (21) 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`. - (22) 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. - (23) Add option `assoc,value` for `references` type. For cases when it cannot be inferred from the attribute name. - (24) Add option `on_delete,value` for `references` type. Pretty often we want to deviate from default value `nothing`. - (25) Add option `column,value` for `references` type. For cases when referenced column differs from default `id`. - (26) 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`. - (27) Add option `table,value` for `references` type. For rear cases when referenced schema is not reachable (e.g. in generators' tests). - (28) Update guides about changes in `references` interface and other parts. Notes NOTE: (29) 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: (30) 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.
ShPakvel · Nov 26, 2024 · 816ab04 · 816ab04
1 parent 8bcf808
commit 816ab04
Show file tree

Hide file tree

Showing 56 changed files with 5,200 additions and 1,717 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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
@@ -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

diff --git a/guides/contexts.md b/guides/contexts.md
diff --git a/guides/ecto.md b/guides/ecto.md
@@ -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
@@ -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
@@ -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:
@@ -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.
 

diff --git a/guides/json_and_apis.md b/guides/json_and_apis.md
@@ -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

diff --git a/guides/mix_tasks.md b/guides/mix_tasks.md
@@ -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
@@ -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
@@ -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
@@ -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
 ```

diff --git a/guides/testing/testing_contexts.md b/guides/testing/testing_contexts.md
@@ -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.
@@ -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.
@@ -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
 ```
 

diff --git a/guides/testing/testing_controllers.md b/guides/testing/testing_controllers.md
@@ -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.
@@ -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")
@@ -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
 
@@ -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}"
@@ -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
 ```

diff --git a/integration_test/test/code_generation/app_with_defaults_test.exs b/integration_test/test/code_generation/app_with_defaults_test.exs
@@ -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, """
@@ -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, """
@@ -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, """
@@ -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, """
@@ -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, """
@@ -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, """