incubator-bloodhound-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Olemis Lang <ole...@gmail.com>
Subject Re: [Apache Bloodhound] #288: Create prototype for legacy database schema proxy
Date Mon, 28 Jan 2013 18:28:17 GMT
On 1/28/13, Jure Zitnik <jure@digiverse.si> wrote:
> On 1/25/13 7:05 PM, Apache Bloodhound wrote:
>>   > One note though, the permission test should include product column
>> value
>>   when inserting into global environment. But you might wait with that
>> till
>>   after my patch...
>>
>>   IMO it's quite important to be consistent with the previous approach
>> (i.e.
>>   no explicit reference to products in SQL queries ). All this should be
>>   encapsulated e.g. the proxy has to detect whether a product is not set
>> in
>>   context and still translate the query considering `product = NULL` (
>>   `product = ''` ? ) . All instances of `product = NULL` will refer to
>>   resources in the global environment . That would be enough to keep the
>>   interface exactly the same as before .
>>
>>   I don't know how much complicated this will be for you to add this
>> feature
>>   , but IMO it's a **MUST** . Is it feasible up to this point ?
>>
>>   Everything **must** be just like before for both product envs and
>> global
>>   env . That's exactly the point .
>>   ''';)'''
>
> I don't find 'product detection' code very appealing. Assuming that
> detecting the product column within the INSERT statement would be
> relatively easy to do, detecting WHERE conditions is not that trivial.
>

I'm aware that there are some subtle details to consider , yes ...

> Therefor I would suggest the following solution - the code should always
> run within ProductEnvironment (and never global Environment).

-1 ... if only widgets and theme is actually needed then multi-product
plugin may be disabled and upgrades skipped .

> In the
> global context (URL namespace), the code should get the
> ProductEnvironment with the `product == ''`, which would reference
> global resources. This would keep the backward compatibility for 3rd
> party plugins/trac code even when running in global context - with the
> database view limited to 'global product'.
>

I think of this a different way . IMO the global environment should be
the usual instance of `trac.env.Environment` class .

However in either case there's a subtle argument to consider and it is
that the contract of Environment class spans beyond the traditional
situation considering attributes , methods , etc ... It also encloses
the fact that components running on top of it will expect an
underlying DB schema to be installed .

Your suggestion consists in breaking that part of the contract at the
exact point where it's always worked and implement a whole new concept
sticking to it as before . In spite making components functional with
little or no modifications by keeping them agnostic to multi-product
enhancements , that approach IMO does not play well .

If not convinced yet I'll mention one such use case . In previous
threads we discussed the fact that site admin *must only* be performed
in global scope and only a subset of those tasks should also be
allowed for product admin . That excludes e.g. environment upgrade
tasks , and also means that checks for TRAC_ADMIN permission might
only be valid in global scope .

The other side of the same coin is that permissions are asserted
against the global env . Those checks are performed by one of those
components that should be muti-product agnostic (e.g.
`DefaultPermissionStore`) instantiated at global environment scope
(i.e. `trac.env.Environment` ). The SQL queries in component source
code are exactly the same as those used for product environments
themselves i.e. no reference to product prefix or alike . At that time
you'll realize that they will fail (like it happens nowadays in perm
test cases, that's what it is for ;) and the Bloodhound instance will
be unusable .

The same reasoning is valid for components implementing
IEnvironmentSetupParticipant because they'll only be instantiated at
global scope (i.e. `trac.env.Environment` ) ... and so on .

[...]
> The translator code would still be disabled
> when accessing database through env.parent, making it possible for the
> code/plugins to access all product's resources.
>

IMO , we'll have to figure out another to get this done rather than
breaking the global environment .

-- 
Regards,

Olemis.

Mime
View raw message