Hello paco,
First, sorry if this post appears twice - this one is resent from
[EMAIL PROTECTED] (with minor edits), as my original mail didn't get
through after an hour...
John Hampton wrote:
> As is evident from previous messages, there is a big question regarding
> the whole "context" stuff.[1]
>
> I believe that the general consensus is to clean up the context stuff by
> refactoring it.[2]
>
> This post is to throw fuel on that fire, so that it doesn't languish in
> back alleys or get swept under the rug.
>
Thanks for providing this summary. As you probably figured out, that
document and more specifically my parts are quite in a
"work-in-progress" state, so there were probably contradictions, the
most notable one being about the use of req.perm.
I'll try to address some of your questions below, where you requested
some clarifications.
> ...
> That leaves us with two semi-related purposes of the Context object:
> 1. permissions
> 2. link rendering
>
> The basis for these is the idea of a resource descriptor. Each resource
> can be identified by a realm+identifier.
>
> ...
> cboos tends to disagree with the use of simple data structures due to
> that being more of a resource identifier, rather than resource
> descriptor [more about this when talking about link rendering] and the
> cumbersomeness of manipulating lists of tuples.
> ... However, I fail to see how the object
> will convey the hierarchy of a resource properly or easily.
>
resource.parent?
> = 1. Permissions =
>
> ...
> cboos raises the point that if the permission system isn't going to be
> tied to the request object, then there is no reason to use req.perm as
> the main entry point (other than backwards compatibility). He proposes
> the rendering context [see below about link rendering] as a possibilty
> for storing the permission info. Or creating a `User` object to wrap
> the req.authname and permission cache.
>
Well, actually the PermissionCache is all we need. The fact that it
is
originally attached to 'req' can be safely ignored ;-)
> = 2. Link Rendering =
>
> ...
>
> cboos rebuts:
> {{{
> We can't simply move all the responsibilities of the rendering context
> to the Formatter class, as Formatter objects are only present when
> explicitly rendering Wiki text, not generally available when rendering
> "content", in the mimeview module (the "content" which is rendered is
> coming from resources and that content may be wiki content). So we need
> something (a RenderingContext class?) that would be made available to
> the content renderers, and from there to the formatter, and from the
> formatter conveyed to the wiki processors.
> }}}
>
> This is where I get lost. What cmlenz proposes seems to make sense, and
> appears to the be simpler solution. I'll admit that I haven't really
> delved into those parts of the code. Why can't the formatter be made
> available to the mimeview stuff? If it's so tied to the wiki, why are
> we even trying to use it with the mimeview stuff (via Context, etc.)
> Does this tie into the Context Factories and Subclasses [see below]? I
> need some schooling here.
>
Quite simply, the wiki engine is layered on top of the mimeview
layer,
so the formatter can't be made available to the mimeview layer. OTOH,
there are various parts of the rendering engine that need to be able
to
know about the "resource" being manipulated (e.g. the blame stuff).
But I agree that at the time you were reading the proposal, the part
of
the RenderingContext was rather vague, and that's normal because I'm
working on that right now ;-)
> == 2a. Context Factories and Subclasses ==
> [note. this is much of the stuff where I get kind of lost, so I hope my
> summary isn't too far off the mark]
>
> Apparently, the these are used in the current Context stuff to:
> {{{
> * return the model object for a context
> * construct a URL to the resource identified by the context
> * provide appropriate "name", "shortname", and "summary" properties
> }}}
>
> Currently, the attachment module is the main user of this. It uses the
> Context object to render the name and URL of the parent resource.
>
> cmlenz proposes an IResourceManager interface to handle such things.[4]
>
> On this topic, cboos opines:
> {{{
> We need some kind of dynamic description of resources, for all the parts
> that are generic about resources in Trac: the attachments, the history
> and diffs of resource content (#2945), the generic reports, etc.
> Those things have been built some time after the WikiContext
> integration, so it's true that they are separate enhancements. But I
> think that they are very worthwhile ones that have to be kept and
> probably expanded upon (generic comments, content change annotation, etc.)
> If there's really a need to decouple the way resources can be identified
> from the way resources can be described, then we could eventually have
> simple Resource objects and "rendering" methods from a manager component
> like the one suggested. That would actually make some parts of the
> implementation simpler, as we could create very simple Resource objects
> which could delegate their dynamic aspects to their managing component.
> }}}
>
> I think this is agreeing with cmlenz (more or less). Cboos does propose
> a modified interface for IResourceManager[5]
>
> Now, I kind of like the idea of IResourceManager, but don't fully grasp
> the necessity. Given that we decide a list of tuples is the way to go
> for the resource descriptor, doesn't the attachment module already have
> all that it needs to generate links to the parent object?
>
> A resource for an attachment would be:
> {{{
> [('wiki', ('WikiStart', 1)), ('attachment', 'README.txt')]
> }}}
> so the attachment module knows what it's parent is.
>
> Can someone clarify where we need the whole IResourceManager/resource
> description stuff?
>
The attachment module has no idea about the conventions for rendering
a
page name or a ticket number.
For sure, the attachment page could display something like:
"Attachment
proposal.txt for wiki:WikiStart" or "Attachment sreenshot.jpg for
ticket:123", but it's nicer to display stuff like: "Attachment
proposal.txt for WikiStart" or "Attachment sreenshot.jpg for Ticket
#123", and for that we're simply using "parent.name" in the template.
Same thing for a summary, which for a ticket shows information about
title, state and resolution.
> = Appendix R - Resource Class =
>
> cboos's solution to osimons's security suggestion is to run all access
> to resources through req.perm. Basically add req.perm.resource() that
> would return the resource if permissions allowed, or raise an error (or
> return none in the case of req.perm.get_resource())
>
> Example usage:
> {{{
> page = req.perm.resource('wiki', 'WikiStart')
> wiki = req.perm.resource('wiki')
> wiki = req.perm.get_resource('wiki') # return None if not authorized
> }}}
>
> The proposal is a bit more extensive, but this has already been an epic.
>
"run all access to resources through req.perm" ... or already
available
resources.
> = Conclusion =
>
> Due to the nature of this beast, I'm inclined to say that less is more.
> I do like osimons's security idea, but I agree with cmlenz and aat
> that it's out of scope for 0.11.
I think what they stated to be out of the scope for 0.11 is a layer
of
the application logic which would take the permissions into account
(as
opposed to the all the permission checks at the web_ui level).
> Cboos's compromise doesn't look bad,
> but I think think it's something that would be too rushed for 0.11.
> Better leave it out and do it properly for 0.12 or 0.13
>
Well, the point is to discuss a way to do fine grained permission in
0.11 until we get it "right", so I don't see how this is rushed...
> I don't fully understand the issues surrounding the whole "resource
> description" vs "resource identification" stuff. I don't quite see the
> benefit to the IResourceManager interface. Why is it that we really
> need it in the first place?
>
I hope I clarified that in the attachment example above, but there
are
many other places where we manipulate Trac resources in a more or
less
generic way.
> I'm definitely for a very simple descriptor/identifier for resources.
> It makes security easy and flexible. Rendering stuff isn't needed there.
>
No, of course it isn't needed for security checks, but it's simply
convenient to be able to use an already available resource identifier
in
order to get a few simple informations out of it and even more
informations by retrieving its associated model. The name/shortname
etc.
properties are already delegating the rendering to the
IResourceManager
component (the resource.manager). An alternative approach would be to
have the resource identifier provide a resource descriptor (an object
more lightweight than the model), which could then be used to have
the
desired rendering (i.e. something like page.descriptor.name instead
of
just page.name), but I'm not sure this additional complexity is worth
it
I hope I answered your questions.
PS: I've just noticed
http://trac.edgewall.org/wiki/TracDev/ContextRefactoring - thanks for
doing the conversion! If there's a need to update the proposal, I
think this should be the authoritative place, now.
-- Christian
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
