MONDRIAN: Fix MONDRIAN-694, "Incorrect handling of child/parent relat…

…ionship

    with hierarchy grants". Problem was that the tuple reader was not giving
    the 'all' member an access-control wrapper.

[git-p4: depot-paths = "//open/mondrian-release/3.2/": change = 13554]

demo/FoodMart.xml

@@ -506,7 +506,7 @@ Iif("sales_fact_1997"."promotion_id" = 0, 0, "sales_fact_1997"."store_sales")
   <Dimension name="Department" foreignKey="department_id">
     <Hierarchy hasAll="true" primaryKey="department_id">
       <Table name="department"/>
-      <Level name="Department Description" uniqueMembers="true"
+      <Level name="Department Description" type="Numeric" uniqueMembers="true"
           column="department_id"/>
     </Hierarchy>
   </Dimension>

src/main/mondrian/rolap/RolapCubeHierarchy.java

@@ -1147,6 +1147,10 @@ public MemberCache getMemberCache() {
         public Object getMemberCacheLock() {
             return memberCacheLock;
         }
+
+        public RolapMember allMember() {
+            return getHierarchy().getAllMember();
+        }
     }
 }
 

src/main/mondrian/rolap/SqlMemberSource.java

@@ -1036,6 +1036,13 @@ public RolapMember makeMember(
         return member;
     }
 
+    public RolapMember allMember() {
+        final RolapHierarchy rolapHierarchy =
+            hierarchy instanceof RolapCubeHierarchy
+                ? ((RolapCubeHierarchy) hierarchy).getRolapHierarchy()
+                : hierarchy;
+        return rolapHierarchy.getAllMember();
+    }
 
     /**
      * <p>Looks up an object (and if needed, stores it) in a cached value pool.

src/main/mondrian/rolap/SqlTupleReader.java

@@ -140,14 +140,7 @@ int internalAddRow(SqlStatement stmt, int column)
                 for (int i = 0; i <= levelDepth; i++) {
                     RolapLevel childLevel = levels[i];
                     if (childLevel.isAll()) {
-                        member = level.getHierarchy().getAllMember();
-                        if (memberBuilder instanceof SqlMemberSource
-                            && !(memberBuilder
-                            instanceof
-                            RolapCubeHierarchy.RolapCubeSqlMemberSource))
-                        {
-                            member = strip(member);
-                        }
+                        member = memberBuilder.allMember();
                         continue;
                     }
                     RolapMember parentMember = member;
@@ -1203,12 +1196,6 @@ void setMaxRows(int maxRows) {
         this.maxRows = maxRows;
     }
 
-    private static RolapLevel strip(RolapLevel level) {
-        return level instanceof RolapCubeLevel
-            ? ((RolapCubeLevel) level).getRolapLevel()
-            : level;
-    }
-
     private static RolapMember strip(RolapMember member) {
         return member instanceof RolapCubeMember
             ? ((RolapCubeMember) member).getRolapMember()

src/main/mondrian/rolap/SubstitutingMemberReader.java

@@ -277,6 +277,10 @@ public RolapMember makeMember(
                     key,
                     column));
         }
+
+        public RolapMember allMember() {
+            return substitute(memberReader.getHierarchy().getAllMember());
+        }
     }
 }
 

src/main/mondrian/rolap/Target.java

@@ -17,12 +17,8 @@
 import java.util.*;
 
  /**
- * <p>
- * helper class for HighCardSqlTupleReader
- *  {@link mondrian.rolap.HighCardSqlTupleReader}
- * Keeps track of target levels and constraints for adding to sql query
- *
- * </p>
+ * Helper class for {@link mondrian.rolap.HighCardSqlTupleReader} that
+ * keeps track of target levels and constraints for adding to SQL query.
  *
  * @author luis f. canals, Kurtis Walker
  * @since July 23, 2009
@@ -52,6 +48,8 @@ public Target(
 
     public void open() {
         levels = (RolapLevel[]) level.getHierarchy().getLevels();
+        // REVIEW: ArrayDeque is preferable to LinkedList (JDK1.6 and up) but it
+        //   doesn't implement List, so we can't easily interoperate the two.
         setList(new LinkedList<RolapMember>());
         levelDepth = level.getDepth();
         parentChild = level.isParentChild();
@@ -69,7 +67,7 @@ int internalAddRow(SqlStatement stmt, int column)
             for (int i = 0; i <= levelDepth; i++) {
                 RolapLevel childLevel = levels[i];
                 if (childLevel.isAll()) {
-                    member = level.getHierarchy().getAllMember();
+                    member = memberBuilder.allMember();
                     continue;
                 }
 
@@ -245,4 +243,3 @@ public void remove() {
 }
 
 // End Target.java
-

src/main/mondrian/rolap/TupleReader.java

@@ -59,6 +59,13 @@ RolapMember makeMember(
             Object key,
             int column)
             throws SQLException;
+
+        /**
+         * Returns the 'all' member of the hierarchy.
+         *
+         * @return The 'all' member
+         */
+        RolapMember allMember();
     }
 
     /**

testsrc/main/mondrian/test/AccessControlTest.java

@@ -1717,6 +1717,92 @@ public void testBugMondrian622() {
 //      System.out.println("Elapsed=" + (t2 - t1) + " millis");
 //      System.out.println("RoleImpl.accessCount=" + RoleImpl.accessCallCount);
     }
+
+    /**
+     * Test case for bug
+     * <a href="http://jira.pentaho.com/browse/MONDRIAN-694">MONDRIAN-694,
+     * "Incorrect handling of child/parent relationship with hierarchy
+     * grants"</a>.
+     */
+    public void testBugMondrian694() {
+        final TestContext testContext =
+            TestContext.create(
+                null, null, null, null, null,
+                "<Role name=\"REG1\"> \n"
+                + "  <SchemaGrant access=\"none\"> \n"
+                + "    <CubeGrant cube=\"HR\" access=\"all\"> \n"
+                + "      <HierarchyGrant hierarchy=\"Employees\" access=\"custom\" rollupPolicy=\"partial\"> \n"
+                + "        <MemberGrant member=\"[Employees].[All Employees]\" access=\"none\"/>\n"
+                + "        <MemberGrant member=\"[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Cody Goldey].[Shanay Steelman].[Steven Betsekas]\" access=\"all\"/> \n"
+                + "        <MemberGrant member=\"[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Cody Goldey].[Shanay Steelman].[Arvid Ziegler]\" access=\"all\"/> \n"
+                + "        <MemberGrant member=\"[Employees].[Sheri Nowmer].[Derrick Whelply].[Laurie Borges].[Cody Goldey].[Shanay Steelman].[Ann Weyerhaeuser]\" access=\"all\"/> \n"
+                + "      </HierarchyGrant> \n"
+                + "    </CubeGrant> \n"
+                + "  </SchemaGrant> \n"
+                + "</Role>")
+                .withRole("REG1");
+
+        // With bug MONDRIAN-694 returns 874.80, should return 79.20.
+        // Test case is minimal: doesn't happen without the Crossjoin, or
+        // without the NON EMPTY, or with [Employees] as opposed to
+        // [Employees].[All Employees], or with [Department].[All Departments].
+        testContext.assertQueryReturns(
+            "select NON EMPTY {[Measures].[Org Salary]} ON COLUMNS,\n"
+            + "NON EMPTY Crossjoin({[Department].[14]}, {[Employees].[All Employees]}) ON ROWS\n"
+            + "from [HR]",
+            "Axis #0:\n"
+            + "{}\n"
+            + "Axis #1:\n"
+            + "{[Measures].[Org Salary]}\n"
+            + "Axis #2:\n"
+            + "{[Department].[14], [Employees].[All Employees]}\n"
+            + "Row #0: $97.20\n");
+
+        // This query gave the right answer, even with MONDRIAN-694.
+        testContext.assertQueryReturns(
+            "select NON EMPTY {[Measures].[Org Salary]} ON COLUMNS, \n"
+            + "NON EMPTY Hierarchize(Crossjoin({[Department].[14]}, {[Employees].[All Employees], [Employees].Children})) ON ROWS \n"
+            + "from [HR] ",
+            "Axis #0:\n"
+            + "{}\n"
+            + "Axis #1:\n"
+            + "{[Measures].[Org Salary]}\n"
+            + "Axis #2:\n"
+            + "{[Department].[14], [Employees].[All Employees]}\n"
+            + "{[Department].[14], [Employees].[Sheri Nowmer]}\n"
+            + "Row #0: $97.20\n"
+            + "Row #1: $97.20\n");
+
+        // Original test case, not quite minimal. With MONDRIAN-694, returns
+        // $874.80 for [All Employees].
+        testContext.assertQueryReturns(
+            "select NON EMPTY {[Measures].[Org Salary]} ON COLUMNS, \n"
+            + "NON EMPTY Hierarchize(Union(Crossjoin({[Department].[All Departments].[14]}, {[Employees].[All Employees]}), Crossjoin({[Department].[All Departments].[14]}, [Employees].[All Employees].Children))) ON ROWS \n"
+            + "from [HR]  ",
+            "Axis #0:\n"
+            + "{}\n"
+            + "Axis #1:\n"
+            + "{[Measures].[Org Salary]}\n"
+            + "Axis #2:\n"
+            + "{[Department].[14], [Employees].[All Employees]}\n"
+            + "{[Department].[14], [Employees].[Sheri Nowmer]}\n"
+            + "Row #0: $97.20\n"
+            + "Row #1: $97.20\n");
+
+        testContext.assertQueryReturns(
+            "select NON EMPTY {[Measures].[Org Salary]} ON COLUMNS, \n"
+            + "NON EMPTY Crossjoin(Hierarchize(Union({[Employees].[All Employees]}, [Employees].[All Employees].Children)), {[Department].[14]}) ON ROWS \n"
+            + "from [HR] ",
+            "Axis #0:\n"
+            + "{}\n"
+            + "Axis #1:\n"
+            + "{[Measures].[Org Salary]}\n"
+            + "Axis #2:\n"
+            + "{[Employees].[All Employees], [Department].[14]}\n"
+            + "{[Employees].[Sheri Nowmer], [Department].[14]}\n"
+            + "Row #0: $97.20\n"
+            + "Row #1: $97.20\n");
+    }
 }
 
 // End AccessControlTest.java