hapipal / schwifty

A model layer for hapi integrating Objection ORM
MIT License
73 stars 10 forks source link

Namespaces, sandboxing, and optional knex binding #88

Closed devinivy closed 4 years ago

devinivy commented 4 years ago

This PR implements namespacing and sandboxing (resolves #84) and opting out of automatic knex binding during server initialization (resolves #85).

The below is partially adapted from the description of the analogous features in https://github.com/hapipal/schmervice/pull/10.

Namespaces are an extension to the ability to pass true to server.models(true) in order to access all models (from the perspective of the root server). Now you may also pass a plugin name as a string server.models('my-plugin') in order to access models from the perspective of that plugin. In other words, calling server.models() inside my-plugin is the same as calling server.models('my-plugin') from any plugin; identically calling server.models() on the root server is the same as calling server.models(true) from any plugin.

Sandboxing is a mechanism to opt-out of models being available to all ancestor plugins/namespaces of the plugin that registered the model. This essentially is the schwifty-y way of having models that are "private" within the plugin that registered them. This works by setting the Schwifty.sandbox symbol to true on a model or knex instance.

These features work together to enable better plugin encapsulation and greater testability, with the ability to not only access, but also mock-out models on a per-plugin basis. These features also apply to knex instances accessed with server.knex().

Sandboxing adds some complexity to the base schwifty Model class in order to fully support sandboxing when there are multiple models that have the same name. Objection has an internal model caching mechanism as a critical performance optimization to bindKnex() and bindTransaction(). Models are cached internally to objection per knex instance by their name and tableName. The custom implementation of static uniqueTag() allows us to control this caching behavior. It allows us to support abstractions where one plugin might use a sandboxed model and another plugin might extend that model with its own methods, fields, etc. Both are totally useful, valid modelsβ€” one is simply more specific!

The ability to opt-out of automatic knex binding during server start is an important escape hatch for applications that have more custom needs. For example, it may be useful when some models are multi-tenant and need to be bound to a connection lazily, e.g. per request or per query. This works by setting the Schwifty.bindKnex symbol to false on a model.

zemccartney commented 4 years ago

How will the counter state (for uniqueTag-ing) and the new symbols play out if an application happens to be using multiple versions of schwifty (thinking down the road, when there are multiple versions that include this update)? I'm not sure I'm thinking at all clearly about this (lol, my bad if not! πŸ˜› ), but I think wondering if we could possibly end up with different models sharing the same ids or sandboxed models appearing outside their plugins.

I have a shaky (at best) understanding of Symbols, so could be loudly announcing my ignorance 🦐 And assuming this is even a concern, feels firmly like a problem for future pals, not today.

ANYWAY, a random thought that occurred reading through this. I don't fully grasp all the implications of this work, but it feels big, like a turning point in how we pal, really expanding how we integrate and make use of plugins. Thanks for wading through this work and sharing so much background on it πŸ™ 🍻 :pal:

devinivy commented 4 years ago

Hey, that is definitely a valid question. I would definitely recommend using just one version of schwifty in a given deployment.

Crucial background: npm will deduplicate compatible version ranges across all dependencies and hoist it up to the nearest node_modules folder that allows common access to the given module. But if your npm ls schwifty turns-up multiple installations, you could already have a problem. Schwifty is designed to be registered in any plugin that wants to use it, but that means that the first plugin that registers schwifty determines the version of schwifty that is "fully registered" to the server (see how schwifty utilizes the multiple plugin attribute). So if you start using this upcoming version of schwifty but an earlier version is registered first, then you already have no support for sandboxing and namespacing.

While this should be rare thanks to npm's behavior, hapi and schwifty do give us the tools to detect when such a situation is happening (see server.registrations), so perhaps we could investigate mitigating that. Schmervice is the exact same story. This work brings us into parity with schmervice, which is the primary goal here. But I am totally into thinking through the best way of messaging to the user that there could be an issue related to multiple versions of schmervice/schwifty appearing in a single deployment. If and when we figure that out, we should aim to apply the changes to schmervice and schwifty at the same time.

On a related note, I don't love that the sandbox and bindKnex settings carry through to extensions of models that have them, but because schmervice behaves that way I am going to leave it as-is for now. Both plugins will then be in parity, and later a breaking change will be made so that extended services and models do not inherit these settings, which I think always ought to be opt-in.

zemccartney commented 4 years ago

Ooh! Very cool. I don't have the brainspace today to fully process that, but I'll chew on it, come back on the weekend πŸ˜› Lots of tricky stuff at play. Thanks for taking the time to spell all that out! 🍻