Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #6175 (closed defect: fixed)

Opened 2 years ago

Last modified 1 year ago

[PATCH] flash not swept when filter chain is halted

Reported by: caio Assigned to: marcel@vernix.org
Priority: normal Milestone: 1.x
Component: ActionPack Version: edge
Severity: normal Keywords: flash filters halt
Cc: bitsweat

Description

See discussion at http://wrath.rubyonrails.org/pipermail/rails-core/2006-July/001898.html

See also #3527

Even if you decide against this patch, check the diff for flash.rb as it fixes another silly bug in the flash.

Attachments

sweep_flash_in_before_filters_v2.diff (1.5 kB) - added by caio on 09/12/06 06:51:20.
patch
actionpack_test_flash_update.diff (1.2 kB) - added by caio on 09/12/06 14:56:22.
tests for the silly flash bug
actionpack_test_sweep_after_halted_filter_chain.diff (1.4 kB) - added by caio on 09/12/06 15:47:05.
test
6175_flash_update_and_filter_chain+tests.diff (3.9 kB) - added by caio on 09/12/06 15:49:45.
all together now
6175_flash_update_and_filter_chain+tests_r5270.diff (5.2 kB) - added by caio on 10/09/06 07:54:46.
revised patch
6175_flash_update_etc.diff (5.2 kB) - added by nzkoz on 02/25/07 00:34:54.
Reattachment of ghost patch.
6175_r6235_filters_and_flash.diff (4.9 kB) - added by caio on 02/25/07 22:09:54.
updated patch
6175_r6462_filters_and_flash.diff (5.4 kB) - added by caio on 03/26/07 09:04:35.
updated patch for r6462
6175_r6650_filters_and_flash.diff (3.5 kB) - added by caio on 05/02/07 16:00:57.
updated patch for r6650

Change History

09/12/06 06:51:20 changed by caio

  • attachment sweep_flash_in_before_filters_v2.diff added.

patch

09/12/06 07:18:52 changed by bitsweat

  • status changed from new to closed.
  • resolution set to untested.

09/12/06 14:56:22 changed by caio

  • attachment actionpack_test_flash_update.diff added.

tests for the silly flash bug

09/12/06 15:47:05 changed by caio

  • attachment actionpack_test_sweep_after_halted_filter_chain.diff added.

test

09/12/06 15:49:45 changed by caio

  • attachment 6175_flash_update_and_filter_chain+tests.diff added.

all together now

09/12/06 15:49:59 changed by caio

  • status changed from closed to reopened.
  • resolution deleted.

10/09/06 01:29:59 changed by david

  • owner changed from David to marcel@vernix.org.
  • status changed from reopened to new.

10/09/06 01:40:37 changed by marcel

Hey Caio. The patch doesn't apply cleanly and the differences between the patched version and the current version aren't obvious. Sorry to let the patch go stale but we're set to try to apply. Could you look into getting it up to date? Thanks.

10/09/06 07:54:46 changed by caio

  • attachment 6175_flash_update_and_filter_chain+tests_r5270.diff added.

revised patch

10/09/06 08:00:21 changed by caio

Hi, new patch is here.

This patch is basically about removing all the code for the special case that is this bug (which was replicated in kaes filter reimplementation simply because he was the first to notice it, I tihnk, see ticket mentioned in description).

So I had to find the updated parts to remove. Among other things, I removed the instance variable @before_filter_chain_aborted, which is now unnecessary, but was being referred to in some tests. I simply removed those assertions. With that, all filter and flash tests pass.

02/25/07 00:34:54 changed by nzkoz

  • attachment 6175_flash_update_etc.diff added.

Reattachment of ghost patch.

02/25/07 22:09:54 changed by caio

  • attachment 6175_r6235_filters_and_flash.diff added.

updated patch

(follow-up: ↓ 7 ) 03/05/07 22:04:49 changed by bitsweat

  • cc set to bitsweat.
  • keywords set to flash filters halt.

I don't quite understand this patch, and the implementation is confusing, especially 

       def update(h) #:nodoc:
-        h.keys.each{ |k| discard(k) }
+        h.keys.each { |k| keep(k) }
         super
       end

Could you explain it in some detail? Thanks caio.

(in reply to: ↑ 6 ) 03/06/07 01:17:55 changed by caio

The code you quoted is actually part of another bug, IIRC I ran into it and ended up patching the whole thing at once. Apparently the filter bug is not even dependent on it.

The bug that part of the patch fixes is that calling #update on the flash causes it to expire the updated keys instead of keeping them for the next request.

The current flash implementation is still based on mine, http://dev.rubyonrails.org/ticket/839, and the lines you mention remain unchanged from the original patch.

So the bug was there since forever. I'm just cleaning up after my own mess.

If you want to understand the filter bug too, not only the code you quoted, please read the discussion and ticket linked in the description.

03/26/07 09:04:35 changed by caio

  • attachment 6175_r6462_filters_and_flash.diff added.

updated patch for r6462

04/17/07 14:45:48 changed by Catfish

I ran into this problem just now, would like to see the patch applied.

05/02/07 16:00:57 changed by caio

  • attachment 6175_r6650_filters_and_flash.diff added.

updated patch for r6650

05/06/07 04:17:05 changed by marcel

  • status changed from new to closed.
  • resolution set to fixed.

(In [6670]) Sweep flash when filter chain is halted. Closes #6175. [Caio Chassot <lists@v2studio.com>]