On Tue, Jul 16, 2013 at 12:19 PM, Jure Zitnik <[email protected]> wrote:
> On 7/16/13 8:46 PM, Ryan Ollos wrote: > >> On Tue, Jul 16, 2013 at 11:38 AM, Matevž Bradač <[email protected]> >> wrote: >> >> On 16. Jul, 2013, at 18:58, Ryan Ollos wrote: >>> >>> On Mon, Jul 15, 2013 at 3:18 PM, Apache Bloodhound < >>>> [email protected]> wrote: >>>> >>>> #579: ProductizedHref fails for dicts as first argument >>>>> ---------------------------+--**--------------------- >>>>> Reporter: olemis | Owner: gjm >>>>> Type: defect | Status: review >>>>> Priority: major | Milestone: Release 7 >>>>> Component: multiproduct | Version: >>>>> Resolution: | Keywords: web href >>>>> ---------------------------+--**--------------------- >>>>> Changes (by olemis): >>>>> >>>>> * owner: olemis => gjm >>>>> >>>>> >>>>> Old description: >>>>> >>>>> Productized Href breaks Href contract when dict objects are passed as >>>>>> first argument. See sample code below >>>>>> >>>>>> {{{#!py >>>>>> >>>>>> from trac.web.href import Href >>>>>>>>> h = Href('/x/y') >>>>>>>>> p = dict(a=1,b=2,c=3) >>>>>>>>> h(p) >>>>>>>>> >>>>>>>> '/x/y?a=1&c=3&b=2' >>>>>> >>>>>>> from multiproduct.hooks import ProductizedHref >>>>>>>>> help(ProductizedHref) >>>>>>>>> help(ProductizedHref) >>>>>>>>> phref = ProductizedHref(h, 'z') >>>>>>>>> phref(p) >>>>>>>>> >>>>>>>> Traceback (most recent call last): >>>>>> File "<stdin>", line 1, in <module> >>>>>> File "multiproduct/hooks.py", line 106, in __call__ >>>>>> filter(lambda x: args[0].startswith(x), self.STATIC_PREFIXES): >>>>>> File "multiproduct/hooks.py", line 106, in <lambda> >>>>>> filter(lambda x: args[0].startswith(x), self.STATIC_PREFIXES): >>>>>> AttributeError: 'dict' object has no attribute 'startswith' >>>>>> }}} >>>>>> >>>>> New description: >>>>> >>>>> Productized Href breaks Href contract when dict objects are passed as >>>>> first argument. See sample code below >>>>> >>>>> {{{#!py >>>>> >>>>> from trac.web.href import Href >>>>>>>> h = Href('/x/y') >>>>>>>> p = dict(a=1,b=2,c=3) >>>>>>>> h(p) >>>>>>>> >>>>>>> '/x/y?a=1&c=3&b=2' >>>>> >>>>>> from multiproduct.hooks import ProductizedHref >>>>>>>> phref = ProductizedHref(h, 'z') >>>>>>>> phref(p) >>>>>>>> >>>>>>> Traceback (most recent call last): >>>>> File "<stdin>", line 1, in <module> >>>>> File "multiproduct/hooks.py", line 106, in __call__ >>>>> filter(lambda x: args[0].startswith(x), self.STATIC_PREFIXES): >>>>> File "multiproduct/hooks.py", line 106, in <lambda> >>>>> filter(lambda x: args[0].startswith(x), self.STATIC_PREFIXES): >>>>> AttributeError: 'dict' object has no attribute 'startswith' >>>>> }}} >>>>> >>>>> -- >>>>> >>>>> Comment: >>>>> >>>>> This is what I get now >>>>> >>>>> {{{#!py >>>>> >>>>> from trac.web.href import Href >>>>>>>> h = Href('/x/y') >>>>>>>> p = dict(a=1,b=2,c=3) >>>>>>>> h(p) >>>>>>>> >>>>>>> '/x/y?a=1&c=3&b=2' >>>>> >>>>>> from multiproduct.hooks import ProductizedHref >>>>>>>> phref = ProductizedHref(h, 'z') >>>>>>>> phref(p) >>>>>>>> >>>>>>> 'z?a=1&c=3&b=2' >>>>> }}} >>>>> >>>>> Looks much better, but now that I take a look I'm not sure either of >>>>> whether that's the expected behavior. I just tried to fix the error and >>>>> stay a bit out of the way wrt semantics. I really only use >>>>> `ProductizedHref` in my local testing environment; never in production >>>>> (that I recall), so I guess **now** I'm not the right person to comment >>>>> >>>> on >>> >>>> this subject, beyond my comments above. I'm forwarding this review >>>>> >>>> request >>> >>>> to gjm . He might have a few ideas. >>>>> >>>>> From SVN annotate, I see that Jure committed the code we have >>>> questions >>>> about in r1453351. Jure, could you help us understand the intended >>>> >>> behavior >>> >>>> of ProductizedHref? >>>> >>> IIRC the ProductizedHref is a wrapper for Href to help with creation of >>> product-scoped URLs from any environment, be it a global, or another >>> product. >>> It's mainly used in the global dashboard's product widgets, so that hrefs >>> to >>> product specific milestones, versions etc. are readily available. >>> (The other use in ProductRequestWithFactory seems obsolete, except in >>> tests) >>> >>> Looking at recent fixes from Olemis (#552), it seems that ProductizedHref >>> may >>> not be needed anymore and could be removed (from tests as well)? >>> >> >> Ah, thanks. I hadn't looked to see that ProductRequestWithSession isn't >> used anywhere either. I will look at removing both ProductizedHref and >> ProductRequestWithSession after #552 is applied. >> >> The ProductizedHref is, as Matevz already mentioned, a wrapper that > encapsulates global Href and renders either product scoped or global (for > PATHS_NO_TRANSFORM, STATIC_PREFIXES) URLs. Creation of ProductizedHref is > controlled by ProductRequestWithSession. > > Example would be: > > from trac.web.href import Href > > from multiproduct.hooks import ProductizedHref > > global_href = Href('/') > > product_href = ProductizedHref(global_href, '/myproduct') > > product_href.admin(a=2, b=3) > '/admin?a=2&b=3' > > product_href.milestone(a=2, b=3) > '/myproduct/milestone?a=2&b=3' > > The use of ProductizedHref should be completely transparent as it's hidden > from the code through the ProductRequestFactory ... > > Cheers, > Jure Okay, now I see how this is set and used through the `[trac] request_factory` configuration setting. I was mainly confused by an example such as the following > global_href = Href('/base') > product_href = ProductizedHref(global_href, '/myproduct') > product_href.admin(a=2, b=3) '/base/admin?a=2&b=3' > product_href.milestone(a=2, b=3) '/myproduct/milestone?a=2&b=3' I expected the `ProductizedHref` to prepend the global href base so that the last string would be '/base/myproduct/milestone?a=2&b=3' By debugging through the code I see now that in practice the following values would be passed: > global_href = Href('/base') > product_href = ProductizedHref(global_href, '/base/myproduct') (i.e. the product base would *not* be relative to the global base) which leads to the result: > product_href.milestone(a=2, b=3) '/base/myproduct/milestone?a=2&b=3' I will add some unit tests and close out #579.
