Hi all,
The Hunchentoot documentation at http://weitz.de/hunchentoot/ mentions that an around-method on process-request "might be a good place to introduce around methods which bind special variables or do other interesting things."
In my eyes there's a problem with this, because the standard process-request sets up crucial parts of the dynamic environment for request processing, and around methods can only modify the scope outside of that. In particluar, error and warning conditions are handled before the around method gets a chance to intervene.
I would suggest adding a new generic function that does just the dispatching, i.e. something like this:
(defmethod dispatch-request ((*request* t)) (funcall (acceptor-request-dispatcher *acceptor*) *request*))
..and have the kernel of the standard process-request method just call dispatch-request in the obvious way.
I think this is a more appropriate configuration point for the request processing protocol than the current process-request is.
(I can/could of course specialize process-request for my acceptor class using an unqualified method, and thus set up the dynamic environment just how I want it. However I would then just have to copy the full body of the standard process-request to insert a line or two inside the body of the handler-bind. I believe it's the purpose of a processing protocol to avoid having to do such things.)
frode@netfonds.no (Frode V. Fjeld) writes:
(I can/could of course specialize process-request for my acceptor class using an unqualified method, and thus set up the dynamic environment just how I want it. However I would then just have to copy the full body of the standard process-request to insert a line or two inside the body of the handler-bind. I believe it's the purpose of a processing protocol to avoid having to do such things.)
This didn't really work out in practice: cut'n'pasting the body of process-request into a different package made it clear that process-request can't really be replaced without breaking lots of stuff. Also, modifying my dispatchers or handlers proved to be impractical. So here's a diff detailing my suggestion in my previous posting. It's simple, and seems to work well.
Index: packages.lisp =================================================================== --- packages.lisp (revision 4474) +++ packages.lisp (working copy) @@ -168,6 +168,7 @@ "DELETE-AUX-REQUEST-VALUE" "DELETE-SESSION-VALUE" "DISPATCH-EASY-HANDLERS" + "DISPATCH-REQUEST" "ESCAPE-FOR-HTML" "EXECUTE-ACCEPTOR" "GET-PARAMETER" Index: request.lisp =================================================================== --- request.lisp (revision 4474) +++ request.lisp (working copy) @@ -210,6 +210,10 @@ ;; we assume it's not our fault... (setf (return-code*) +http-bad-request+)))))
+(defmethod dispatch-request (request) + (funcall (acceptor-request-dispatcher *acceptor*) + request)) + (defmethod process-request (request) "Standard implementation for processing a request. You should not change or replace this functionality unless you know what you're @@ -227,7 +231,7 @@ ;; skip dispatch if bad request (when (eql (return-code *reply*) +http-ok+) ;; now do the work - (funcall (acceptor-request-dispatcher *acceptor*) *request*))))) + (dispatch-request request))))) (when error (setf (return-code *reply*) +http-internal-server-error+))
Frode,
the patch looks good. Can you please provide a patch with a docstring and a documentation update?
Thanks, Hans
On Wed, Nov 25, 2009 at 11:23, Frode V. Fjeld frode@netfonds.no wrote:
frode@netfonds.no (Frode V. Fjeld) writes:
(I can/could of course specialize process-request for my acceptor class using an unqualified method, and thus set up the dynamic environment just how I want it. However I would then just have to copy the full body of the standard process-request to insert a line or two inside the body of the handler-bind. I believe it's the purpose of a processing protocol to avoid having to do such things.)
This didn't really work out in practice: cut'n'pasting the body of process-request into a different package made it clear that process-request can't really be replaced without breaking lots of stuff. Also, modifying my dispatchers or handlers proved to be impractical. So here's a diff detailing my suggestion in my previous posting. It's simple, and seems to work well.
Index: packages.lisp
--- packages.lisp (revision 4474) +++ packages.lisp (working copy) @@ -168,6 +168,7 @@ "DELETE-AUX-REQUEST-VALUE" "DELETE-SESSION-VALUE" "DISPATCH-EASY-HANDLERS"
- "DISPATCH-REQUEST"
"ESCAPE-FOR-HTML" "EXECUTE-ACCEPTOR" "GET-PARAMETER" Index: request.lisp =================================================================== --- request.lisp (revision 4474) +++ request.lisp (working copy) @@ -210,6 +210,10 @@ ;; we assume it's not our fault... (setf (return-code*) +http-bad-request+)))))
+(defmethod dispatch-request (request)
- (funcall (acceptor-request-dispatcher *acceptor*)
- request))
(defmethod process-request (request) "Standard implementation for processing a request. You should not change or replace this functionality unless you know what you're @@ -227,7 +231,7 @@ ;; skip dispatch if bad request (when (eql (return-code *reply*) +http-ok+) ;; now do the work
- (funcall (acceptor-request-dispatcher *acceptor*) *request*)))))
- (dispatch-request request)))))
(when error (setf (return-code *reply*) +http-internal-server-error+))
-- Frode V. Fjeld Netfonds Bank ASA
tbnl-devel site list tbnl-devel@common-lisp.net http://common-lisp.net/mailman/listinfo/tbnl-devel
Hans Hübner hans.huebner@gmail.com writes:
the patch looks good. Can you please provide a patch with a docstring and a documentation update?
Sure, I'm attaching a patch. I tweaked the docstring for process-request also.
This patch is simple in the sense that it provides an extra configuration point without otherwise interfering with the API.
BTW, having now looked into using process-request as a "configuration point", I would suggest that there should be some refactoring: Move the pieces of it that are intrinsic to hunchentoot, such as bindings etc. for *tmp-files*, *headers-sent*, *request*, handler-done, and *within-request-p* [why not just (not (null *request*))?] into process-connection (or some internal defun), and keep just the stuff that implements the default condition handling.
The point would be to have two well-defined configuration points: process-request that does condition handinling, and dispatch-request that is within the scope of said condition handling.
Frode,
On Wed, Nov 25, 2009 at 13:14, Frode V. Fjeld frode@netfonds.no wrote:
Hans Hübner hans.huebner@gmail.com writes:
the patch looks good. Can you please provide a patch with a docstring and a documentation update?
Sure, I'm attaching a patch. I tweaked the docstring for process-request also.
I have committed that patch to the Subversion repository.
BTW, having now looked into using process-request as a "configuration point", I would suggest that there should be some refactoring: Move the pieces of it that are intrinsic to hunchentoot, such as bindings etc. for *tmp-files*, *headers-sent*, *request*, handler-done, and *within-request-p* [why not just (not (null *request*))?] into process-connection (or some internal defun), and keep just the stuff that implements the default condition handling.
The point would be to have two well-defined configuration points: process-request that does condition handinling, and dispatch-request that is within the scope of said condition handling.
I'd be fine with such a refactoring and cleanup! I have not reviewed the code myself, but replacing *within-request-p* with an access to *request* sounds like a worthwhile simplification.
-Hans
On Mon, Nov 30, 2009 at 2:16 PM, Hans Hübner hans.huebner@gmail.com wrote:
I have committed that patch to the Subversion repository.
Sorry, I'm very slowly catching up with Hunchentoot work. I've revoked this patch for now because the documentation was incorrect in several places (it said that process-request is called anew for each request, that process-request calls the acceptor's request dispatcher, and so on).
Also, I think that this addition is not really necessary as you can bind any special variables in your own request dispatcher if you want, or am I missing something?
Thanks, Edi.
On Wed, Nov 25, 2009 at 1:14 PM, Frode V. Fjeld frode@netfonds.no wrote:
*within-request-p* [why not just (not (null *request*))?]
Because *request* is unbound outside of requests... :)
But I've removed *with-request-p* in the repository now.
Thanks, Edi.
-
Edi Weitz -
frode＠netfonds.no -
Hans Hübner