Complex traits

[NoahTheDuke]

Problem statement

I would like to reference other traits and set field data conditionally within traits.

Background / Examples

In another fixture-replacement library factory_bot, traits are defined using Ruby blocks, which are basically anonymous functions/callbacks. This allows for a given trait to perform more complex logic than just column value substitution. Here are some examples.

A trait can reference another trait, allowing for deduplication:

factory :order do
  trait :completed do
    completed_at { 3.days.ago }
  end

  trait :refunded do
    completed
    refunded_at { 1.day.ago }
  end
end

A trait can set multiple attributes based on random data:

factory :order do
  trait :chaos do
    if rand(1) == 0
      status { :in_progress }
      completed_at { nil }
    else
      status { :complete }
      completed_at { 1.day.ago }
    end
  end
end

In factory_bot, they can also hold before/after handlers, but that's outside of the scope of this issue.

Potential solutions

Base idea is that trait definitions could take a function that returns a map instead of a map literal. Building on that are a number of other solutions:

• Referencing other traits could be a "lazy reference", so it's not evaluated until the factory produces an instance. Not sure how to apply it without state.
• There could be a helper function that accepts and returns a map (to be used as the return value of the trait function), but has overloads for referencing other traits or performing other actions.
• The trait function itself could be a 1-parameter function that provides the factory map, allowing for threading changes to it directly (instead of relying on merging in values after). Along with helper functions, this could allow for referencing traits as well ((apply-trait factory-map ::trait-name)).

I'm willing and interested in helping build this functionality, if y'all are interested!

[plexus]

I definitely agree we want something like this. Currently we expect a trait to be a map, namely a template map that gets merged in. But how do you provide extra (dependent) traits? I actually had this same issue with regular factories. A first iteration you just passed any extra values directly

(defactory user
  {:name "Arne" :id #(rand-int 100)}
  :traits
  {:deleted
   {:deleted-at #(java.util.Date.)}})

(user {:last-name "Brasseur"}) ;; where to put the traits?
;; solution: extra wrapping map
(user {:with {:last-name "Brasseur"} :traits [:deleted]}) ;; where to put the traits?

We could do something similar here

(defactory order
  {}
  :traits
  {:completed
   {:with {:completed-at #(java.util.Date.)}}

   :refunded
   {:traits [:completed]
    :with {:refunded-at #(java.util.Date.)}}})

This would also allow us to put per-trait hooks, something I've been wanting to add anyway. I believe factory-bot also has after-build and after-create (after persisted in the DB)

(defactory order
  {}

  :after-build
  (fn [...] ...)

  :after-create
  (fn [...] ...)

  :traits
  {:completed
   {:with {:completed-at #(java.util.Date.)}
    :after-build (fn [...])}

   :refunded
   {:traits [:completed]
    :with {:refunded-at #(java.util.Date.)}
    :after-build (fn [...])}})

Downside is the extra wrapping map. An alternative approach is that you recognize special keys in the top-level map (e.g. :facai.factory/traits and the rest gets merged in, but I'd like to avoid that kind of special casing for Facai.

Third alternative is more what you suggest, treat a map as we treat it now, treat a function as a hook, and anything fancy like adding traits has to be done through code inside the hook.

In all of these cases there's an open question of what the semantics are of the function. The simplest is that it's simply value-in value-out, you get a partially built map, and return some updated version.

Internally there's a context map that gets passed around throughout the building process, which lets you see for instance where in the hierarchy you currently are. I think we should make that available to the hook functions, but maybe this can be optional by providing a different hook type.

:after-build (fn [m]
               (assoc m ...))
:after-build-ctx (fn [ctx]
                   (update ctx :facai.build/value assoc ...))

[NoahTheDuke]

Thanks for the detailed reply!

I seem to have missed that you were already using :with as a method to embed the overrides and traits; given that, I think your suggestion of moving traits to use that as well is a smart one. Additional layers are annoying but I think the consistency provided would be a real boon to factory authors who will only have to learn one rule: "when overriding, wrap in a :with map". I agree with you that a special key is a bad idea. You shouldn't assume anything about a user's data.

You're correct that factory_bot has after-build and after-create. (They have even more than that, see below.) Adding them would be as "simple" as threading in the context. Instead of having two separate functions, I'd suggest putting the partially built map onto the context before passing it in (like I think you already have?). It's only 1 level of indirection and gives you leeway in the future to make changes.

Example with simple indirection:

(defactory order
  {:total-cost nil
   :amount 0}
  :after-build
  (fn [ctx]
    (let [amount (-> ctx :facai.build/value :amount)]
      (assoc-in ctx [:facai.build/value :total-cost] (if (< amount 10) 0 100)))))
...
(f/build-val order)
; => {:total-cost 0 :amount 0}
(f/build-val order {:with {:amount 20}})
; => {:total-cost 100 :amount 20}

For example, that leeway would allow you to add transient attributes (attributes that only exist during the build process but that don't end up on the returned instance) without changing the semantics of your callbacks:

(defactory order
  {:total-cost nil}
  :transients {:amount 0}
  :after-build
  (fn [ctx]
    (let [amount (-> ctx :facai.build/transients :amount)]
      (assoc-in ctx [:facai.build/value :total-cost] (if (< amount 10) 0 100)))))
...
(f/build-val order)
; => {:total-cost 0}
(f/build-val order {:with {:amount 20}})
; => {:total-cost 100}

It's possible to achieve this by, for example, passing in two arguments (ctx and instance), but that's less obvious and more cumbersome.

One thing to start thinking about is, what is the order that you apply all of these things in? The base factory, the overrides at the call site, the overrides in traits, the callbacks, the callbacks inside of traits. Might be worth figuring that out now before people start relying on it one way or the other.

Note: They actually allow for building your own hooks as well, because they use the strategy pattern as the basis for their functions build and create (and attributes_for and build_stubbed which Clojure doesn't need). This allows for writing to_json-style custom strategies.

[plexus]

That all makes sense, I think I need to dive into the factory bot sources a bit for inspiration. Regarding execution order, Intuitively I would say

• Traits are meant to override/augment the base template, so they get merged on top
• Explicit :with at the top level is most specific, so overrides values from traits or base template ("always wins")
• A top-level :after-build is executed after the complete value is built, including any :with and :traits
• If a trait relies on another trait, then the dependent trait should be executed (merged/hooked) first
• For the rest traits are executed in the order of the :traits [...] option

This does mean some smart (or "reverse") merging, e.g. if we have

(defactory foo
  {:bar 1}

  :traits
  {:some-trait
   {:with {:bar 2}
    :after-build (fn [ctx]
                   (update-in ctx :facai.build/value inc))}})

(foo {:with {:bar 3}
      :traits [:some-trait])

The result should be {:bar 4}. So the :with {:bar 3} overrides the :with {:bar 2} from the trait, but the :after-build hook does already "see" the 3. Does that make sense. So the process will look something like this

• start from base template
• apply :with
• for each trait
  • apply :with, but exclude keys already set via passed-in :with
  • apply hook
• top level hook

[NoahTheDuke]

That looks good. My only suggestion would be to move apply :with to after traits. That way, calling (f/build-fn foo {:with {:bar 100} :traits [:trait-that-sets-bar-to-55]}) will return a foo with bar 100. Otherwise, might be hard to enforce returning an object that uses traits but still sets a specific value unconditionally.

[plexus]

That's the case that I tried to explain above, if :trait-that-sets-bar-to-55 has a hook, should that hook see a value with :bar 55, or with :bar 100? I would assume the latter. Otherwise your transient example also doesn't really work.

[NoahTheDuke]

Oops, I completely misread your example and got it backwards hah. Sorry about that. You're correct, the hook should see :bar 100.

[plexus]

I've partially implemented this in #3, main things missing:

• traits depending on other traits
• after-create hooks

I'm going to punt on the after create hooks, I think for that we first need to revisit how create works, currently these are separate implementation for datomic/jdbc. It's partially pluggable but there's still too much shared boilerplate, we'll have to employ some kind of strategy pattern (as factory_bot does). That said I tried to make sense of the factory_bot source code and it's this typical OO code base where almost nothing happens in any single file, I find that so hard to reason about now... but we'll come up with something.

Regarding traits depending on traits, that's relatively easy to do but to your earlier point we need to define what order makes sense.

I think what makes sense is:

• process each trait in order they are requested at the top level :traits [:first-this-one :then-this-one]
• if a trait depends on other traits, then they are handled before that trait

(defactory foo
 {}
 :traits
 {:foo {}
  :bar {}
  :baz {:traits [:bar]}})

(foo {:traits [:foo :baz]}) ;;=> processes [:foo :bar :baz]

What if a trait is pulled in twice?

• I would say: same rule as above, but if a trait has already been processed it isn't processed again

(foo {:traits [:bar :foo :baz]}) ;; does [:bar :foo :baz], not [:bar :foo :foo :baz]

In all of these cases processing actually happens in two passes, first we merge all the :with clauses (factory, traits, option), generate the data, then handle the hooks.