On 1/28/13, Jure Zitnik <[email protected]> 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.
