Remove Core::Context in favor of smarter Core::Request

[xsawyerx]

One thing that bugs me is Core::Context. It is created in the app from the request, contains a reference back to the app, and the app contains a reference to itself. It holds both data and actions, but neither on itself.

Until now I wasn't even sure what the hell it is exactly meant for. Now I understand it better, and it seems like we could actually remove it (and some duplicate code that exists there) in favor of making the Core::Request object more aware of able.

I'd like to get some opinions on this.

[xsawyerx]

@mickeyn and I are hacking on it.

[xsawyerx]

We've done a ton of work on this, available in a branch.

We were basically able to remove from the codebase the biggest part of the Core::Context variable, which is its request attribute. That's the majority of what's used in it. Friday we will continue and try to detach more and more strings.

All tests pass so far.

[mickeyn]

so, https://github.com/PerlDancer/Dancer2/tree/feature/untangle-context is fully functional and context-less.
we need reviewing before merging it, anyone want to assist with that?

[xsawyerx]

I am willing to offer $something in exchange. Just please someone review this. @mickeyn and I will be happy to help lend our explanation and assistance.

I'd prefer not to push it without another person (or more) reviewing this.

[veryrusty]

I'll review it. It may take a week, but I will get there (there's a lot to get through)!

/me starts thinking about what $something to ask @xsawyerx for...

[mickeyn]

@veryrusty don't be shy, he actually meant $anything :)

[veryrusty]

Ok - got through that branch! @mickeyn++ for the AWESOME commit messages!!

I've made comments inline and/or on individual commits as necessary. There are two things that need addressing before attempting a merge (I'm probably repeating myself):

1. A change to Core::Role::DSL in 9c3b21b removes the distinction between DSL keywords that can be used within or outside a route definition.
2. Moving Core::App->session method to the DSL in 61a47fd will break any plugin that currently does $app->session(...).

We may also wish to log a warning (deprecate) the context DSL keyword.

There has also been a pass method added to the context object since this work was started. An almost identical commit to e1989ca (with s/halt/pass/) should suffice.

[xsawyerx]

I just want to note I coerced @mickeyn to write longer commit messages, other than "fixed shit". :)

[veryrusty]

Looking at @mickeyn's "merged" branch (mickey/merging_context_untangle_-_still_broken), for the failing t/dsl/pass.t the issue is in the Dispatcher.

1. if pass is used, we look for the next matching route; in this case its not required to do the $app->cleanup in Line 79.
2. That leaves the possibility of a route passing, but there is no further matching routes. We'd need an $app->cleanup after iterating through all the routes after here. Though you then can't pass between routes defined in different apps and have parts of the response (like headers) propagate.

[mickeyn]

@veryrusty - spot on! fixed.
thanks ;)

[xsawyerx]

Last fail is simply the Mutable Serializer trying to resolve the content type using the request information from the context. Because it has no context, it can't do that.

It makes sense for a serializer to have access to the content type (or resolved content type, which is derived from multiple headers), maybe even the request. I'm still not sure how to do this, since the serialization and deserialization methods are called from the response, which doesn't have the request either.

[xsawyerx]

Merged. Releasing now as development version. We need to get working on cleaning up the plugins.