Phoenix v1.3.0-rc.0 released

Phoenix Forum Phoenix News
61

An RC can be treated as a final release, only bugs will be fixed :slight_smile:

2 Likes
62

Still, it is not recommended to use a RC in production, while you can a final release.

3 Likes
63

I wanted to provide some feedback on the changes in Phoenix 1.3.

First, the organization for a new umbrella app is really nice. Moving my models into a separate OTP app that the web app references is something I pieced together in 1.2 with examples posted on this forum. (https://github.com/wojtekmach/acme_bank)

Grouping up functional areas for the models is probably a good idea. It happens naturally over the course of a larger application, and it’s a solid DDD approach, so I am for that.

Where I am having trouble is the top level files we will be maintaining, like accounts.ex and accounts_test.exs.

  1. Many functions
    These files are going to become unwieldy very quickly: 3 models is 21 functions and that’s just getting started.

  2. That are very similar
    These files have repeated functionality that is extremely similar across models. Accounts.list_users, Accounts.list_roles, Accounts.list_permissions. My thought is that I should get Accounts.User.list / get! / update / etc for free through a used module. I am trying to weigh that against being too OO, but I think there is precedence with user Genserver and providing callbacks to supply arguments the common functionality will use. A function that provides a changeset function and struct/map type to a Model module is what comes to mind.

  3. Changesets aren’t by their schemas
    I am finding the natural flow for me is to look at my schema while writing my changeset logic, and even though I can open it in another tab, it feels like it should be in the same file.

I like the 1.2 idea of testing models separate from repo side effects. I don’t want to lock into existing ways, but I think some of the new changes are going to result in more code the end developers will have to write and maintain.

I realize I am free to do as I please, and that a lot of thought went into these changes. Overall the changes are great, but maybe the above can help make things a little smoother. Thanks.

4 Likes
64

Thanks for the feedback! Answers inline.

21 functions is not a huge API surface area. I can understand the concern of God Modules, but as previous posts have shown, it’s difficult to make suggestions if a module is doing too much without concrete use cases. Also note that just because you have 3 entities in your context, it does not necessarily mean you’ll have the same conventional functions for all of them. Sometimes you will, sometimes you won’t. For example, imagine a a bank system with a Deposit, an Account, and a AccountHolder. Under this case, you likely wouldn’t expose the crud functions for deposits at all, instead delegating the details of that deeper into the system. That’s not to say you won’t end up with > 21 functions. As JosĂ© has said, it’s easier to start with the parts of your app in the same place, then see where they can be separated than trying to predict the future up front. If these parts of your app are necessarily coupled and depend on one another, grouping together their functionality is not a problem. If you have specific examples we can try to say more.

There is duplication here if all your entities are using ecto, but I don’t feel this is a good candidate for code injection. use GenServer is injecting code to satisfy a behaviour with known functionality. The point of your contexts is that they are the boundary to the internal details of listing users/roles/permissions. If your user permissions are stored in ets, but the users and roles in Postgres, then you can’t just inject code. I also would prefer to see an explicit call when viewing the module. If you want to offload the query generation that you handle the same way, then I would do something like this:

def list_users(opts \\ []) do
  opts
  |> QueryBuilder.apply_opts(User)
  |> Repo.all()
end

def list_permissions(user, opts) do
  from(p in assoc(user, :permissions))
  |> QueryBuilder.apply_opts(opts)
  |> Repo.all()
end

Notice how even using your user and permission example, our “duplicated” api for listing resources already contains some differences. For example, list/create/update/delete_permissions is going to take the user as an argument, where the generated functions won’t have such scoping. I don’t feel code injection is best here, but there are still ways to offload shared querying by delegating outside, as QueryBuilder in my example that would know how to build an Ecto query given some options.

Changests can be for any source, and I usually go the other way and collocate them close the user input or changes I am modeling. That said, there is nothing stopping you from collocating changeset functions with the schema. It’s a valid option if that suits you, but since my boundaries expose my Schema structs publicly, I like to keep private things like changeset building out of the module.

This is still a good approach, but since Ecto achieved parallel database transactions, this design is no longer an essential part of your testing strategy. I actually find it easier to write and maintain tests that simply hit the database internally as needed than those that go through hoops to manage or stub db calls.

The first part of this sentence is absolutely true – the new context generators will result in more code than throwing a Repo.insert in the controller, but as far as maintenance is concerned seeing the LOC that the new generators build vs the old ones is not a good maintenance indicator. For example, every time you duplicate Repo.insert / Ecto.build_assoc/ etc calls in multiple controllers, channels, other modules, etc, you are placing a future burden on yourself. Compound this over a few years, and it will be a massive maintenance cost. Imagine needing to offload persistence of parts of the system to another store, or another app entirely? Now you have to refactor every part of the system that this “simpler” code touched.

Every decision we make in software is a trade-off, but I feel the current approach maintenance wise is an absolute win, with the potential for slightly more effort up front. We feel that effort is going to be worth it :slight_smile:

9 Likes
65

I wanted to a quick follow up of a possible approach I talked about. Someone who is better at Elixir can probably make this better, but it shows a quick pass at it.

I think the benefit of this is not every model has to test all. That should be something out of the box, and having an opinion about a common way of doing this is ok.

I realize this is maybe like ActiveRecord, but Ecto defines similar methods. The difference is in Phoenix we have to explicitly stitch together our schemas and the Ecto methods we need to call. This would make that all transparent and tested separately.

1 Like
66

I believe the meta-programming suggestion and the original comment about having too many similar functions is missing the point that generator are guiding tools and they are not the end-goal of your design. For 3 resources in the same context, it is very unlikely that all of the functions will look like list_comments/list_posts/lists_likes. Rather a function like list_comments will end-up being list_comments_per_post, because it is unlikely you will want to list all comments in your domain but rather per post.

That’s exactly where your “use Foo.Model” idea breaks off. You will want to customize the generated code more often than not and relying on meta-programmed functions will only add indirection.

Finally I want to echo @chrismccord in that we have extensively debated all of those topics, such as “should the changeset functions belong to the schema” and “will there be too many functions in the context”. We agree the jury is still out there but I believe the best way to answer those questions is by practicing them on long term applications. We have already been doing so and it ended-up that many of the concerns we had did not actually manifest with time. We will continue slightly refining the generators based on feedback.

6 Likes
67

New to generators, when running:

mix phx.gen.html Main Test tests proxy_id:references:main_proxies

Is it expected behavior that “references” doesn’t create html fields?


/test/show.html.eex

<h2>Show Test</h2>

<ul>

</ul>

<span><%= link "Edit", to: test_path(@conn, :edit, @test) %></span>
<span><%= link "Back", to: test_path(@conn, :index) %></span>
2 Likes
68

Yes, it is. Setting up references automatically can lead to wrong data being exposed or modified.

4 Likes
69

@josevalim Is authentication/ authorization an application or interface responsibility? In my opinion it would make much more sense to build it into the application’s api, as the app would then be self-contained and provide consistent functionality across interfaces.

{:ok, token} = App.Auth.login(email, password)
{:ok, post} = App.Blog.create_post(token, attrs)
{:error, :unauthorized} = App.Admin.destroy_application(token)
App.Auth.logout(token)
{:error, :unauthenticated} = App.Blog.create_post(token, attrs)

## vs. ##

plug :ensure_authenticated
plug :ensure_authorized
{:ok, post} = App.Blog.create_post(user_id, attrs)

Also the approach that contexts shouldn’t know about other resources surpresses the use of Elixir’s pattern matching capabilities, is this an issue or a feature?

App.Blog.create_post(user_id, attrs)

## vs ##

App.Blog.create_post(%App.Auth.User{}, attrs)

Edit: Better example

App.Fleet.create_car_for_employee(employee_id, attrs)
App.Fleet.create_car_for_manager(manager_id, attrs)

## vs ##

App.Fleet.create_car(%App.HR.Employee{}, attrs)
App.Fleet.create_car(%App.HR.Manager{}, attrs)
2 Likes
70

Design it in your app and see what feels better. We are not going to arrive to conclusions based on pseudo-code. :slight_smile:

A context can know about resources in other contexts, as long as those resources are explicitly returned by the context (i.e. as long as you respect the code boundaries).

5 Likes
72

Where would common “models” go? As an example, address model is the same and is shared by user, company, employee. What are the suggested alternatives for such a use case with the new paradigm? Love the new paradigm and thanks everyone on the elixir/phoenix team that help guide the community towards better ways. One way I can think of is to push it into a “communications” context and use it that way. Any other ideas that are better are welcome.

Also, where do models that cut across domains fit? Example, accounts has the user model (if I may still call it that). A company model lives somewhere. A user can belong to a company with a certain role. If we assume company is a different context (and not the accounts context), then role model (a users role within a company) fits where? - maybe in a new context called “user_roles”?

2 Likes
73

It is the same as in any other Elixir project. If you need to have functions or data structures that are shared across multiple contexts, you can define it at lib/my_app/foo.ex or lib/my_app/shared/foo.ex and just have everyone depend on it.

Btw, Phoenix does not have models from v1.3 and Ecto does not have models for a long while already. You probably meant to say schemas but remember schemas are simply data structures with type information attached to each field.

2 Likes
74

Nice work with this!

A little idea for non-umbrella apps, what if the web folder was moved up one level to be placed directly into lib and renamed into myapp_web. That would bring it little bit closer to the folder structure of umbrella apps.

lib
└── site
    ├── application.ex
    ├── blog
    │   ├── author.ex
    │   └── blog.ex
    ├── repo.ex
    └── web
        ├── channels
        │   └── user_socket.ex
        ├── controllers
        │   ├── author_controller.ex
        │   └── page_controller.ex


lib
├── site
│   ├── application.ex
│   ├── blog
│   │   ├── author.ex
│   │   └── blog.ex
│   └── repo.ex
└── site_web
    ├── channels
    │   └── user_socket.ex
    ├── controllers
    │   ├── author_controller.ex
    │   └── page_controller.ex
6 Likes
75

Thanks @josevalim

I used the term models because I am still stuck in phoenix 1.2 land :slight_smile:
Thanks for correcting me.

Would it make more sense to use something like “shared” or have some new context - like “communications” for things like address? Or maybe create separate “business_address”, “user_address” and “employee_address” in their respective domains but use macros to bring in the fields from an address module defined in some common location? Just trying to get a few ideas. I know there is no hard and fast rule, so if you have any rules of thumb it would be great to know. Otherwise, thanks again for everything you do.

2 Likes
76

@chrismccord, @josevalim, @wojtekmach and others many thanks for your great work guys, you’re awesome!
Why did you decide to prefix schema and DB table names with context name in generators? Shouldn’t we separate a DB from application logic? If we ever change a context name or structure, we will have to rename all the tables related to this context. One schema probably can be related to multiple contexts. Multiple applications probably can use the same DB tables with different contexts in minds.

4 Likes
77

I had the same question: Phoenix v1.3.0-rc.0 released

I decided to strip the context prefix from the database table name. To me, contexts are limited to the app and shouldn’t bleed into the database.

4 Likes
78

Just a quick update, we’ve recently converted hex.pm to follow v1.3 project structure [1]. There’s still more to be done (using FallbackController, moving some functions around etc) but it should give you an idea how the new structure works in practice. Feedback & PRs are welcome!

[1] https://github.com/hexpm/hexpm

6 Likes
79

Cool idea, I’m not sure if this particular approach was discussed before. It would indeed make it easier to convert a non-umbrella project to an umbrella one down the road
(literally: mv lib/myapp_web apps/myapp_web/lib/ +/- project boilerplate & git commands)

edit: I posted this too hastily, it isn’t any more easy than mv lib/myapp/web apps/myapp_web/lib/myapp_web :stuck_out_tongue: but I still kinda like that it’s less nested.

3 Likes
80

Just wanted to drop my 2 cents for the directory structure.

I think, this depends on the perspective from which you’re discovering the project.

If you’re browsing through the directories (eg. in your editor’s navigation panel), then I guest it’s quite easy to understand that the lib/accounts/accounts.ex is the main entry point (because the file is named the same way as the directory).

On the other hand, if your starting point of exploration is the source code, eg. exception message like

** (UndefinedFunctionError) function Accounts.create_transaction/0 is undefined or private.

you might start looking for file lib/accounts.ex, which doesn’t exist.

I guess, for shallow directory structure (like the one presented here) this might not be too much of a problem, but is this something that will scale well if it gets more nested? (PS. I also accept, that this might be a smell that should make one think about refactoring, thou).

Bottom line is - both Accounts.Foo and Accounts modules are defined in lib/accounts/ - in this context, module name builds quite valuable expectation, which unmet, might cause confusion.

I think, convention, is something that will help you in longer term. Not only for people maintaining projects, but also newcomers starting their journey with Elixir/Phoenix.

3 Likes
81

Web.Plugs is still defined in web/plugs.ex (as it should be).

I don’t get where this change came from, really. It’s not as if it changes anything, but the namespaces and file structure had a one to one mapping before and now they suddenly don’t. For what reason, exactly?

2 Likes