ActiveRecord integration design
geekq opened this issue · 6 comments
The goals are:
- to support different ActiveRecord versions at the same time
- to enable further development without regressions (without breaking applications using different versions of ActiveRecord)
- convenience
For that we need
- to have clear responsibilities for
workflowandworkflow-activerecord(separation of concerns) - single direction for dependencies
Plan is: workflow would contain changes to DSL defining the states and events, and workflow-activerecord would depend on particular ActiveRecord API
+------------------+ +-----------------------+
| workflow <-----+ workflow-activerecord |
+------------------+ +------+----------------+
|
|
+------------------+ |
| ActiveRecord <------------+
+------------------+
We have some design options to choose from. Once chosen, it would be hard to change without breaking things. I've highlighted my current preference in bold.
Versioning workflow-activerecord gems
Imagine, Rails 6.0 brings some incompatible API changes. Think something like protected_attributes ;-) In this case workflow-activerecord for 4.1, 4.2, 5.0, 5.1, 5.2 would be based on the same code base, while workflow-activerecord for ActiveRecord 6.0 would require different implementation.
Option 1. Release workflow-activerecord version only when it's code base changes.
Easy to build/release. Harder to reference the right version. If your application is for Rails 5.2, use
gem 'workflow-activerecord', '>= 4.1', '< 6.0'
Option 2. Release workflow-activerecord version for every ActiveRecord version.
Makes easy for users to reference the right version. If your application is for Rails 5.2, use
gem 'workflow-activerecord', '~> 5.2'
But more work to release. Multiple versions, that are binary equal.
Referencing Gems
Option 1. workflow-activerecord defines dependency on workflow itself so user
just needs a single line:
gem 'workflow-activerecord', '>= 4.2', '< 6.0'
But which version of workflow should workflow-activerecord depend on? Latest? Some particular?
Consider this article about the differences in dependency versions for libraries vs. applications. https://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/
Option 2. User to write both dependencies in her Gemfile
gem 'workflow', '~> 2.0'
gem 'workflow-activerecord', '>= 4.2', '< 6.0'
require
In Rails you do not need to write require nowadays since it uses require 'bundler/setup' and always requires everything?
include
Option 1. Multiple includes
class Article < ApplicationRecord
include Workflow
include WorkflowActiverecord
workflow do
# list states and transitions here
end
Option 2. Single include Workflow
When used in conjunction with workflow-activerecord and used inside a class inheriting from
ApplicationRecord, modify the behaviour of include Workflow to also activate the persistence.
But this is probably too much magic
Option 3. Single include WorkflowActiverecord
Probably the best trade-off the explicitness and convinience.
@voltechs What do you think?
@geekq Looks good. I have two comments/suggestions.
Dependency Management
I'd encourage leveraging the built-in dependency management that bundler goes through. What you'll get from this is that workflow-activerecord will have a runtime dependency on activerecord, with a range of versions that it is known to support:
spec.add_runtime_dependency :activerecord, [">= 4.2", "< 6.0"]
You can't really support activerecord 6.0 until it's been released, so this is reasonable. You'll also specify a dependency to workflow, which will likely be semantic:
spec.add_runtime_dependency :workflow, '~> 2.0'
Now users who wish to use workflow with the activerecord adapter, they simply need to require one gem. The gem (workflow-activerecord) based on it's dependencies will automatically resolve to the correct (usually latest, unless locked) version for the user.
This also eliminates the need for the user to specify—redundantly—both gem 'workflow' and gem 'workflow-activerecord' in their Gemfile.
A user will have already specified gem 'rails', '~> x.x' in their Gemfile, and that will act as the limiting factor around which other gems jostle to find an appropriate version.
Let me know if I did a poor job of explaining that.
Activation
Given the incredibly simple entry-point for workflow, I recommend sending a :prepend or :include to ActiveRecord::Base upon ::ActiveSupport.on_load(:active_record). Users can immediately then use workflow do within their models.
P.S. I recommend snagging workflow-activerecord as soon as possible. Just throw up a 0.0.0 blank gem for now.
Gem Dependency Management
I've created a sample application https://github.com/geekq/workflow-rails-sample
which I use in an empty rbenv to test dependency management with bundler.
Gem/bundler patterns and prerelease support work quite well! I was hesitating
a bit to reference workflow gem from workflow-activerecord since referencing an exact
version of a lib from another lib is not recommended. Referencing without version (latest?
any already installed version?) could potentially lead to an incompatible lib pair.
But following ranges feel OK.
Now I am happy with:
-
gem 'workflow-activerecord', '>= 4.1pre', '< 6.0'
And just in case, the developer of an application can still put 2 gem lines and specify exact versions of both libraries. In addition Gemfile.lock ensures a repeatable build.
Gems released as:
- https://rubygems.org/gems/workflow-activerecord/versions/4.1.0.pre
- https://rubygems.org/gems/workflow/versions/2.0.0.pre
Next step: Activation. Currently requires 2 includes, but will try out your suggestion on Thursday @voltechs
I am hesitating with a magic workflow activation for every model @voltechs
since it would put some unneeded functionality to every model class
because including WorkflowActiverecord creates multiple methods, e.g. for the workflow spec and for persistency
The Scopes extension would also need to be adjusted since workflow_spec would be nil.
At least, I've just reduces number of includes from 2 to 1 742b124
I totally understand that perspective. I have a couple tricks that I use to keep it relatively clean.
Essentially what I do is I include one method onto ActiveRecord::Base which when called, injects into that class the rest of the extension. That way you're only "polluting" the base class with one method. I find this to be a healthy compromise. One could argue that it's a grey area for determining where to draw the line on composition vs inheritance. If you have a class that doesn't use any primary keys, why is primary key functionality in the base class? Why not just include that in all the classes you want a primary key in? I'm sure the line has something to do with majority use-cases, but it's not worth debating in this forum (maybe over beers 😛)
Anyhow, looks like you're making good progress! Looking forward to 2.0.0!
module Core
# all your goodies
end
module Base
def self.workflow(*args, &block)
self.prepend(Workflow::Core) unless ancestors.include?(Workflow::Core)
Workflow::Core.workflow(*args, &block)
end
endIn the real use case, out of, say 10 active model files, a one or two are going to use workflow.
So we are talking about saving 1-2 lines of code by applying magic to all models.
(if workflow-spec-defined-on-this-model then include_persistency)
I would stick to include WorkflowActiverecord for now and concentrate on finishing refactoring, separating READMEs and getting the first workflow 2.0 release and complementing workflow-activerecord release ready.
Please have a look at
https://rubygems.org/gems/workflow-activerecord/versions/4.1.2 (requires https://rubygems.org/gems/workflow/versions/2.0.0) and try it out!
Will stick with include WorkflowActiverecord