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. (GitHub - wojtekmach/acme_bank: An example ☂ project · GitHub)
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.
Many functions
These files are going to become unwieldy very quickly: 3 models is 21 functions and that’s just getting started.
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.
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.
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
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.
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.
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>
@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.
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)
Design it in your app and see what feels better. We are not going to arrive to conclusions based on pseudo-code.
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).
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”?
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.
