Subdomains unexpected boundary violation
aspett opened this issue ยท 13 comments
๐ Howdy. I'm experiencing an issue with subdomains.
Below is a contrived example - I am actually having this issue in a larger codebase and wanted to reproduce it on a smaller scale.
If I have these boundaries:
Blah
(deps: [], exports: :all)Blah.Context
(deps: [], exports: :all)
BlahWeb
(deps: [Blah], exports: :all)
Then I have a file Blah.Context.Subcontext.Module
with a call/0
Compiling gives a forbidden reference error. This is unexpected to me, since Blah
exports :all
and Blah.Context
exports all.
What I've tried
- โ Using
exports: [{Subcontext, []}]
syntax does not help. - โ
If I specify in
Blah.Context
the exact modules to export (exports: [Subcontext.Module]
), the violation disappears. - โ Adding a boundary on
Blah.Context.Subcontext
and exporting all does not help.- โ If I export
Module
inSubcontext
, the violation is still present which surprises me since exportingSubcontext.Module
fromContext
did work.
- โ If I export
I've demonstrated this in this repo where if you compile on main
, you'll see the violation.
warning: forbidden reference to Blah.Context.Subcontext.Module
(references from BlahWeb to Blah.Context are not allowed)
lib/blah_web/controllers/page_html/home.html.heex:1
Any thoughts / help would be greatly appreciated ๐. I'm not sure if I've just missed how this case should be idiomatically handled, or if this is a bug, or not a feature at all.
Thank you
In this particular case, I am adding Boundary to an existing application where we'll iterate away some dirty xrefs. Unfortunately it means I can't promote this particular subboundary to top level because that'll introduce a circular dependency - so seems with the current boundary implementation, it's necessary to explicitly export these modules for aunt/uncle dependents.
Thanks for the report. Currently, exports: :all
doesn't include sub-boundaries. I don't think supporting this would be very hard, but I'm not sure if this is something we want to do.
TBH, exports; :all
should IMO mostly be avoided. It was originally introduced for the purposes of Ecto schemas, which are the modules you usually want to export, and there's typically a lot of them, so listing them all explicitly is tedious. But otherwise I'd mostly avoid this approach because it effectively removes the private modules features. Everything in the boundary becomes public.
If you have a mixture of public and private modules (which I presume is the case, given that you also have sub-boundaries), then I think explicit exports are a better approach.
So my first question is what's wrong with explicitly listing exports? Are there too many of them?
In this use case, there are many subcontexts, and also many modules in this particular Subcontext
which make sense to be exported as part of a migration towards cleaner boundaries. I had actually discovered this issue when trying to export like,
use Boundary,
deps: [...],
export: [
# ...,
{Subcontext, []}
]
There are quite a few modules, but not so many that it's unfeasible to list them explicitly ๐
I agree that exports: :all
is ideally avoided; this is a struggle of migrating an existing codebase only.
Perhaps I could look into implementing this as an option, but not default?
Adding our use case here as we also have hit this limitation in the past.
In our case, we have a main boundary in a module that defines the main API.
This module then exposes 2 other modules that define the interfaces (inputs/outputs) for this API.
So we end up with something like:
defmodule API do
use Boundary
end
defmodule API.Inputs do
alias API.Inputs.<some sub module>
...
end
defmodule API.Outputs do
alias API.Outputs.<some sub module>
....
end
To export Outputs/Inputs, we tried to use mass exports in the top API
boundary but that didn't work so we have to either:
a) explicitly list each of the Output/Input's sub-modules in the main boundary
b) make Output/Input into top-level boundaries
Both a) and b) work, but for a) we lose the benefit of having a top-level module name that "encapsulates" the public interfaces and, for b), the module that depends on this boundary needs to explicitly depend on these sub-boundaries.
I had actually discovered this issue when trying to export like,
use Boundary, deps: [...], export: [ # ..., {Subcontext, []} ]
@aspett What didn't work with this? This is supposed to allow you to bubble up all the exports of a sub-boundary.
I agree that
exports: :all
is ideally avoided; this is a struggle of migrating an existing codebase only.
That's true. I actually use it myself in one place in a large codebase I'm moving to boundary.
Perhaps I could look into implementing this as an option, but not default?
I mean I'm open to this, clearly there's a need. But I'm also worried that if we make an ad hoc choice now, we'll regret it soon. So I'm still trying to grasp the problem, and I'd like us to devise a solid solution.
I understand that you're working on an existing codebase, and so you need to cut some corners. But this begs the question if introducing nested boundaries is a good choice at this point? Maybe instead just configure level 1 boundaries and make that work, and then refine deeper gradually? I feel I don't understand enough about your boundary strategy to make a choice here.
@AfonsoTsukamoto I don't quite understand this. Are Input/Output sub-boundaries? What is it you're trying to export that doesn't work?
I had actually discovered this issue when trying to export like,
use Boundary, deps: [...], export: [ # ..., {Subcontext, []} ]
@aspett What didn't work with this? This is supposed to allow you to bubble up all the exports of a sub-boundary.
The same scenario as the issue described; if Blah
exports [{Context, []}]
and Blah.Context
exports [{Subcontext, []}]
and BlahWeb
depends on Blah
, then I'd hoped that it would not violate boundaries by calling Blah.Context.Subcontext.Module.call/0
from BlahWeb
. (This is not the case, it does result in a violation)
So I'm still trying to grasp the problem, and I'd like us to devise a solid solution.
I understand that you're working on an existing codebase, and so you need to cut some corners. But this begs the question if introducing nested boundaries is a good choice at this point? Maybe instead just configure level 1 boundaries and make that work, and then refine deeper gradually? I feel I don't understand enough about your boundary strategy to make a choice here.
There are two components here that I think will help convey the strategy. The details are fudged, but same idea:
-
We want contexts to talk to each other via clearly documented APIs. Currently contexts own several standardised namespaces - e.g. imagine commands, queries, workers, etc. Those currently can be called from anywhere, despite many of those modules being "internal". The idea for migration is that each context will instead begin to expose it's public interface on the context module (
Application.Context1
), but there will also be some additional modules (domain records) that will be public, e.g.- Application.Context1
- Application.Context1.Public
- Application.Context1.Public.DomainObject1
- Application.Context1.Public
- Application.Context1
-
Fully migrating to a model like that will be a slow burn, so the idea is to set up each context to export the namespaces we actually want (i.e.
Public
) and then list everything else asdirty_xrefs
and that list can be driven down over time.
Hopefully that makes sense ๐. So my contrived example in the original comment actually reflects trying to expose the Public
namespace of contexts so that DomainObject
can be depended upon between contexts, but also in other top level boundaries (and in their children).
We can do this by explicitly listing modules, but more ideal is to use notation like exports: [{Public, []}]
@AfonsoTsukamoto I don't quite understand this. Are Input/Output sub-boundaries? What is it you're trying to export that doesn't work?
Yes, they are sub-boundaries - I think it is easier to see the issue when scoped in the app so let me update the example.
So, taking the same example but with the top level fixed:
defmodule MyApp do
# expectation: MyApp mass exports API
use Boundary, top_level?: true, exports: [{API, {}}]
end
defmodule MyApp.API do
# expectation: API mass exports Inputs/Outputs
use Boundary, exports: [{Inputs, []}, {Outputs, []}]
def some_fn, do: :some_fn
end
defmodule MyApp.API.Inputs do
def some_fn, do: :some_fn
end
defmodule MyApp.API.Outputs do
def some_fn, do: :some_fn
end
If I then add a dependent module, I get:
defmodule MyAppWeb do
use Boundary, top_level?: true, deps: [MyApp]
alias MyApp.API
def test do
API.some_fn
API.Inputs.some_fn # forbidden reference to MyApp.API.Inputs
end
end
In this case, looking from the top level down, I would expect the boundary to:
- create a boundary on
MyApp
level that also exportsMyApp.API
and all its sub-modules MyApp.API
redefines the boundary include onlyMyApp.API.Inputs/Outpus
and their sub-modules as sub-modules/boundaries
making the final MyApp
boundary something like:
{MyApp, MyApp.API, MyApp.API.Inputs, MyApp.API.Outputs, MyApp.API.Inputs.*, MyApp.API.Outputs.*}
but the final result, at least from a dependent module call, is {MyApp, MyApp.API}
Thank you both. This is definitely contrasting the documentation (https://hexdocs.pm/boundary/Boundary.html#module-exporting-from-sub-boundaries).
On one hand, I'm not a fan of exporting something from a sub-sub-boundary. Not gonna lie, this seem smelly to me. But I also see how the current behaviour seems weird. So I'm also reconsidering caving in, and including sub-boundary exports into :all
. The advantage here is that the model is simple to explain. We can basically say that sub-boundary public modules are also treated as the members of the parent boundary. This is already the case for the root module, so now we'd just expand this definition.
Another choice is to fix the mass export, and/or maybe even add the special option, so you can say exports: [{Foo, as: :boundary}]
.
I need to think about this a bit. I probably won't find much time this week though, as I'm away on a conference.
Thank you for taking the time to look into this @sasa1977 ๐
I agree that it might come as smelly but also that the model becomes easier to explain - in our case, we assumed this is how it worked already.
I'm at the beginning of my Elixir journey but would be happy to help with the implementation of this once you have a decision ๐