New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/zero response #1082

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@veryrusty
Member

veryrusty commented Dec 28, 2015

The change to allow before_serializer hooks in commit 5697359 introduced a side effect for any code "getting" the response content. This reverts the changes to the around content modifier made in that commit, resolving #1079 (while keeping @ambs test from #1080).

However this undoes desired functionality from #1053 for a before_serializer being able to change the response content in the case when no route returns content for the response. To trigger the serializer hooks in this case, set the response content attribute to the default (an empty string) when constructing the PSGI response (rather than inject the empty string in the PSGI response as done previously).

( I'm keeping @mickeyn to his word for removing that around modifier )

ambs and others added some commits Dec 26, 2015

Fix resposne->content around modifier
Changes in commit 5697359 broke the around modifier when used as a
getter. Rather than revert, this commit cleans up the logical cases the
modifier has to deal with:

  * If called as a "getter", the modifier is only passed two params.
    Check for that case and return the result of the original method.

  * If there is no serializer defined, encode the content and call the
    original method.

  * Otherwise serialize any content, then call the original method.

Also reverts the changes to the before_serializer counts in t/hooks.t
from the same commit.
Test serializer hooks when route return no content
Adds a test for an app with a serializer defined that has routes that
  * return no conent ( /nothing )
  * return content to be serialized (all the others)
verifying that the before_serializer hook can change the response content
in both cases. See #1053.
Set default response content so hooks get called
If a serializer is defined, before_serializer hooks may change the
response content, including the "default" of an empty string if no route
returned any content for the request.

Set the content to the empty string if the response has no defined
content allowing any hooks to betriggered before constructing the
PSGI response.
@ambs

This comment has been minimized.

Member

ambs commented Dec 30, 2015

👍 by me. Probably @mickeyn and @xsawyerx should comment on this.

Also, sorry for pushing, but it would be great if you could include this patch before the next release.

Happy New Year.

@mickeyn

This comment has been minimized.

Contributor

mickeyn commented Dec 31, 2015

Fine with me.
Happy New Year.

@veryrusty veryrusty added this to the 0.166000 milestone Jan 5, 2016

@veryrusty

This comment has been minimized.

Member

veryrusty commented Jan 5, 2016

Merged as 4969c34

Thanks everyone!

@veryrusty veryrusty closed this Jan 5, 2016

@veryrusty veryrusty deleted the bugfix/zero_response branch Jan 5, 2016

This was referenced Jan 5, 2016

@xsawyerx

This comment has been minimized.

Member

xsawyerx commented Jan 5, 2016

👍

xsawyerx added a commit that referenced this pull request Jan 12, 2016

v0.166000
    [ BUG FIXES ]
    * GH #1013, #1092: Remove race condition caused by caching available
      engines. (Sawyer X, Menno Blom, Russell Jenkins)
    * GH #1089: Exact macthing of route regex comments for tokens/splats.
      (Sawyer X)
    * GH #1079, #1082: Allow routes to return '0' as response content,
      and serializer hooks are called when default response content is
      to be returned. (Alberto Simões, Russell Jenkins)
    * GH #1093, 1095: Use a dynamic TT2 INCLUDE_PATH to allow relative
      views with relative includes; fixing regression introduced by #1037.
      (Russell Jenkins)
    * GH #1096, #1097: Return compatibility on Perl 5.8.x!
      (Peter Mottram - @SysPete)

    [ DOCUMENTATION ]
    * GH #1076: Typo in Dancer2::Core::Hook POD. (Jonathan Scott Duff)

    [ ENHANCEMENTS ]
    * GH #1074: Add sample session engine config to skeleton app.
      (Peter Mottram - @SysPete)
    * GH #1088: Return route objects when defining new routes.
      (Sawyer X)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment