> Complex non-obvious objects with unbounded storage options should not
> be passed as part of APIs. Even 'req' fails this test because we
> cannot faithfully recreate all parts and variations outside a request
> context. So let's instead gather the bits we all should depend on and
> that would be the minimum set needed to do useful work in all
> contexts. perm, user, resource and href is a good start, and pass each
> as named arguments and set to None if unavailable. In a pinch I would
> settle for using a well-defined OfflineRequest with essential data and
> passed as 'req'.
IMO, passing context data (such as perm, user, resource and href) as events
args brings a few problems:
- That will bloat interfaces, complicates logic and introduce a lot of
repeatable code because this parameter(s) have to be sent almost to each
method and be part of almost each interface.
- As you said, it defines a minimum set. What if a plugin requires some
data out of this minimum?
- That breaks Single Responsibility Principle
(http://en.wikipedia.org/wiki/Single_responsibility_principle) - a class
have to be aware of context data even if it is not required by the class
functionality.
- obtaining backward compatibility brings additional effort
I suggest the different approach:
Trac ComponentManager already provides component resolving functionality
with the only supported visibility scope - singleton per environment.
ComponentManager could go a step forward and support additional scopes
(lifestyles) e.g. per request, transient etc.
Let's take an example: a plugin wants to get a source IP (usename,
authentication type... you name it) on ticket change event - that is
currently not a simple trick because the request parameter is not a part of
event method.
What can be done: at the begging of web request handling, Trac obtains an
instance of a WebRequest component from ComponentManager (OfflineRequest
for trac-admin tool) and fill the appropriate data. The WebRequest class
should be marked as PerRequest:
{{{
@lifestyle(PerRequest)
class WebRequest(Components):
implements(IRequest)
#request data is here
....
}}}
ComponentManager should know how to track components with PerRequest
lifestyle. For examle, ComponentManager can use thread local context for
this (thread local context should be cleared at the end of the request).
The plugin implementation could look like:
{{{
class SampleTicketChangeListener(Component):
implements(ITicketChangeListener)
def ticket_created(ticket):
#get_component returns an instance of a component that implements
IRequest interface. Because WebRequest implements IRequest declares and it's
# lifestyle is PerRequest the result will be the instance of
WebRequest bounded to the current request.
source_ip = self.env.get_component(IRequest).source_ip
#... plugin's logic here
}}}
Does it all make sense?
This is not a small change but functionality can be added incrementally
without breaking backward compatibility.
Cheers, Andrej
--
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/trac-dev?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
