Skip to content
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

serializer + before_serializer weird behavior #1052

Closed
mickeyn opened this issue Nov 3, 2015 · 6 comments
Closed

serializer + before_serializer weird behavior #1052

mickeyn opened this issue Nov 3, 2015 · 6 comments

Comments

@mickeyn
Copy link
Contributor

mickeyn commented Nov 3, 2015

tried to set the ref to be serialized in the before_serializer hook rather than the route handler, didn't work so well.

here's a minimized version to show the problem:
code: http://pastebin.com/5Gg3CSQS
output: http://pastebin.com/wmcFwbaZ

bug?

@xsawyerx
Copy link
Member

xsawyerx commented Nov 3, 2015

Nope.

package BUG;
use Dancer2;
set serializer => 'JSON';

hook before_serializer => sub {
    $_[0] = { blah => 'blah' };
};

get '/' => sub {
    print STDERR "do nothing in the route handler\n";
};

@xsawyerx xsawyerx closed this as completed Nov 3, 2015
@mickeyn
Copy link
Contributor Author

mickeyn commented Nov 3, 2015

thanks @xsawyerx ;)

so what happens here is that $content is passed to the hook (undefined value), since @_ is aliasing the same input variable (which is later passed to the serializer) we can set it there.
nice.

@xsawyerx
Copy link
Member

xsawyerx commented Nov 3, 2015

@mickeyn++ Thanks for adding an explanation of how it works!

@mickeyn
Copy link
Contributor Author

mickeyn commented Nov 3, 2015

ok, checked and this doesn't really work (@xsawyerx - we only checked a generalized function call outside this problem scope).

problem 1:
since the hook is executed through an intermediate execute_hook which copies @_ into @Args and passes it down to the hook code, $_[0] is not an alias to the original $content (that's created in 'around serialize').

(minor) problem 2:
there's a check on $content and it's length, so the route handler has to return a value in any case to reach the hook execution.
(that can potentially be solved by moving the content check till after the hook - so I'm not worried about it).

I'm not blocked by this, solved my problem with a completely different approach, but this still bugs me :)

though I agree this is not a bug, it's still an open question -
how can I replace the content sent to the serializer after the route handler has run?
(I do believe the proper place should be the 'before_serializer' hook)

@xsawyerx
Copy link
Member

@mickeyn Is this fixed by #1053?

@xsawyerx
Copy link
Member

I guess so. :)

xsawyerx added a commit that referenced this issue 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)
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

No branches or pull requests

2 participants