Hey Christian,
Fantastic work cleaning this stuff up, looks like it was a lot of work.
I've only looked at the Resource class itself, and have some comments:
- In general I think having four different ways of creating resources
is both redundant and confusing:
- Resource(realm, id, version)
- Resource.from_spec(realm, id, version)
- Resource.from_spec(resource)
- Resource.from_spec((realm, id, version)).
This is extreme overkill IMO.
The tuple form in particular can be replaced with argument expansion
(*mytuple).
From looking at the usage of from_spec() it seems that the sole
reason for its existence is as a convenience so you can pass in a
resource *or* a realm and have a Resource object returned.
Is it a big deal if we put this logic into the constructor? The only
difference will be that it will always return a new object rather
than sometimes returning the resource object passed in.
This would leave us with:
- Resource(realm, id, version)
- Resource(resource)
Which I think is a lot cleaner and simpler.
- I'd prefer it if Resource.__call__() was renamed to clone, duplicate or
copy or something.
Alec
On 10/30/07, Christian Boos <[EMAIL PROTECTED]> wrote:
>
> Hi all,
>
> After much debate and tweaking, the context-refactoring should now be in
> a good shape for merging on trunk.
>
> For those who have missed the (sometimes heated) discussions of the past
> months, here's what it is about. During 0.11dev, I introduced a
> `Context` class which was meant to hold all the necessary information
> for rendering "content". To that end, this object encapsulated the
> commonly used env, req and db objects. A context was also indicating
> which Trac "resource" was being worked on, by the way of .realm, .id and
> .version properties. There were a few methods that could be used to
> describe that resource as well, like for getting the name or the url of
> that resource. In addition, contexts could be nested (have a parent
> context). Finally, the fine grained permission scheme introduced in
> 0.11dev was using the context in order to grant different permissions
> for individual resources.
>
> Now there were some concerns that this design was a bit crufty, and I
> agreed to rework this into something more consensual: a Context class
> focusing on the rendering aspects and a Resource class focusing on the
> identification of the resource.
>
> The context doesn't contain unnecessary information anymore (no env, no
> db) and only keeps the req because at that point there's no way to do
> without. The longer term goal (hopefully 0.12) is to have a rendering
> layer _hence the Context_ be completely independent from the web Request
> object. The context is still associated to a resource by the way of a
> .resource property, which is a Resource object, so that among other
> things, relative links in the rendered wiki text can work.
>
> A resource object bundles together .realm (like 'wiki', 'ticket', etc.),
> .id (unique identifier within a realm) and .version (a number or None
> for the latest), as well as a .parent resource in case of dependent
> resources (typical and only example for now, the resources in the
> 'attachment' realm). There's no way to get to the underlying data model
> object implicitly; if you need it, you'll have to instantiate explicitly
> the appropriate model class for the given resource. Also, all the helper
> methods for getting the url, name etc. of a resource in a generic way
> have been replaced by helper functions taking an env and a resource
> (there are more convenient name_of(resource, url_of(resource) helper
> functions available in the templates, though). Finally, the fine grained
> permission scheme is now using exclusively the resources for doing the
> permission checks. A typical fined grained permission check can now be
> done by writing something like:
>
> req.perm(ticket.resource).require('TICKET_VIEW')
>
> where ticket is the model object for some ticket.
>
> There's no trac.context module anymore. The new Context class is now
> located in trac.mimeview. The new Resource class, along with the
> ResourceNotFound exception, IResourceManager interface and the
> ResourceSystem, is located in trac.resource.
>
> The full set of changes can be examined using the following diff URL,
> but be warned that this is a somewhat big set of changes...
>
> http://trac.edgewall.org/anydiff?new_path=%2Fsandbox%2Fcontext-refactoring&old_path=%2Fsandbox%2Fcontext-refactoring&new_rev=6117&old_rev=6117
>
> Things are certainly not yet 100% polished at this point, but the branch
> did receive considerable attention, so almost everything should work (or
> is it's a bug ;-) ).
>
>
> Now, once that branch is in trunk, the timeline refactoring will follow
> (I'll write a second mail about that one, but it should be much less
> contentious) and after the most critical remaining bugs for 0.11 are
> committed as well (#5025 and #2048), I propose that we release a
> 0.11beta1 and upgrade t.e.o to it. The advantage is that this will
> provide some clarity when talking about the 0.11dev API: pre-beta1 will
> be the old trac.context.Context and trac.timeline.TimelineEvent APIs,
> and post-beta1 will be the new trac.resource.Resource and
> trac.mimeview.Context ones. Therefore, a plugin would be either "beta1"
> compatible or not.
>
> -- Christian
>
>
> >
>
--
Evolution: Taking care of those too stupid to take care of themselves.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
