Merge pull request #7401 from nadvornik/channel_access

Optimize org channel accessibility query (bsc#1211874)
uyuni-project · Aug 15, 2023 · a0b2ff6 · a0b2ff6
2 parents ada05c7 + cb6ed8b
commit a0b2ff6
Show file tree

Hide file tree

Showing 3 changed files with 148 additions and 6 deletions.
diff --git a/java/code/src/com/redhat/rhn/domain/channel/Channel.hbm.xml b/java/code/src/com/redhat/rhn/domain/channel/Channel.hbm.xml
@@ -600,12 +600,40 @@ PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN"
     </sql-query>
 
     <sql-query name="Channel.isAccessibleBy">
-        <return-scalar type="int" column="count"/>
-        SELECT COUNT(*) AS count
-          FROM rhnChannel c
-          JOIN rhnAvailableChannels ac ON c.id = ac.channel_id
-          WHERE c.label = :channel_label
-            AND ac.org_id = :org_id
+        <return-scalar type="int" column="result"/>
+         <![CDATA[SELECT case when (EXISTS (
+              SELECT 1
+              FROM rhnChannel c
+              JOIN rhnChannelFamilyMembers cfm ON cfm.channel_id = c.id
+              JOIN rhnPrivateChannelFamily pcf ON pcf.channel_family_id = cfm.channel_family_id
+              WHERE c.label = :channel_label
+              AND pcf.org_id = :org_id
+              LIMIT 1
+        ) OR EXISTS (
+              SELECT 1
+              FROM rhnChannel c
+              JOIN rhnChannelFamilyMembers cfm ON cfm.channel_id = c.id
+              JOIN rhnPublicChannelFamily pcf ON pcf.channel_family_id = cfm.channel_family_id
+              WHERE c.label = :channel_label
+              LIMIT 1
+        ) OR EXISTS (
+              SELECT 1
+              FROM rhnChannel c
+              JOIN rhnTrustedOrgs tr ON c.org_id = tr.org_id
+              WHERE c.parent_channel IS NULL AND c.channel_access = 'public'
+              AND c.label = :channel_label
+              AND tr.org_trust_id = :org_id
+              LIMIT 1
+        ) OR EXISTS (
+              SELECT 1
+              FROM rhnChannel c
+              JOIN rhnChannelTrust tr ON c.id = tr.channel_id
+              WHERE c.channel_access = 'protected'
+              AND c.label = :channel_label
+              AND tr.org_trust_id = :org_id
+              LIMIT 1
+        )) then 1 else 0 end AS result
+        ]]>
     </sql-query>
 
     <sql-query name="Channel.isAccessibleByUser">

diff --git a/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFactoryTest.java b/java/code/src/com/redhat/rhn/domain/channel/test/ChannelFactoryTest.java
@@ -545,6 +545,119 @@ public void testAccessibility() throws Exception {
         assertFalse(ChannelFactory.isAccessibleByUser(c.getLabel(), user3.getId()));
     }
 
+    /**
+     * Test org channel accessibility
+     *
+     * @throws Exception if anything goes wrong
+     */
+    @Test
+    public void testOrgAccessibility() throws Exception {
+        User user1 = UserTestUtils.findNewUser("testuser1", "testorg1");
+        User user2 = UserTestUtils.findNewUser("testuser2", "testorg2");
+        User user3 = UserTestUtils.findNewUser("testuser3", "testorg3");
+        User user4 = UserTestUtils.findNewUser("testuser4", "testorg4");
+        Org org1 = user1.getOrg();
+        Org org2 = user2.getOrg();
+        Org org3 = user3.getOrg();
+        Org org4 = user4.getOrg();
+
+
+        Channel c1 = ChannelFactoryTest.createTestChannel(user1);
+        Channel c2 = ChannelFactoryTest.createTestChannel(user2);
+
+        assertTrue(ChannelFactory.isAccessibleBy(c1.getLabel(), org1.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org2.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org3.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org4.getId()));
+
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org1.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org2.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org3.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org4.getId()));
+
+        ChannelFamily privcfam = ChannelFamilyFactoryTest.createTestChannelFamily(user3, false);
+        ChannelFamily pubcfam = ChannelFamilyFactoryTest.createTestChannelFamily(user4, true);
+
+        c1.setChannelFamily(privcfam);
+        TestUtils.saveAndFlush(c1);
+
+        c2.setChannelFamily(pubcfam);
+        TestUtils.saveAndFlush(c2);
+
+        // c1 belongs to user3 org now
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org1.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org2.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c1.getLabel(), org3.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org4.getId()));
+
+        // c2 is public now
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org1.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org2.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org3.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org4.getId()));
+    }
+
+    /**
+     * Test trusted org channel accessibility
+     *
+     * @throws Exception if anything goes wrong
+     */
+    @Test
+    public void testTrustedOrgAccessibility() throws Exception {
+        User user1 = UserTestUtils.findNewUser("testuser1", "testorg1");
+        User user2 = UserTestUtils.findNewUser("testuser2", "testorg2");
+        User user3 = UserTestUtils.findNewUser("testuser3", "testorg3");
+        User user4 = UserTestUtils.findNewUser("testuser4", "testorg4");
+        Org org1 = user1.getOrg();
+        Org org2 = user2.getOrg();
+        Org org3 = user3.getOrg();
+        Org org4 = user4.getOrg();
+
+        Channel c1 = ChannelFactoryTest.createTestChannel(user1);
+        Channel c2 = ChannelFactoryTest.createTestChannel(user2);
+
+        assertTrue(ChannelFactory.isAccessibleBy(c1.getLabel(), org1.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org2.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org3.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org4.getId()));
+
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org1.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org2.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org3.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org4.getId()));
+
+        // trusted org added to org
+        org1.getTrustedOrgs().add(org3);
+        c1.setAccess(Channel.PUBLIC);
+        flushAndEvict(org1);
+        flushAndEvict(c1);
+
+        assertTrue(ChannelFactory.isAccessibleBy(c1.getLabel(), org1.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org2.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c1.getLabel(), org3.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org4.getId()));
+
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org1.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org2.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org3.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org4.getId()));
+
+        // trusted org added to channel
+        c2.getTrustedOrgs().add(org4);
+        c2.setAccess(Channel.PROTECTED);
+        flushAndEvict(c2);
+
+        assertTrue(ChannelFactory.isAccessibleBy(c1.getLabel(), org1.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org2.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c1.getLabel(), org3.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c1.getLabel(), org4.getId()));
+
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org1.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org2.getId()));
+        assertFalse(ChannelFactory.isAccessibleBy(c2.getLabel(), org3.getId()));
+        assertTrue(ChannelFactory.isAccessibleBy(c2.getLabel(), org4.getId()));
+    }
+
     /**
      * Test "ChannelFactory.findAllByUserOrderByChild"
      * @throws Exception

diff --git a/java/spacewalk-java.changes.nadvornik.channel_access b/java/spacewalk-java.changes.nadvornik.channel_access
@@ -0,0 +1 @@
+- Optimize org channel accessibility query (bsc#1211874)