You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bloodhound.apache.org by ma...@apache.org on 2013/06/04 15:37:49 UTC
svn commit: r1489442 -
/bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
Author: matevz
Date: Tue Jun 4 13:37:49 2013
New Revision: 1489442
URL: http://svn.apache.org/r1489442
Log:
Ref. #402 - PRODUCT_VIEW permission was missing for 'anonymous' and 'authenticated' users
Modified:
bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
Modified: bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py?rev=1489442&r1=1489441&r2=1489442&view=diff
==============================================================================
--- bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py (original)
+++ bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py Tue Jun 4 13:37:49 2013
@@ -369,6 +369,9 @@ class MultiProductSystem(Component):
# - populate system tables with global configuration for each product
# - exception is permission table where permissions
# are also populated in global scope
+ #
+ # permission table specifics: 'anonymous' and 'authenticated' users
+ # should by default have a PRODUCT_VIEW permission for all products
self.log.info("Migrating system tables to a new schema")
for table in self.MIGRATE_TABLES:
if table == 'wiki':
@@ -379,11 +382,24 @@ class MultiProductSystem(Component):
table, product.name, product.prefix)
db("""INSERT INTO %s (%s, product) SELECT %s,'%s' FROM %s""" %
(table, cols, cols, product.prefix, temp_table_name))
+ if table == 'permission':
+ db.executemany(
+ """INSERT INTO permission (username, action, product)
+ VALUES (%s, %s, %s)""",
+ [('anonymous', 'PRODUCT_VIEW', product.prefix),
+ ('authenticated', 'PRODUCT_VIEW', product.prefix)])
+
if table == 'permission':
self.log.info("Populating table '%s' for global scope", table)
db("""INSERT INTO %s (%s, product) SELECT %s,'%s' FROM %s""" %
(table, cols, cols, '', temp_table_name))
self._drop_temp_table(db, temp_table_name)
+ db.executemany(
+ """INSERT INTO permission (username, action, product)
+ VALUES (%s, %s, %s)""",
+ [('anonymous', 'PRODUCT_VIEW', ''),
+ ('authenticated', 'PRODUCT_VIEW', '')])
+
def _upgrade_wikis(self, db, create_temp_table):
# migrate wiki table
Re: svn commit: r1489442 - /bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
Posted by Olemis Lang <ol...@gmail.com>.
On 6/7/13, Matevž Bradač <ma...@digiverse.si> wrote:
> On 6. Jun, 2013, at 14:09, Gary Martin wrote:
[...]
>>
>> I'm happy to correct the test but I was also wondering if the logic was
>> correct: given that in this case TICKET_VIEW is only given to 'x', should
>> we assume PRODUCT_VEW should be provided to 'anonymous' and
>> 'authenticated'? Would it be more appropriate to match the users that
>> TICKET_VIEW was provided to for the initial setup?
>
> I think so for this test case - since the test deletes all permissions
> first, and
> then adds TICKET_VIEW for a specific user, the same should be done for
> PRODUCT_VIEW
> as well. As for the anonymous/authenticated users, I gather the PRODUCT_VIEW
> should
> be removed after running the upgrade.
> Olemis, would this be the correct approach?
>
On upgrades I'd rather say that *_VIEW permissions do matter . I mean
if e.g. user 'x' is granted with only WIKI_VIEW permission before
upgrade then user should still be able to read wiki pages , and
thereby deserves PRODUCT_VIEW permission (at least) in default product
.
For that particular tests , well ... I advocate to assert system
behavior . It seems to me that including PRODUCT_VIEW for x in
permissions table beforehand will obscure an interesting testing
scenario that should be handled by the upgrade process itself, as
mentioned in preceding paragraph .
--
Regards,
Olemis.
Re: svn commit: r1489442 - /bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
Posted by Matevž Bradač <ma...@digiverse.si>.
On 6. Jun, 2013, at 14:09, Gary Martin wrote:
> On 04/06/13 14:37, matevz@apache.org wrote:
>> Author: matevz
>> Date: Tue Jun 4 13:37:49 2013
>> New Revision: 1489442
>>
>> URL: http://svn.apache.org/r1489442
>> Log:
>> Ref. #402 - PRODUCT_VIEW permission was missing for 'anonymous' and 'authenticated' users
>>
>> Modified:
>> bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
>>
>> Modified: bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
>> URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py?rev=1489442&r1=1489441&r2=1489442&view=diff
>> ==============================================================================
>> --- bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py (original)
>> +++ bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py Tue Jun 4 13:37:49 2013
>> @@ -369,6 +369,9 @@ class MultiProductSystem(Component):
>> # - populate system tables with global configuration for each product
>> # - exception is permission table where permissions
>> # are also populated in global scope
>> + #
>> + # permission table specifics: 'anonymous' and 'authenticated' users
>> + # should by default have a PRODUCT_VIEW permission for all products
>> self.log.info("Migrating system tables to a new schema")
>> for table in self.MIGRATE_TABLES:
>> if table == 'wiki':
>> @@ -379,11 +382,24 @@ class MultiProductSystem(Component):
>> table, product.name, product.prefix)
>> db("""INSERT INTO %s (%s, product) SELECT %s,'%s' FROM %s""" %
>> (table, cols, cols, product.prefix, temp_table_name))
>> + if table == 'permission':
>> + db.executemany(
>> + """INSERT INTO permission (username, action, product)
>> + VALUES (%s, %s, %s)""",
>> + [('anonymous', 'PRODUCT_VIEW', product.prefix),
>> + ('authenticated', 'PRODUCT_VIEW', product.prefix)])
>> +
>> if table == 'permission':
>> self.log.info("Populating table '%s' for global scope", table)
>> db("""INSERT INTO %s (%s, product) SELECT %s,'%s' FROM %s""" %
>> (table, cols, cols, '', temp_table_name))
>> self._drop_temp_table(db, temp_table_name)
>> + db.executemany(
>> + """INSERT INTO permission (username, action, product)
>> + VALUES (%s, %s, %s)""",
>> + [('anonymous', 'PRODUCT_VIEW', ''),
>> + ('authenticated', 'PRODUCT_VIEW', '')])
>> +
>> def _upgrade_wikis(self, db, create_temp_table):
>> # migrate wiki table
>>
>>
>
> Hi,
>
> Looks like this might have caused a test to fail:
Good catch, I missed that one.
>
>> FAIL: test_upgrade_copies_content_of_system_tables_to_all_products (tests.upgrade.EnvironmentUpgradeTestCase)
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>> File "/home/buildslave25/slave25/bh-unit-tests/build/bloodhound_multiproduct/tests/upgrade.py", line 197, in test_upgrade_copies_content_of_system_tables_to_all_products
>> % (table, len(rows), 7, rows))
>> AssertionError: Wrong number of lines in permission (21 instead of 7)
>> [(u'x', u'TICKET_VIEW', u'p1'), (u'anonymous', u'PRODUCT_VIEW', u'p1'), (u'authenticated', u'PRODUCT_VIEW', u'p1'), (u'x', u'TICKET_VIEW', u'p2'), (u'anonymous', u'PRODUCT_VIEW', u'p2'), (u'authenticated', u'PRODUCT_VIEW', u'p2'), (u'x', u'TICKET_VIEW', u'p3'), (u'anonymous', u'PRODUCT_VIEW', u'p3'), (u'authenticated', u'PRODUCT_VIEW', u'p3'), (u'x', u'TICKET_VIEW', u'p4'), (u'anonymous', u'PRODUCT_VIEW', u'p4'), (u'authenticated', u'PRODUCT_VIEW', u'p4'), (u'x', u'TICKET_VIEW', u'p5'), (u'anonymous', u'PRODUCT_VIEW', u'p5'), (u'authenticated', u'PRODUCT_VIEW', u'p5'), (u'x', u'TICKET_VIEW', u'@'), (u'anonymous', u'PRODUCT_VIEW', u'@'), (u'authenticated', u'PRODUCT_VIEW', u'@'), (u'x', u'TICKET_VIEW', u''), (u'anonymous', u'PRODUCT_VIEW', u''), (u'authenticated', u'PRODUCT_VIEW', u'')]
>
> I'm happy to correct the test but I was also wondering if the logic was correct: given that in this case TICKET_VIEW is only given to 'x', should we assume PRODUCT_VEW should be provided to 'anonymous' and 'authenticated'? Would it be more appropriate to match the users that TICKET_VIEW was provided to for the initial setup?
I think so for this test case - since the test deletes all permissions first, and
then adds TICKET_VIEW for a specific user, the same should be done for PRODUCT_VIEW
as well. As for the anonymous/authenticated users, I gather the PRODUCT_VIEW should
be removed after running the upgrade.
Olemis, would this be the correct approach?
>
> Cheers,
> Gary
Re: svn commit: r1489442 - /bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
Posted by Gary Martin <ga...@wandisco.com>.
On 04/06/13 14:37, matevz@apache.org wrote:
> Author: matevz
> Date: Tue Jun 4 13:37:49 2013
> New Revision: 1489442
>
> URL: http://svn.apache.org/r1489442
> Log:
> Ref. #402 - PRODUCT_VIEW permission was missing for 'anonymous' and 'authenticated' users
>
> Modified:
> bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
>
> Modified: bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py
> URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py?rev=1489442&r1=1489441&r2=1489442&view=diff
> ==============================================================================
> --- bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py (original)
> +++ bloodhound/trunk/bloodhound_multiproduct/multiproduct/api.py Tue Jun 4 13:37:49 2013
> @@ -369,6 +369,9 @@ class MultiProductSystem(Component):
> # - populate system tables with global configuration for each product
> # - exception is permission table where permissions
> # are also populated in global scope
> + #
> + # permission table specifics: 'anonymous' and 'authenticated' users
> + # should by default have a PRODUCT_VIEW permission for all products
> self.log.info("Migrating system tables to a new schema")
> for table in self.MIGRATE_TABLES:
> if table == 'wiki':
> @@ -379,11 +382,24 @@ class MultiProductSystem(Component):
> table, product.name, product.prefix)
> db("""INSERT INTO %s (%s, product) SELECT %s,'%s' FROM %s""" %
> (table, cols, cols, product.prefix, temp_table_name))
> + if table == 'permission':
> + db.executemany(
> + """INSERT INTO permission (username, action, product)
> + VALUES (%s, %s, %s)""",
> + [('anonymous', 'PRODUCT_VIEW', product.prefix),
> + ('authenticated', 'PRODUCT_VIEW', product.prefix)])
> +
> if table == 'permission':
> self.log.info("Populating table '%s' for global scope", table)
> db("""INSERT INTO %s (%s, product) SELECT %s,'%s' FROM %s""" %
> (table, cols, cols, '', temp_table_name))
> self._drop_temp_table(db, temp_table_name)
> + db.executemany(
> + """INSERT INTO permission (username, action, product)
> + VALUES (%s, %s, %s)""",
> + [('anonymous', 'PRODUCT_VIEW', ''),
> + ('authenticated', 'PRODUCT_VIEW', '')])
> +
>
> def _upgrade_wikis(self, db, create_temp_table):
> # migrate wiki table
>
>
Hi,
Looks like this might have caused a test to fail:
> FAIL: test_upgrade_copies_content_of_system_tables_to_all_products (tests.upgrade.EnvironmentUpgradeTestCase)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/home/buildslave25/slave25/bh-unit-tests/build/bloodhound_multiproduct/tests/upgrade.py", line 197, in test_upgrade_copies_content_of_system_tables_to_all_products
> % (table, len(rows), 7, rows))
> AssertionError: Wrong number of lines in permission (21 instead of 7)
> [(u'x', u'TICKET_VIEW', u'p1'), (u'anonymous', u'PRODUCT_VIEW', u'p1'), (u'authenticated', u'PRODUCT_VIEW', u'p1'), (u'x', u'TICKET_VIEW', u'p2'), (u'anonymous', u'PRODUCT_VIEW', u'p2'), (u'authenticated', u'PRODUCT_VIEW', u'p2'), (u'x', u'TICKET_VIEW', u'p3'), (u'anonymous', u'PRODUCT_VIEW', u'p3'), (u'authenticated', u'PRODUCT_VIEW', u'p3'), (u'x', u'TICKET_VIEW', u'p4'), (u'anonymous', u'PRODUCT_VIEW', u'p4'), (u'authenticated', u'PRODUCT_VIEW', u'p4'), (u'x', u'TICKET_VIEW', u'p5'), (u'anonymous', u'PRODUCT_VIEW', u'p5'), (u'authenticated', u'PRODUCT_VIEW', u'p5'), (u'x', u'TICKET_VIEW', u'@'), (u'anonymous', u'PRODUCT_VIEW', u'@'), (u'authenticated', u'PRODUCT_VIEW', u'@'), (u'x', u'TICKET_VIEW', u''), (u'anonymous', u'PRODUCT_VIEW', u''), (u'authenticated', u'PRODUCT_VIEW', u'')]
I'm happy to correct the test but I was also wondering if the logic was
correct: given that in this case TICKET_VIEW is only given to 'x',
should we assume PRODUCT_VEW should be provided to 'anonymous' and
'authenticated'? Would it be more appropriate to match the users that
TICKET_VIEW was provided to for the initial setup?
Cheers,
Gary