Skip to content

Comments

Allow before_serializer to set content (using the alias in @_)#1053

Closed
mickeyn wants to merge 1 commit intomasterfrom
mickey/allow_before_serializer_to_set_content
Closed

Allow before_serializer to set content (using the alias in @_)#1053
mickeyn wants to merge 1 commit intomasterfrom
mickey/allow_before_serializer_to_set_content

Conversation

@mickeyn
Copy link
Contributor

@mickeyn mickeyn commented Nov 5, 2015

To make it even more meaningful, this change also makes sure
before_serializer will always be called when 'serializer' is
set.

The check against the content being empty is done after the
before_serializer hook is called.

I also altered the checks against the content's length.
It's likely that such check will produce warnings in future
Perl versions, as this will check length of stringified refs
when content is a ref (very common).

To make it even more meaningful, this change also makes sure
before_serializer will always be called when 'serializer' is
set.

The check against the content being empty is done after the
before_serializer hook is called.

I also altered the checks against the content's length.
It's likely that such check will produce warnings in future
Perl versions, as this will check length of stringified refs
when content is a ref (very common).
@mickeyn
Copy link
Contributor Author

mickeyn commented Nov 5, 2015

tied to this discussion: #1052

@xsawyerx
Copy link
Member

xsawyerx commented Nov 6, 2015

This commit also does other stuff:

  • Change a length check to a ref check.
  • Remove a content length check entirely.

We talked in person of the latter, but why the former?

@mickeyn
Copy link
Contributor Author

mickeyn commented Nov 7, 2015

You are referring to another length check (we discussed the one on $content). the change that you mention here is for the passed hook name, checking if it's defined and that it has length is wrong as this check will pass for a reference which is then stringified to be used as a hash key (sure it won't match existing hooks but like in the other 'length' case, relies on ref stringification which is wrong and may lead to warnings in future Perl releases - I had a discussion with Rafael about it and he was surprised it doesn't already).
The suggested check for having a value and it not being a ref seems more sensible to me.

@xsawyerx xsawyerx closed this Dec 16, 2015
@xsawyerx xsawyerx deleted the mickey/allow_before_serializer_to_set_content branch December 16, 2015 19:48
@xsawyerx
Copy link
Member

Merged, thanks! 👍

xsawyerx added a commit that referenced this pull request Dec 16, 2015
    [ DOCUMENTATION ]
    * Update core team members and contributors list. (Russell Jenkins)
    * GH #1066: Fix typo in Cookbook. (gertvanoss)
    * Correct typo. It's "query_parameters", not "request_parameters".
      Thanks to mst for letting me know and making sure I fix it!
      (Sawyer X)

    [ BUG FIXES ]
    * GH #1040: Forward with a post body no longer tries to re-read body
      filehandle. (Bas Bloemsaat)
    * GH #1042: Add Diggest::SHA as explicit prequisite for installs on
      perl < v5.9.3. (Russell Jenkins)
    * GH #1071, #1070: HTML escape the message in the default error page.
      (Peter Mottram)
    * GH #1062, #1063: Command line interface didn't support
      "-s SKELETON_DIRECTORY" in any order.
      (Nuno Carvalho)
    * GH #1052, #1053: Always call before_serializer hook when serializer
      is set.
      (Mickey Nasriachi)
    * GH #1034: Correctly use different session cookie name for Dancer2.
      (Jason A. Crome)
    * GH #1060: Remove trailing slashes when providing skeleton
      directory.
      (Gabor Szabo)

    [ ENHANCEMENTS ]
    * Use Plack 1.0035 to make sure you only have HTTP::Headers::Fast
      in the Plack::Request object internally.
    * GH #951 #1037: Dancer2::Template::TemplateToolkit no longer sets TT2
      INCLUDE_PATH directive, allowing `views` setting to be non-absolute
      paths. (Russell Jenkins)
    * GH #1032 #1043: Add .dancer file to new app scaffolding.
      (Jason A. Crome)
    * GH #1045: Small cleanups to Request class. (Russell Jenkins)
    * GH #1033: strict && warnings in Dancer2::CLI. (Mohammad S Anwar)
    * GH #1052, #1053: Allow before_serializer hook to change the content
      using @_.
      (Mickey Nasriachi)
    * GH #1060: Ignore .git directory when using an external skeleton
      directory.
      (Gabor Szabo)
    * GH #1060: Support more asset file extensions. (Gabor Szabo)
    * GH #1072: Add request->is_options(). (Theo van Hoesel)
@veryrusty veryrusty mentioned this pull request Dec 27, 2015
veryrusty added a commit that referenced this pull request Dec 28, 2015
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.
@veryrusty veryrusty mentioned this pull request Dec 28, 2015
veryrusty added a commit that referenced this pull request Jan 5, 2016
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants