Friday, 30 November 2007

Pick your layer

I've been noticing that some of our newer Rails developers are having trouble picking which layer (Model, View or Controller) a method belongs in. Here are some heuristics I've picked up along the way - in no particular order.

1 - Don't put it into a library unless it really has nothing to do with the rest of your site (or you want to package it up to hand to other people).

I've recently noticed a few methods being added to a library I wrote that like this: my_method(@my_object) This is a prime candidate for going into the MyObject model itself. Especially when the body of said method just accesses some related objects eg:

# instead of:
module MyLibrary
  def my_method(my_obj) {|sc| [,] }
# put it on the model
class MyModel < ActiveRecord::Base
  has_many :sub_classes
  def my_method
    return [] if sub_classes.blank? {|sc| [,] }

2 - A model should know how to deal with itself

I'd like to say that everything that can go into a model should... but that's not quite true. Anything data-related, however, is a prime candidate. A model should know how to deal with it's own data. It should know how to calcuate and derive from its own data. It should know how to pull stuff out of related objects, and to parse other objects into a form that allows it to stuff data into itself.

I've seen too many of these sorts of calculations going into views or helpers, or getting stuck in the middle of controller-code. These will only come back to bite you later, when the client suddenly realises they want the report to also be generated into pdf or graphical form as well...

If it can go on the model, put it there, because you'll always have access to the methods on your model.

3 - Don't put html into your model

By all means put display-names and other, similar methods that produce displayable information... but don't put html into it. After all, right now you may be concentrating on web-only delivery of your data... but who's to say the client won't be asking for xml next week... and pdf the week after that... followed by CSV?

Make your model-based display methods text-only eg:

class MyModel < ActiveRecord::Base
  def good_display_name
    name || "Unspecified"
  def bad_display_name
    "<div class=\"left_aligned_red_box\">#{good_display_name}</div>"

4 - Put format-specific display functions in the helpers

If you're displaying a selection of a certain class of objects (eg Dates) in a specific format in multiple places across the site, and want consistency of appearance (a Good Thing), feel free to add a display helper to the ApplicationHelper file, or even update the generic display function for that class in your environment.rb

  # eg add these to application helper
  def display_percentage(num)
    return "" unless num
  def display_date(date)
    return "" unless date
    date.strftime("%a %d/%m/%Y")
  def display_datetime(datetime)
    return "" unless datetime
    "#{display_date(datetime)} #{datetime.strftime("%H:%M")}"

Note that you can over-ride the default datetime display in rails using the following (in environment.rb or similar) then use "to_s" or "to_s(:datetime)" in your views.

   :default => "%a %d/%m/%Y",
   :datetime  => "%a %d/%m/%Y %H:%M"

Monday, 26 November 2007

Rails gotchas: ruby-enhanced yaml != rails-enhanced yaml

I you've just added some funky ruby-generated something into your yaml fixture and are suddenly getting this error message:

Fixture::FormatError: a YAML error occurred parsing /myproject/config/../test/fixtures/widgets.yml. Please note that YAML must be consistently indented using spaces. Tabs are not allowed. Please have a look at
The exact error was:
  SyntaxError: compile error
(erb):8: syntax error, unexpected ')'
_erbout.concat "  created_at: "; _erbout.concat(( '1 January 2006'.to_date.to_s(:db) -).to_s); _erbout.concat "\n"

Check to make sure you haven't done the "good" thing and used -%>. It appears that YAML files don't like the whitespace-eating ending... you can only use: %>

Monday, 19 November 2007

Rails gotchas: Kamikaze data migrations

This is a pernicious little gotcha I've encountered a couple of times, generally by using somebody else's plugins. The problem with it is that it usually doesn't actually affect the developer - just anybody else that uses the system. This can lead to the developer refusing to believe that there is a problem.[1]

So what am I talking about here?

I'm talking about a certain type of data migration that explodes as you run it, sometimes even causing the database to get stuck in a nasty halfway-inbetween state where you can neither continue migrating up... or roll back to a previous stable state.

"But migrations are transactional" I hear you cry. Sure they are. That doesn't stop this particular error from occurring... and *still* leaving the db in a nasty stuck state - honest, just gimme a chance to explain why.

Picture this: You've been happily developing away building the next generation of your category-killer Web 2.0 Widget application when you suddenly realise that your widgets need to refer to pieces instead of parts. So, like a good agile developer you:

  • Create a new Piece resource with all the bells and whistles.
  • Create and run the migration that a) creates the pieces table b) data migrates widgets so they point at pieces instead of parts c) drops the parts table.
  • Update all references in your code from pieces to parts.
  • Remove all the part code from controllers/views etc.

All goes smoothly. rake test runs flawlessly and you check in your code. You continue on your merry way, adding more functionality and reflecting on your brilliance...

...until a user of your code emails you to say that your migration breaks and has left their db in some wierd state that doesn't let them migrate up or down.

The problem is that when you (the developer) ran the migration, you still had the Part class in app/models and the Widget class still had references like "has_one :part" in them. When your user downloads your system, however, it may be several changes later... and the system has no recollection of a Part class or how they relate to Widgets. So when you have a data migration such as:

   Widget.find_all.each { |w| make_piece_from_part(w.part) }

It won't know what a part is - or how you get one off your widget class. The nasty part is that often you will find this kind of data migration stuffed in the middle of the schema migrations thus:

def self.up
  create_table :pieces do |t|
     #... creates a table here
  add_column :widgets, :piece_id, :integer, :default => nil
  Widget.find_all.each { |w| make_piece_from_part(w.part) }
  remove_column :widgets, :part_id
def self.up
  add_column :widgets, :part_id, :integer, :default => nil
  Widget.find_all.each { |w| make_part_from_piece(w.piece) }
  remove_column :widgets, :piece_id
  drop_table :pieces

I'm told that migrations are transactional - ie if it fails, then everything rolls back to the state before the migration... this doesn't seem to work when data migrations are involved. If the data part of the migration falls over, it stops halfway through the migration with a nasty backtrace, but doesn't perform any rollback. So at that point it's already added the pieces table and the piece_id column to the widgets table, but it hasn't removed the part_id column and the schema is still pointing at the migration number before this one. So if you try to run the migration again, it will fail on the first line, saying something like:

== MovePartsToPieces: migrating ============================================
-- create_table(:pieces)
rake aborted!
Mysql::Error: Table 'pieces' already exists: CREATE TABLE pieces (<piece-table insert statement here>) ENGINE=InnoDB

(See full trace by running task with --trace)

Unfortunately it also won't migrate you back. The bad data migration failed before it had a chance to update the current migration level - so migrating back gives you entirely unhelpful "finished in 0.000183" seconds message (or similar), without doing anything at all.

Recovering from the stuck state

Recovering is possible, but it takes some small amount of hacking in the migration. Basically you need to force Rails to run an empty migration so that it does nothing except to put the version number up. Then you can force it to run the bits of the down migration that cancel the bits that it did the first time you tried running the up migration. In detail:

  1. Comment out every line of code in self.up
  2. Comment out the appropriate lines in self.down that correspond with stuff that never got run when the migration fell over the first time - ie leave anything that cancels out actions that were *successfully* done when you did the up migration. in the example above, you'd comment out the downward data migration and the addition of the piece_id column - leaving only the removal of the part_id column and the drop table - which would leave you back in the state you started in.
  3. Save the file
  4. Now you can run rake db:migrate. This should should leave you with a database that at least has the right version number for you to be able to migrate back down again.
  5. Now run the down migration with rake db:migrate VERSION=<one before this one>.
  6. Now go fix your migration so this doesn't happen to anybody else.

Fixing this kind of data-migration nightmare is the only time that I recommend you go back and edit old migrations rather than creating a new one.

  • [1]Ok, so the real reason I'm writing this post is partly so that I can point said unbelievers at it the next time they swear that nothing's wrong and that I should just quit fussing about.

Wednesday, 14 November 2007

Securing Rails

Quick post today. Just found a really great Rails Security Cheatsheet here. Has links to useful articles on all sorts of topics about rails security, including (but not limited to):

  • Why do security for Rails
  • Securing your sessions
  • Useful security plugins
  • Securing files on your server
  • Common web security issues (XSS, SQL injection etc)