List Info

Thread: context-refactoring sandbox branch




context-refactoring sandbox branch
country flaguser name
United States
2007-10-16 04:07:10
Hello,

Yesterday I committed the first version of the context
refactoring,
in a new branch located at:

http://trac.edgewall.org/browser/sandbox/context-ref
actoring

Excerpt from the changeset comment:
----
The main idea here is to remove from the `Context` class
all
the aspects related to resource identification and
description,
and to keep only what is needed for doing the rendering.

In effect, the main focus of that `Context` class is now to
be
exclusively a "rendering context", which aims at
providing all
the necessary information for the rendering layer,
ultimately
allowing us to get rid on the dependency on the `req` in
this
part of the code.


The resource identification part is now the responsibility
of a
new `Resource` class, which is quite simple (no
subclassing)
and mainly offers a convenient access to the `.realm`,
`.id`
and `.version` information.

The fine grained permissions are now operating exclusively
on
the `Resource` objects, not on `Context` objects anymore.
----

The initial change is pretty large, but there would have
been
no point in splitting that in smaller changesets, as the
changes are mostly all interdependent. It is also quite
large
because I think I covered all the areas and everything
should
work OK.

The context.py was mostly rewritten and now contains the
"new"
Context class and the Resource class and related
components:

http://trac.edgewall.org/br
owser/sandbox/context-refactoring/trac/context.py?rev=6057

== The Context class ==
In earlier versions of the refactoring, I had the Context
class
renamed to RenderingContext and moved into
trac/mimeview/api.py, but this was artificially breaking
compatibility with earlier 0.11 code. The only major
difference
is now that there's no `.db` field anymore, and of course
the
fact that the resource information is now delegated to a
Resource - but creating a Context and passing it around to
formatters for example is kept the same.

The Context class is now focused on holding together the
information needed by the various content renderers and the
wiki formatters. The idea is to remove the need for having
to
provide the req to this object, but we're not at that point
yet, for several reasons: the "layout" based
templates
currently depends on the req for storing chrome data, the
0.10
style wiki macros still depend on the req.path_info for
knowing
about the resource being addressed, the rendering layer
uses
the request for sending data directly back to the user,
etc.

== The Resource class ==
The Resource class is on the other hand focusing on
identifying
Trac resources, i.e. any typed and identified content
within
the system.  I removed the possibility to access the model
from
the resource, as in all the places that were using this
possibility, it was equally simple to create directly the
model
and conversely there's now the possibility to query the
`.resource` from the model objects. That way, we don't have
to
pass around two objects (e.g. the permissions checks can now
be
something like `'WIKI_VIEW' in perm(page.resource)` and
doing
things in this order is indeed less verbose in the end.

The Resource class itself could perhaps be simplified
further,
like changeset r6056 did for the .url could also be done
for
the .name, .shortname and .summary - I've not gone to the
point
 to remove the class completely, as I still think that
having the
` __call__` and `child` methods, plus the possibility to
access
directly the `.realm`, `.id`, `.version` and `.parent`
information
without having to resort to tuple and list manipulations is
a
must.

The usage of resources in a generic way in templates can be
seen
in the diff_view.html, history_view.html and the
attachment.html
files and more generally in the other files touched by
r6057.


In summary, there's still room for improvements, but I
think
it was worth checkpointing my work in progress at that
stage
and ask for some feedback.

-- Christian


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Trac Development" group.
To post to this group, send email to trac-devgooglegroups.com
To unsubscribe from this group, send email to
trac-dev-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---


Re: context-refactoring sandbox branch
user name
2007-10-16 14:37:59
On 10/16/07, chboosgooglemail.com <chboosgooglemail.com> wrote:
> The Resource class itself could perhaps be simplified
further,
> like changeset r6056 did for the .url could also be
done for
> the .name, .shortname and .summary - I've not gone to
the point
>  to remove the class completely, as I still think that
having the
> ` __call__` and `child` methods, plus the possibility
to access
> directly the `.realm`, `.id`, `.version` and `.parent`
information
> without having to resort to tuple and list
manipulations is a
> must.

I agree--having a simple class for Resources is still easier
to deal
with and less error-prone than a list of tuples, and also
easier to
extend in the future, if necessary, without breaking
everything that
uses it.  Having one more small class of objects being
passed around
is not really going to hurt anything, I don't think.

Erik

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Trac Development" group.
To post to this group, send email to trac-devgooglegroups.com
To unsubscribe from this group, send email to
trac-dev-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---


Re: context-refactoring sandbox branch
country flaguser name
United States
2007-10-18 11:44:44
Just a little follow-up note:

On Oct 16, 11:07 am, "chb...googlemail.com"
<chb...googlemail.com>
wrote:
> == The Resource class ==
> The Resource class is on the other hand focusing on
identifying
> Trac resources, i.e. any typed and identified content
within
> the system.  I removed the possibility to access the
model from
> the resource,
...
> The Resource class itself could perhaps be simplified
further,
> like changeset r6056 did for the .url could also be
done for
> the .name, .shortname and .summary

... and this is what I did in the changesets that followed,
among other little bug fixes.
r6067 went as far as to remove the env from the Resource
class.
So things look like they're as simple as possible now.
One positive side-effect of this is that it's pretty simple
now
to identify the generic  uses of resources, by greping for
get_name
et al.

Testing feedback on r6068 welcomed.

-- Christian


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Trac Development" group.
To post to this group, send email to trac-devgooglegroups.com
To unsubscribe from this group, send email to
trac-dev-unsubscribegooglegroups.com
For more options, visit this group at http://
groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---


[1-3]

about | contact  Other archives ( Real Estate discussion Medical topics )