Hi, Good observations.
> Why not return RedirectResponse from RedirectResolver#resolve(SlingHttpServletRequest)? This would obviate the need for the magic NO_REDIRECT status code. Instead null could be returned if no redirect should be performed. Initially an interface was used, and hence passed in to avoid binding implementations of the provider to the interface. Being passed in and not returned means the provider will continue to work regardless of changes to the RedirectResponse interface over time. Obviously methods will be deprecated and ideally not removed for ages so providers dont have to be patched. That said, on reflection, I switched to using a non abstract, non final class, so both consumer and provider are loosely bound to the class and free to implement by extension. With a class, it could be returned and implemented on either side, provided the sides agree. Then the question comes down to, which side ? Should the consumer drive this or the producer ? The consumer has to act on what is returned from the producer, the producer suggesting what should be done. Hence, even with a class, I think the consumer should extend the class and pass it as a param for the producer propose how redirection should be. > I am also wondering if it is sensible to encode the word "redirect" into the class/interface names. Is a redirect really the only use-case? Or would it be conceivable to use the same mechanism for other responses that don't require a response body? While the mechanism could be used for any non-body response, the intention is only for redirection, hence I think RedirectResolver is better than ResponseResolver or something wider. Best Regards Ian On Fri, 31 Jan 2020 at 10:42, Julian Sedding <jsedd...@gmail.com> wrote: > Hi, > > Why not return RedirectResponse from > RedirectResolver#resolve(SlingHttpServletRequest)? This would obviate > the need for the magic NO_REDIRECT status code. Instead null could be > returned if no redirect should be performed. > > I am also wondering if it is sensible to encode the word "redirect" > into the class/interface names. Is a redirect really the only > use-case? Or would it be conceivable to use the same mechanism for > other responses that don't require a response body? > > Another consideration could be whether the same mechanism could > (should?) be used to allow setting headers for responses that then > continue through normal processing. (I know, the same can be achieved > using a servlet filter.) > > No hard opinions, just soft thoughts ;) > > Regards > Julian > > On Fri, Jan 31, 2020 at 11:00 AM Ian Boston <i...@tfd.co.uk> wrote: > > > > Hi, > > NO_REDIRECT, good point, added. > > > > I think HttpServletRequest is better as it is wider. > > If an implementation of a RedirectResolver requires > SlingHttpServletRequest > > it can check for that and cast to it, returning NO_REDIRECT if the > context > > isn't what it required. > > > > Best Regards > > Ian > > > > > > On Fri, 31 Jan 2020 at 08:49, Bertrand Delacretaz < > bdelacre...@apache.org> > > wrote: > > > > > Hi, > > > > > > On Fri, Jan 31, 2020 at 9:11 AM Ian Boston <i...@tfd.co.uk> wrote: > > > .. > > > > 1 https://gist.github.com/ieb/5f217e2c160afb7bb4098bca99896621 > > > ... > > > > > > The RedirectResolver gets an HttpServletRequest, shouldn't that be a > > > SlingHttpServletRequest to be able to get at the Resource, selectors > > > etc? > > > > > > I understand the Resource can come from having used adaptTo before, > > > but other request attributes might influence the redirect and we have > > > them already parsed in SlingHttpServletRequest. > > > > > > And a nitpick, in RedirectResponse I would make -1 a constant, > NO_REDIRECT. > > > > > > -Bertrand > > > >
