Re: [CalendarServer-changes] [1345] CalendarServer/trunk/lib-patches/Twisted/twisted.web2.dav.resource. patch
It's true that resources looked up via the "fast path" in the directory hierarchy are not cached, but I don't know why this is broken. Only resources looked up through the request are cached, and anyone calling urlForResource() has to be aware of that. So don't call urlForResource() on a resource you obtained by other means. In the case of principal resources, call principalURL(). It has always worked this way. -wsv On Mar 8, 2007, at 8:53 AM, source_changes@macosforge.org wrote:
With the update to the new Twisted branch, urlForResource now raises an exception. We need to trap and ignore this. However, this whole urlForResource thing is now broken as resource can be found without going through locateResource when using the directory services thingy - lookups there do not depend on the request object and thus do not cache a urlForResource. This needs to be fixed one way or another.
One option would be to have urlForResource be a method on the DAVResource object that defaulted to calling the request implementation. Then we could override that on our directory related resources and have then return their "hard-coded" uris.
— Wilfredo Sánchez - wsanchez@wsanchez.net
Hi Wilfredo, --On March 9, 2007 9:50:46 AM -0800 Wilfredo Sánchez Vega <wsanchez@wsanchez.net> wrote:
It's true that resources looked up via the "fast path" in the directory hierarchy are not cached, but I don't know why this is broken.
Only resources looked up through the request are cached, and anyone calling urlForResource() has to be aware of that. So don't call urlForResource() on a resource you obtained by other means. In the case of principal resources, call principalURL(). It has always worked this way.
Except that some "core" functions need to use urlForResource and there is no way of knowing where those are. The problem I ran into was the principal-match report did the fast lookup of the principals, but then needed to return properties on those. One of the property lookup behaviors is to check quota values and that requires getting the parent for the current resource which means doing the urlForResource, parentForURL etc thing. That broke because there was no urlForResource entry for the fast lookup. The problem is how should I know that I have a fast looked up resource? All I did was ask for the group-membership set from a principal. BTW this was in Twisted code no caldav code, and Twisted certainly has no idea about fast lookup. That is why I think it is better if urlForResource is a method on the DAVResource class, and the resource then figures out whether to query the request or return a value it has cached (the fast case). We can then override that for our "fast" resources, whilst the default is to use the request. In fact urlForResource is used a lot as part of finding the parent resource. So I think we should also have a parentResource() method on DAVResource that encompasses the urlForResource etc stuff. Again, the fast resources can override that and return their parentdirectly (they have self.parent defined) without needing to do a locateResource on the parent URL. -- Cyrus Daboo
On Mar 9, 2007, at 10:13 AM, Cyrus Daboo wrote:
Except that some "core" functions need to use urlForResource and there is no way of knowing where those are. The problem I ran into was the principal-match report did the fast lookup of the principals, but then needed to return properties on those. One of the property lookup behaviors is to check quota values and that requires getting the parent for the current resource which means doing the urlForResource, parentForURL etc thing. That broke because there was no urlForResource entry for the fast lookup.
The problem is how should I know that I have a fast looked up resource? All I did was ask for the group-membership set from a principal. BTW this was in Twisted code no caldav code, and Twisted certainly has no idea about fast lookup.
If you didn't obtain it from the request, you can't call urlForResource() and expect it to work. If you got a resource via the group-membership lookup, then you did not get it from a request and therefore should not expect the request to know it's URL. In that specific case, you do know it's a principal, and that you can call principalURL() instead. The Twisted code in question is buggy is if makes the wrong assumption.
That is why I think it is better if urlForResource is a method on the DAVResource class, and the resource then figures out whether to query the request or return a value it has cached (the fast case). We can then override that for our "fast" resources, whilst the default is to use the request.
I think this encourages the broken assumptions I mentioned above, since resource.url() would only work in the same cases, except for certain types of resources. For most resources, it would break in the same way.
In fact urlForResource is used a lot as part of finding the parent resource. So I think we should also have a parentResource() method on DAVResource that encompasses the urlForResource etc stuff. Again, the fast resources can override that and return their parentdirectly (they have self.parent defined) without needing to do a locateResource on the parent URL.
Again, this isn't appropriate for DAVResource, most subclasses of which don't actually have the information needed to return the parent resource. url() and parent() are only feasible in a statically configured schema. We do have that for some of our resources, so it might make sense to have a mix-in class that has a faster ACL lookup implementation which uses the parent and url attributes instead of locateResource(), etc. But let's not add API to the base class which can't be implemented by all subclasses, which will certainly lead to buggy code. -wsv
participants (2)
-
Cyrus Daboo
-
Wilfredo Sánchez Vega