Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

AbstractController#asset_path overrides routes helper

#5578

Commit 2af8fd2829ec573ef7da8a44d31e5f25ef4a8408 introduces a new method, #asset_path, into AbstractController. Unfortunately, this method overrides a method of the same name generated for an Asset resource.

Asset presumably is a very widespread model name and comes with corresponding routes (resources :assets). This change is bound to break a lot of applications in a way that is not initially obvious.

If #asset_path(some_asset) is used in a controller, say for redirection, it causes an ArgumentError (wrong number of arguments (1 for 0)). The routes-generated method expected by the programmer would have taken one argument, but the newly introduced AbstractController#asset_path indeed doesn't take any.

As long as this issue isn't fixed, a workaround is to use #asset_url instead of #asset_path.

Reported by Michael Schuerig · September 8th, 2010 @ 07:14 AM

State: open
Milestone: 3.0.6
Assigned to: nobody
Importance: High

Activity

  1. Michael Koziarski
    Michael Koziarski
    • Milestone cleared.
    • Importance changed from to High

    September 8th, 2010 @ 09:48 PM

  2. rdavila
    rdavila
    • Tag changed from actionpack rails3 to actionpack rails3, invalid

    I can't reproduce this bug, I'm using the following:

    • Rails Edge
    • ruby 1.8.7 (2010-08-16 patchlevel 302) [i686-linux]

    I had created an Asset scaffold and insert one record, next I had modified the index action to:

    redirect_to asset_path(Asset.last)

    And all it's ok, the action redirect to AssetsController#show.

    September 10th, 2010 @ 11:05 PM

  3. Michael Schuerig
    Michael Schuerig
    • Tag changed from actionpack rails3, invalid to actionpack rails3

    I've tried this again in a fresh app on 3.0.0 and it still breaks. I can't test on Rails Edge, as apparently bundler can't resolve dependencies right now (something about activesupport ~> 3.0.0 and 3.1.0.beta).

    Anyway, the relevant code hasn't changed in the meantime. Please have a look at abstract_controller/asset_paths.rb. There config_accessor defines a method #asset_path taking no arguments. AssetPaths is included in ActionController::Base which makes #asset_path (no-args variant) available in controllers.

    I don't know why you don't get an exception where I do. Let's establish two things:

    • The method #asset_path defined in AbstractController::AssetPaths is supposed to be available in controllers.
    • If resources :assets is defined in routes.rb, #asset_path(asset) is supposed to be available in controllers (among other places).
    • These two methods are in conflict with each other.

    Do you agree?

    September 10th, 2010 @ 11:54 PM

  4. David Trasbo
    David Trasbo
    • State changed from new to open

    Michael,

    Yes, asset_path is supposed to be accessible in both Action Mailer and Action Controller.

    I can see two ways around it. You could obviously rename it. Or: You could change the syntax of accessing asset_host and asset_path to be assets.host and assets.path.

    I might look into making a patch for solution #2 - though I suspect it could get a bit complicated.

    September 20th, 2010 @ 07:39 PM

  5. Andrea Campi
    Andrea Campi

    I can confirm this issue on 3.0.0 with both Ruby 1.8.7 and 1.9.2; I could not reproduce it on Rails Edge.

    However, the opposite is true: #asset_path would now be unaccessible in AssetsController and its views.

    October 8th, 2010 @ 01:59 PM

  6. Andrea Campi
    Andrea Campi

    ... and replying to myself: there's no point in calling AssetPaths#asset_path since it may well be a Proc.

    October 8th, 2010 @ 02:11 PM

  7. Andrea Campi
    Andrea Campi

    I'm attaching two patches, on master and 3.0 respectively.
    With this patch applied I cannot reproduce the issue on 3.0; on master the behavior is unchanged.

    That said, I don't like asset_base_path any better than the previous name (it used to be asset_path_template).
    If you have a better idea, I can easily reroll the patches.

    (also, I've made sure tests pass with success)

    October 8th, 2010 @ 04:02 PM

  8. Jeff Kreeftmeijer
    Jeff Kreeftmeijer
    • Tag changed from actionpack rails3 to 3.0, actionpack, patch

    October 10th, 2010 @ 08:25 AM

  9. Jeremy Kemper
  10. Ryan Bigg
    Ryan Bigg
    • Tag changed from sheepskin boots, 3, actionpack, patch to 3.0 actionpack patch

    Automatic cleanup of spam.

    October 16th, 2010 @ 02:32 AM

  11. Andrea Campi
    Andrea Campi
    • Tag changed from 3.0 actionpack patch to 3.0, actionpack, patch

    October 16th, 2010 @ 08:26 PM

  12. Santiago Pastorino
  13. Samuel Kadolph
    Samuel Kadolph

    Quick fix to get 3-0-stable working correctly. Not sure about best solution for master. Probably best to separate out asset configuration from AC and AV and put it with the Application.

    December 15th, 2010 @ 04:22 PM

  14. Santiago Pastorino
  15. thoefer
    thoefer

    Fix for 3.0.5.RC1 attached. It´s basically Samuel Kadolph´s patch, but without the RoutingError in 3.0.5.RC1. This error seems pretty easy to solve. But maybe I don´t see the big picture?! In this case: Any hints? Any comments?

    February 27th, 2011 @ 12:37 AM

  16. Santiago Pastorino
    Santiago Pastorino
    • Milestone changed from 3.0.5 to 3.0.6

    February 27th, 2011 @ 03:15 AM