On 6. Jun, 2013, at 14:09, Gary Martin wrote: > On 04/06/13 14:37, [email protected] 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
