Feature proposal: expand-key

[weavejester]

Goal

The goal would be to have a system where multiple components can be bundled together to denote common behavior. For example, it might be useful to group a web server with a database, or a web server with a bunch of default middleware or interceptors.

I've experimented with something like this in Duct, which builds on Integrant. This proposed system would be far more constrained than that of Duct. In Duct, the configuration can be modified by a chain of pure functions. In this system, only merging is permitted.

This follows on from the introduction of the prep-key multimethod, which had similar goals. The functionality of expand-key would be a superset of that of prep-key, and therefore prep-key can be deprecated.

Usage

First we define an expansion for a keyword. I'll use the term "module" to denote keywords with expansions:

(defmethod ig/expand-key :example/module [k v]
  {:example/server   {:port 8080, :db (ig/ref :example/database)}
   :example/database {:uri "sqlite://mem"}})

When this keyword is added to a configuration, the return value will be merged with the configuration:

(ig/expand {:example/module {}})

=> {:example/server   {:port 8080, :db (ig/ref :example/database)}
    :example/database {:uri "sqlite://mem"}}

The default expansion is:

(defmethod ig/expand-key :default [k v]
  {k v})

Therefore we can mix keywords with user-defined expansions, along with those without:

(ig/expand {:example/module {}
            :example/queue  {}})

=> {:example/server   {:port 8080, :db (ig/ref :example/database)}
    :example/database {:uri "sqlite://mem"}
    :example/queue    {}}

Handling conflicts

There is an obvious concern with this approach: what happens when two expansions have conflicting keys? For example:

(ig/expand {:example/module {}
            :example/server {:port 3000}})

What does this expand to? The proposed resolution mechanism is to attempt to merge values, giving precedence to the default expansion where possible, and raising an exception otherwise.

More formally, given two conflicting expansions:

(expand-key k1 _) => {k v1}
(expand-key k2 _} => {k v2}

The values, v1 and v2 are assumed to be maps. If v1 and v2 share no keys in common, the output is:

{k (merge v1 v2)}   ;; no keys in common, so we can merge whichever way around

If the share keys, but k is equal to k1, then v1 takes priority:

{k (merge v2 v1)}   ;; keys in common, but k = k1, so prefer v1

Similarly, the mirror is true. If k is equal to k2, then v2 takes priority:

{k (merge v1 v2)}   ;; keys in common, but k = k2, so prefer v2

If they have keys in common, and neither k1 nor k2 is equal to k, then an error is raised. So in the above case:

(ig/expand {:example/module {}
            :example/server {:port 3000}})

=> {:example/server   {:port 3000, :db (ig/ref :example/database)}
    :example/database {:uri "sqlite://mem"}}

The port defined the example server takes priority over the default in the example module. This should hopefully feel intuitive, and allows a user to always override expanded values.

Suppose instead we had a second module:

(defmethod ig/expand-key :example/module2 [k v]
  {:example/server {:port 3000, :queue (ig/ref :example/queue)}
   :example/queue  {:size 32}})

In the case of two modules there's no obvious precedence as to what the server port should be set to. Therefore two module with conflicting keys will generate an exception:

(ig/expand {:example/module {}
            :example/module2 {}})

=> ExceptionInfo "Conflicting expansion: [:example/server :port]"

This can be resolved by setting the server port explicitly:

(ig/expand {:example/module {}
            :example/module2 {}
            :example/server {:port 3000}})

=> {:example/server   {:port 3000, :db (ig/ref :example/database), :queue (ig/ref :example/queue)}
    :example/database {:uri "sqlite://mem"}
    :example/queue    {:size 32}}

Problematic changes

To make this work, there are two potentially breaking changes:

1. The existing integrant.core/expand function will be changed in purpose
2. The value of a top level key must be a map to be merged

I don't think anyone actually uses integrant.core/expand, so we should be okay to repurpose that, and in the worst case, use a different name.

The second point is more problematic, as while the majority of keys expect maps, I'm sure not all of them do, particularly in the case of constants:

{:example/server-name "alpha"}

We could change this, and set all values to be maps:

{:example/server-name {:value "alpha"}}

Or when expanding, we could have maps be a special case, retaining backward compatibility and existing functionality at the cost of making things a little more complex.

I think it's important to merge values that are maps, otherwise much of the benefits of this system is lost.

[cch1]

FWIW, I've found many use cases for non-map values in the integrant configuration. Some examples:

:aws/region
:datomic/system
:datomic/db-name

These are usually strings, but I have some values that are vectors and some that are booleans.

[weavejester]

Thanks for the feedback. The way I have it working at the moment is that maps are deep merged, and everything else is replaced. This ensures non-map values still work. Any conflicts between keys in the map will raise an error, unless one of the values has the ^:override metadata.