search

metabase / toucan

A classy high-level Clojure library for defining application models and retrieving them from a DB
Eclipse Public License 1.0
570 stars 49 forks source link

Scanning for hydration keys does a keyword call on every var #55

Closed plexus closed 5 years ago

plexus commented 5 years ago

The require-model-namespaces-and-find-hydration-keys function loops over every var in every namespace to check if it's a model. To do this it calls :toucan.models/model (via toucan.models/model?) on the var's value.

(:toucan.models/model val)

This is problematic since that value could be anything, and calling it like this with a keyword could have all kinds of side effects.

To make this concrete, say Datomic is present and loaded in the same process as Toucan, then it will call

(toucan.models/model? datomic.pull/normalized-pattern-cache)

Which raises an exception, making Toucan unusable.

@camsaul I'm not sure what to propose as the best solution here, in my project I've monkey-patched require-model-namespaces-and-find-hydration-fns to call (and (record? model) (models/model? model)). Would that be acceptable?

camsaul commented 5 years ago

I think model? could pretty easily be changed to first check whether the object is a record (with record? and then see whether it contains that key. Alternatively, model? could be implemented as a multimethod or protocol whose default implementation returns false for everything that's not a model, and Toucan model classes can get an implementation that returns true.

If you'd like to open a PR I'd be happy to merge it