June 15 2010

The Road to Rails 3 - resource_controllerby bdq

There's been lots of discussion recently on spree-user and at the Spree BOF at RailsConf regarding Spree's use of resource_controller (r_c), and our options as we move towards Rails 3. We're confident that our migration to Rails 3 is going to be a great step forward for the platform, as it will greatly simplify how Spree implements extensions due to the fantastic engine and railtie features in Rails 3. Some members of the community have suggested that we use the Rails 3 migration to also remove r_c from Spree. We've seriously considered this and I'd like to explain how we've arrived at our decision to keep resource_controller.

Issue 1: Removing resource_controller will simplify controller code and make it more understandable especially to new developers:
This is probably one of the most compelling reasons to remove r_c as we accept that the learning curve for the controller logic can be a little steeper than a traditional rails application. Some controllers especially the checkouts controller would benefit greatly by removing r_c code as it's not a true restful resource and not a good candidate for r_c use. In the future we plan to ensure all unsuitable controllers will be rewritten to reduce the usage of r_c and improve readability and maintainability, starting with the checkouts_controller.

Issue 2: resource_controller isn't actively maintained and not currently Rails 3 compatible.
While r_c hasn't been getting much attention from it's original author (James Golick) it does have a large network of forks and it still receiving frequent improvements / bug fixes. We've spent a considerable amount of time migrating r_c to Rails 3 and we're happy to say that it's now fully functional and we're also very close to getting all tests passing after some badly needed attention at RailsConf / BohConf (thank you Derek). Please take a look at our fork and patches / comments are welcome.

Issue 3: inherited_resources is a better alternative:
After some review we’ve found that inherited_resources is not missing many of the features that resource_controller offers, it does however implement them in a significantly different way which would add considerable overhead for any migration (as core and all extensions would need to be-written to match). There are two key features provided by resource_controller that differ in inherited_resources:

  • In-action before and after filters that unlike standard Rails filters get executed just before / after the actual event takes place, when the object has already been loaded / built. This allows extensions and core Spree to cleanly and efficiently hook into actions without adding lots of unnecessary database activity that's normally associated with traditional before / after filters. r_c also supports filters based on the result of a particular action (success / failure), and they're also stack-able allowing core and multiple extensions to add multiple callbacks to a single filter.  

  • Render / redirect overrides allows developers to simply change what gets rendered or where an action will redirect a user after completion in both success and failure states.

Both of the these r_c features enable developers to add / replace small pieces of logic without overriding and replacing entire controller actions which greatly eases version upgrades, and provides a unique pluggable architecture for Spree extensions. Re-implementing these features using inherited_resources would be very time consuming and counter-productive at this point in time.

Code bloat
During one of our BohConf hacking sessions we decided to rewrite a single controller (admin zones_controller) removing r_c while still retaining all the hooks / extensibility features mentioned in issue 3. This exercise allowed us to evaluate what the codebase might look like after the removal of r_c. We did expect the code to grow a small amount but we were surprised that it actually ballooned from 35 lines to 105 lines after the change. While readability and simplicity did improve, DRYness and maintainability suffered greatly. The overall consensus on this point was that the DRYness / maintainability benefits far out weights any simplifications gained.

After reviewing both options of either removing resource_controller completely or switching to inherited_resources it's become clear that a lot of the customization that Spree offers is deeply linked to resource_controller functionality. While inherited_resources is a promising option it currently poses too much of an overhead in terms of rewriting coding and changes to the overall internal workings of Spree.      

Overall we think that resource_controller adds a lot of value to Spree and that any plans to remove it would damage Spree's well known and loved extensibility.