I have a Class struct which contains a set of Topics. I am
trying to treat Class as an Aggregate root in DDD parlance. Which
means all updates to sub-entities of Class (Topics in this case)
have to be managed by the Class. This is to maintain invariants within
the Class and provide transactional consistency within Class and its
sub entities.
Here is an outline of what I have:
def changeset(class, attrs) do
class
|> cast(attrs, [:name,...])
|> cast_assoc(:class_topics)
|> validate_required([:name, ...])
end
def create_topic(%Class{} = class, attrs \\ %{}) do
number = length(class.topics) + 1
new_topic =
class.topics
|> Enum.concat([Map.put(attrs, "number", number)])
result = class
|> Class.changeset(%{topics: new_topic})
|> Repo.update()
end
This complains about cast_assoc requiring Maps (whereas in this case
Topic structs are supplied)
So I updated it to this:
def create_topic(%Class{} = class, attrs \\ %{}) do
number = length(class.topics) + 1
new_topic =
class.topics
|> Enum.map(fn i -> %{id: i.id} end)
|> Enum.concat([Map.put(attrs, "number", number)])
result = class
|> Class.changeset(%{topics: new_topic})
|> Repo.update()
end
Which also doesn’t achieve what I want.
I am doing it like this as there are additional constraints on the topics
being created - which prevents me from just creating a topic without first
preloading and validating against existing topics. (For example, I want
to ensure that there is at most 1 Topic which has a particular category
in a Class - which requires me to have them all available to validate this
when creating or updating a topic.)
I think there is something obvious I am not understanding - so I thought
I’d post the question here.
The code seems to be trying too hard to literally reflect the design concept you have in mind, by literally updating the Class db entry in order to create a new Topic row. Storage != responsibility.
It is entirely sensible to have create_topic in the Class module (I often do this myself as well for exclusively-owned associated data, for similar reasons as you are doing here), but trying to mimic this with storage layer interaction is a step too far imho.
Just do the validation in create_topic/2 and then … well … create a new Topic row in the db using Topic.changeset/2 and Repo.insert. Easy-peasy, and you’ll not need to work around Ecto so much, let alone have those Enum.map/2/Enum.concat/2 chains.
Thanks @aseigo - I think I understand what you are saying.
So to do that I would need to read/preload the topics, do the validation/calculate derived data and create/update the new Topic record all inside of a transaction. Holding a lock on the Class row to ensure that there is not another concurrent update to the list of Topics associated with the class - which could break my desired invariants.
You would need to do such a lock anyways, since as it is written now you could end up in the same situation. create_topic could be called from multiple processes as it is!
But … this is what databases are for: ACID. I would personally put such a constraint check in the database itself, either as a constraint on the relevant fields, or as a function used as a constraint trigger. Putting constraints that require global consistency in the application layer is rarely worth it. This would also obviate the need to fetch the current topics (with a lock, no less): the topic number could be set and the constraints checked in the database itself, and it would be done with guaranteed consistency. So, better guarantees and fewer roundtrips to the database. Constraint violations are returned as errors from Ecto, so you would be able to detect the violations in the application code still.
