-
Notifications
You must be signed in to change notification settings - Fork 66
#416 proposed solution with dynamic rule lookup #588
Conversation
Haven't fully wrapped my mind around this yet, but a question did pop up: this assumes the presence of conf:*-rules() functions. Isn't it likely they will disappear in future as well, since everything is now recorded in rewriter.xml? Would it make sense to somehow consume that file instead? Or maybe generate a new customized rewriter.xml that appends the Roxy routes? |
I've actually played around with trying to get rewriter.xml working to route to Roxy but it's not trivial. Even though I've gotten it 75% working the problem is that you end up in an "infinite loop" in a lot of situations where rewriter.xml reroutes to rewriter.xqy who's routed request then hits rewriter.xml etc. You also have problems pulling the originally requested URI and parameters because the incoming request to rewriter.xqy comes from rewriter.xml. After implementing a lot of tricks (passing the original route as a param from rewriter.xml, modifications to the rewriter.xqy, careful regex, etc.) I got most of the routes working but not all. I could push this into a new branch on my fork if you're interested in taking a look. Ultimately, after working away on adding rewriter.xml, I think the best thing would be to make a substantial change to routing so that it's handled exclusively by rewriter.xml or rewriter.xqy. @dmcassel has done the latter and I fully support it for an immediate solution. Long-term, as @grtjn points out, it's probably worth trying to migrate to rewriter.xml to avoid this happening again. |
issue-416: No longer eval the rewrite to rest
Any attempts should interpret rewriter.xml instead of making rewriter.xml point to roxy rewriter or vice versa. One of both needs to take control, or to put it more precisely: one of both needs to take full control, because you can no longer invoke one from the other in a simple elegant way.. |
I gave this solution to another consultant who in turn gave it to a large health insurer today (not the one y'all probably affiliate me with) to help them migrate a hybrid app from ML7 to ML8. Worked like a charm in all of their testing. |
@RobertSzkutak excellent! @grtjn @paxtonhare what do you think about accepting this PR? |
I still need a spare moment to look closer, just back from some time away.. |
Would also solve #599. The comment from Robert sounds very convincing. I wouldn't mind if we merge this. It only touches mvc and hybrid anyhow, which are used less these days.. |
@garyrusso confirmed in #599. Merging.. |
I took a similar approach to what other people had posted. Because the rule functions have changed over time, I dynamically loaded the conf:*-rule() functions. Rewriting needs to be fast, so to avoid doing that on every call, I put the results into a server field.
@tdiepenbrock, @grtjn, @JoeG13, @RobertSzkutak, @garyrusso, @aajacobs -- anybody available to test in different scenarios?
I encountered #416 trying to run unit tests with ML 8.0-4 with an MVC project.