Edgewall Software

Changes between Version 5 and Version 6 of TracDev/ContextRefactoring


Ignore:
Timestamp:
Sep 27, 2007, 11:04:01 AM (12 years ago)
Author:
Christian Boos
Comment:

Updating my proposal: cut out some discussion coming from the intermediate stages and update the code sample with the latest status of the prototype

Legend:

Unmodified
Added
Removed
Modified
  • TracDev/ContextRefactoring

    v5 v6  
    150150 * [#ContextFactoriesandSubclasses Context Factories and Subclasses]:[[br]]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.[[br]]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.)[[br]]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.
    151151 * [#WikiRenderingContext Wiki Rendering Context]:[[br]]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. For example, the "blame" kind of annotation, now available for repository source file, later for other resources (#5821) is done at the mimeview layer, and doesn't depend on the wiki layer, so using a Formatter here is not appropriate. Another example of the use of a context which knowns about the "parent" context is to provide a way to guard against recursive content display. This is not yet that common for now, as the possibility to embed one content into another are limited so far (the !TicketQuery macro in table mode could lead to such a situation, though). So the `RenderingContext` is really something that belongs to the `trac.mimeview` layer and could be used in further developments to refactor/simplify that API as well (see #3332).
    152  * [#PermissionSystem Permission System]:[[br]] if the permission system is not to be bound to the req, then I see no good reason why we use req.perm as its main entry point (other than backward compatibility issues). The rendering context  seems to be a possible place to store this information (who is accessing the content?). Alternatively, we could introduce an User object, for now as simple as a wrapper for the authname and a place to store the authentification cache.
    153    * About [#osimons osimons] points 2. and 3.: I think this is an interesting proposal. In order to get a resource descriptor at all, the accessor should at least have read access to it. This could for example be enforced by requiring a mandatory user argument when creating a resource descriptor: `ResourceManager(self.env, user).ticket(42)`.[[br]]Also, regardless from how the `PermissionCache` is obtained (currently req.perm), I think using it as the principal way to produce the resource identifiers is a good idea.
    154    * In general, I'm fully for ditching out the req out of the picture, of both the resource descriptors (of course) and the rendering contexts. As osimons said, it would be really nice to be able to use the rendering stuff of Trac in an off-line context. We don't necessarily need the req in all of the rendering chain, but we do need some of the information so far only available from the req, most notably the user information (req.authname). Other important things that are currently stored in the Request is the Chrome data (the parts of the template data consumed by layout.html).
    155 
    156 Now, the above ideas have been confronted to some actual coding and I'm getting to the point of having a working prototype. Several details are still in need of additional work (especially the rendering context par), but I think I'm getting there.[[br]]
     152 * [#PermissionSystem Permission System]:[[br]]
     153   * About [#osimons osimons] points 2. and 3.: I think this is an interesting proposal. In order to get a resource descriptor at all, the accessor should at least have read access to it. I can't think of a reason when one should be able to manipulate a resource without having at least read permission. If that's ''really'' needed, that should still be possible, but it's all but the most common case. So the point would be to find a nice and simple way to create resource identifiers/descriptors for authorized resources. [[br]] The `PermissionCache` seems to be the best place to stick this convenient resource creation mechanism, for a couple of reasons:
     154     - there's already a `PermissionCache` is available in most place of the code where we already use it (`req.perm`),
     155     - it's convenient to create when there's no `req` available (`PermissionCache(env, username)`)
     156     - we're already using it for enforcing read permissions (`if 'WIKI_VIEW' in req.perm: ...`), even for fine-grained permissions (`if 'WIKI_VIEW' in req.perm('wiki', 'WikiStart'): ...`)
     157   * In general, I'm fully for ditching out the req out of the picture, of both the resource descriptors (of course) and the rendering contexts. As osimons said, it would be really nice to be able to use the rendering stuff of Trac in an off-line context. We don't necessarily need the req in all of the rendering chain, but we do need some of the information so far only available from the req, most notably the user information (req.authname). Other important things that are currently stored in the Request is the Chrome data (the parts of the template data consumed by layout.html). Overall, the current code base relies too much on the `req` object in the rendering process, so removing the request from the rendering should be left for later iterations. OTOH, the request is now removed from the resource and permission sides completely. See osimons point 1. and cmlenz first comment and paragraph in "Goals".
     158
     159
     160Now, the above ideas have been confronted to some actual coding and I'm getting to the point of having a working prototype (wiki and attachment module mostly done, others on the way).
    157161I think I've addressed cmlenz concerns and beyond this, I've taken osimons points further and actually try to use the permission system as the primary way to access the resources.
    158162
    159   '''Warning:''' ''work in progress -Christian Boos 9/24/07 4:31 PM''
     163  '''Warning:''' ''work in progress -Christian Boos 2007-09-27''
    160164
    161165=== The `Resource` class for resource identification ===
    162166A Resource object is used to uniquely identify some resource in a Trac environment.[[br]]
    163167The identification is based on realm, id and eventually version information. `Resource` objects are used throughout Trac in order to conveniently identify and manipulate resources, in a light-weight and convenient way. As fine-grained security and permission checks are becoming the rule in Trac, one shouldn't be able to access a resource without the appropriate credentials.[[br]]
    164 Therefore, resources have to be created by the permission system. More precisely, the `PermissionCache` object is used for creating  `Resource` objects. In turn, each Resource object let you access the permission cache, by the way of a perm propery. That way:
     168Therefore, resources have to be created by the permission system. More precisely, the `PermissionCache` object is used for creating  `Resource` objects. In turn, each Resource object let you access the permission cache, by the way of a perm property. That way:
    165169 * permission checks can be conveniently performed on a Resource (keeping the existing `PermissionCache` API).
    166170 * access to new resources can be performed starting from an existing resource, either by going through the resource's permission cache, or by using some convenience methods for specialized situations (other resources from the same realm, child resources).
    167171
    168 Let's illustrate the usage of resource objects. In the general case, all what is needed is the realm and id information:
    169 {{{
    170 #!python
    171     page = req.perm.resource('wiki', 'WikiStart')
     172
     173The syntax for creating resource objects can be exactly the one used up to now for checking fine grained permissions:
     174{{{
     175#!python
     176page = req.perm('wiki', 'WikiStart')
    172177}}}
    173178By default, the version information is set to None, specifying that we're interested in the latest version of the resource.[[br]]
     179In case there's no read permission for that resource, `None` is returned instead of an object.
     180
     181Alternatively, the equivalent to `req.perm.require('wiki', 'WikiStart')` can be achieved using:
     182{{{
     183#!python
     184page = req.perm.assert_resource('wiki', 'WikiStart')
     185}}}
     186Here, in case there's no read permission, a `PermissionError` exception is raised.
     187See [#osimons osimons] point 2.
     188
     189Note that if for some reason there's really a need to create a Resource object without the proper credentials, it's of course possible to do so, using directly the `Resource` constructor - simply it's not the "advised" way.
     190
     191In addition to the above, it should also be possible to duplicate resource objects using convenient ways (e.g. `page(version=req.args.get('version'))` in order to get a specific version of an already available resource). When doing this, the permissions are checked again for the newly created resource, and `None` is returned if there's no read permission. The alternative `copy()` method can be used if one would like to trigger a `PermissionError` instead ''(or should that be `assert_copy()` to mirror `assert_resource()`?)''.
     192
     193
    174194`Resource` can also be retrieved without specifying an explicit id. In that case, the resource can be used to refer to the realm as a whole.
    175195{{{
    176196#!python
    177     wiki = req.perm.resource('wiki')
    178 }}}
    179 Note that from this point, the Request object is put out of the picture for permission checks and is not otherwise used by resources.[[br]]
    180 (See osimons point 1. and cmlenz first comment and paragraph in "Goals")
    181 If the req.perm credentials don't provide the <REALM>_VIEW permission for the resource, it is simply not possible to build a `Resource` instance for the specified resource and the call will fail, triggering a `PermissionError`. (See osimons point 2.)[[br]]
    182 There's an alternate method, which can be used when one does not want to get a `PermissionError`, which will simply return `None` if there's no permission to access the resource (the "get" is reminiscent of the dictionary lookup get).
    183 {{{
    184 #!python
    185     wiki = req.perm.get_resource('wiki')
    186     if wiki:
    187         # do something with it
    188 }}}
     197wiki_realm = req.perm('wiki')
     198}}}
     199
    189200Beyond checks for read access which are done upon resource creation, any other permission check can be done afterwards on the resource, simply by using the perm property (a familiar `PermissionCache` object):
    190201{{{
     
    194205        continue
    195206}}}
    196 Any kind of primary resources could be created using a `PermissionCache`, obtained either from `req.perm` or `<someresource>.perm`, but in some cases there are even more convenient ways to create a new resource.[[br]]
    197 For example, one can create a resource by copying an existing one. This is convenient when one has only to change the id or the version of an existing resource identifier. So when staying within the same realm, resources can be created using the copy() method:
    198 {{{
    199 #!python
    200 
    201     versioned_page = page.copy(version=version)
    202 }}}
    203 Like for resource(), when dealing with a large number of resources one could use an alternative "get" form, in order to get the authorized resources without putting exceptions into play:
    204 {{{
    205 #!python
    206     pages = []
    207     for name in WikiSystem(env).get_all_pages(prefix):
    208         page = wiki.get_copy(id=name)
    209         if page:
    210             pages.append(page)
    211 }}}
    212 The `get_copy(id)` method will return `None` if there's no access right to resource with the given id.[[br]]
     207''('''Note:''' maybe we could even not expose the `perm` property, and write the above like that:)''
     208{{{
     209    page.require('WIKI_CREATE')
     210    if 'WIKI_ADMIN' in page:
     211        continue
     212}}}
     213
    213214Another situation where it's much more convenient to create a resource from an existing resource is when one wants to create a secondary resource, i.e. a resource which only exists within the scope of another resource, like an attachment:
    214215{{{
    215216#!python
    216     attachment = parent.get_child('attachment', filename)
    217 }}}
    218 
    219 After a few iterations of the prototype implementation, I finally came to something very similar to the `IResourceManager` interface from '''Context Factories and Subclasses''' chapter above, mainly for efficiency concerns and simplicity of the implementation. But for convenience purpose, the `Resource` class has still the `name`, `shortname` and `summary` properties, as well as a `model` property and an `url(href)` method. Those properties and method simply delegate the actual work to the resource's manager (i.e. the Component implementing the `IResourceManager ` interface):
     217    attachment = parent.child('attachment', filename)
     218}}}
     219''('''Note''': this will probably soon become `child()/assert_child()` for symmetry with the other methods)''
     220
     221After a few iterations of the prototype implementation, I finally came to something very similar to the `IResourceManager` interface described in the [#ContextFactoriesandSubclasses Context Factories and Subclasses] section above, mainly for efficiency concerns and simplicity of the implementation. But for convenience purpose, the `Resource` class has still the `name`, `shortname` and `summary` properties, as well as a `model` property and an `url(href)` method. Those properties and method simply delegate the actual work to the resource's manager which is the Component implementing the `IResourceManager ` interface which has claimed to manage the resource's realm (see `get_resource_realms()` below):
    220222{{{
    221223#!python