MONDRIAN-LAGUNITAS: Fix MemberCache (members were identified by key w…

…ithin

    parent, now identified by possibly-compositive key within level); and fix
    access-control. Now 1123 exceptions out of 3045 tests.

[git-p4: depot-paths = "//open/mondrian-release/lagunitas/": change = 14595]

demo/NewFoodMart.xml

@@ -202,9 +202,6 @@
                 <Hierarchy name='Store Size in SQFT'>
                     <Level attribute='Store Sqft'/>
                 </Hierarchy>
-                <Hierarchy name='Store Size in SQFT'>
-                    <Level attribute='Store Sqft'/>
-                </Hierarchy>
                 <Hierarchy name='Store Type'>
                     <Level attribute='Store Type'/>
                 </Hierarchy>

src/main/mondrian/rolap/CacheControlImpl.java

@@ -1483,7 +1483,8 @@ public void visit(RolapMember member) {
             member = stripMember(member);
             final MemberCache memberCache = getMemberCache(member);
             final Object cacheKey = member.getKeyCompact();
-            final RolapMember cacheMember = memberCache.getMember(cacheKey);
+            final RolapMember cacheMember =
+                memberCache.getMember(member.getLevel(), cacheKey);
             if (cacheMember == null) {
                 return;
             }
@@ -1544,7 +1545,7 @@ private void deleteMember(
         // Remove the member itself. The MemberCacheHelper takes care of
         // removing the member's children as well.
         final Object key = member.getKeyCompact();
-        memberCache.removeMember(key);
+        memberCache.removeMember(member.getLevel(), key);
 
         // Cells for member and its ancestors are now invalid.
         // It's sufficient to flush the member.
@@ -1610,7 +1611,7 @@ private void addMember(
 
         // Now add the member itself into cache
         final Object memberKey = member.getKeyCompact();
-        memberCache.putMember(memberKey, member);
+        memberCache.putMember(member.getLevel(), memberKey, member);
 
         // Cells for all of member's ancestors are now invalid. It's sufficient
         // to flush its parent.
@@ -1629,7 +1630,7 @@ private void flushMember(
     {
         final MemberCache memberCache = getMemberCache(member);
         final Object key = member.getKeyCompact();
-        memberCache.removeMember(key);
+        memberCache.removeMember(member.getLevel(), key);
         cellRegionList.add(createMemberRegion(member, false));
     }
 }

src/main/mondrian/rolap/CacheMemberReader.java

@@ -18,6 +18,7 @@
 import mondrian.rolap.TupleReader.MemberBuilder;
 import mondrian.rolap.sql.TupleConstraint;
 import mondrian.rolap.sql.MemberChildrenConstraint;
+import mondrian.util.Pair;
 
 /**
  * <code>CacheMemberReader</code> implements {@link MemberReader} by reading
@@ -37,16 +38,21 @@ class CacheMemberReader implements MemberReader, MemberCache {
     /**
      * Looks up a member by its key. The key is an object if non-composite, a
      * list if composite.
+     *
+     * <p>REVIEW: Might be more memory efficient to have a two-level map (that
+     * is, a map for each level) rather than using Pair as a compound key. Also,
+     * we can use an IdentityHashMap for levels, which should be faster.</p>
      */
-    private final Map<Object, RolapMember> mapKeyToMember;
+    private final Map<Pair<RolapLevel, Object>, RolapMember> mapKeyToMember;
 
     CacheMemberReader(MemberSource source) {
         this.source = source;
         if (false) {
             // we don't want the reader to write back to our cache
             Util.discard(source.setCache(this));
         }
-        this.mapKeyToMember = new HashMap<Object, RolapMember>();
+        this.mapKeyToMember =
+            new HashMap<Pair<RolapLevel, Object>, RolapMember>();
         this.members = source.getMembers();
         for (int i = 0; i < members.size(); i++) {
             RolapMember member = RolapUtil.strip(members.get(i));
@@ -79,26 +85,21 @@ public List<RolapMember> getMembers() {
     }
 
     // implement MemberCache
-    public Object makeKey(RolapMember parent, Object[] key) {
-        return key.length == 1 ? key[0] : Arrays.asList(key);
+    public RolapMember getMember(RolapLevel level, Object key) {
+        return mapKeyToMember.get(Pair.of(level, key));
     }
 
-    // implement MemberCache
-    public Object makeKey(RolapMember parent, List<Object> key) {
-        return new MemberKey(parent, key);
-    }
-
-    // implement MemberCache
-    public RolapMember getMember(Object key) {
-        return mapKeyToMember.get(key);
-    }
-    public RolapMember getMember(Object key, boolean mustCheckCacheStatus) {
-        return mapKeyToMember.get(key);
+    public RolapMember getMember(
+        RolapLevel level,
+        Object key,
+        boolean mustCheckCacheStatus)
+    {
+        return mapKeyToMember.get(Pair.of(level, key));
     }
 
     // implement MemberCache
-    public Object putMember(Object key, RolapMember value) {
-        return mapKeyToMember.put(key, value);
+    public Object putMember(RolapLevel level, Object key, RolapMember value) {
+        return mapKeyToMember.put(Pair.of(level, key), value);
     }
 
     // don't need to implement this MemberCache method because we're never
@@ -117,12 +118,12 @@ public boolean isMutable()
         return false;
     }
 
-    public RolapMember removeMember(Object key)
+    public RolapMember removeMember(RolapLevel level, Object key)
     {
         throw new UnsupportedOperationException();
     }
 
-    public RolapMember removeMemberAndDescendants(Object key)
+    public RolapMember removeMemberAndDescendants(RolapLevel level, Object key)
     {
         throw new UnsupportedOperationException();
     }

src/main/mondrian/rolap/MemberCache.java

@@ -30,34 +30,40 @@ interface MemberCache {
     /**
      * Retrieves the {@link RolapMember} with a given key.
      *
-     * @param key cache key, created by {@link #makeKey}
+     * @param level Level
+     * @param key Member key
      * @return member with a given cache key
      */
-    RolapMember getMember(Object key);
+    RolapMember getMember(RolapLevel level, Object key);
 
     /**
      * Retrieves the {@link RolapMember} with a given key.
      *
-     * @param key cache key, created by {@link #makeKey}
+     * @param level Level
+     * @param key Member key
      * @param mustCheckCacheStatus If {@code true}, do not check cache status
      * @return member with a given cache key
      */
-    RolapMember getMember(Object key, boolean mustCheckCacheStatus);
+    RolapMember getMember(
+        RolapLevel level,
+        Object key,
+        boolean mustCheckCacheStatus);
 
     /**
      * Replaces the {@link RolapMember} with a given key and returns the
      * previous member if any.
      *
-     * @param key cache key, created by {@link #makeKey}
+     * @param level Level
+     * @param key Member key
      * @param member new member
      * @return Previous member with that key, or null.
      */
-    Object putMember(Object key, RolapMember member);
+    Object putMember(RolapLevel level, Object key, RolapMember member);
 
     /**
      * Returns whether the cache supports removing selected items. If it does,
-     * it is valid to call the {@link #removeMember(Object)} and
-     * {@link #removeMemberAndDescendants(Object)} methods.
+     * it is valid to call the {@link #removeMember(RolapLevel, Object)} and
+     * {@link #removeMemberAndDescendants(RolapLevel, Object)} methods.
      *
      * <p>REVIEW: remove isMutable and move removeMember and
      * removeMemberAndDescendants to new interface MutableMemberCache
@@ -71,20 +77,22 @@ interface MemberCache {
      * Returns the previous member with that key, or null.
      * Optional operation: see {@link #isMutable}.
      *
-     * @param key cache key, created by {@link #makeKey}
+     * @param level Level
+     * @param key Member key
      * @return previous member with that key, or null
      */
-    RolapMember removeMember(Object key);
+    RolapMember removeMember(RolapLevel level, Object key);
 
     /**
      * Removes the designated {@link RolapMember} and all its descendants.
      * Returns the previous member with that key, or null.
      * Optional operation: see {@link #isMutable}.
      *
-     * @param key cache key, created by {@link #makeKey}
+     * @param level Level
+     * @param key Member key
      * @return previous member with that key, or null
      */
-    RolapMember removeMemberAndDescendants(Object key);
+    RolapMember removeMemberAndDescendants(RolapLevel level, Object key);
 
     /**
      * Returns the children of <code>member</code> if they are currently in the

src/main/mondrian/rolap/MemberCacheHelper.java

@@ -37,7 +37,7 @@ public class MemberCacheHelper implements MemberCache {
         mapMemberToChildren;
 
     /** a cache for all members to ensure uniqueness */
-    SmartCache<Object, RolapMember> mapKeyToMember;
+    SmartCache<Pair<RolapLevel, Object>, RolapMember> mapKeyToMember;
     RolapHierarchy rolapHierarchy;
     DataSourceChangeListener changeListener;
 
@@ -55,7 +55,7 @@ public MemberCacheHelper(RolapHierarchy rolapHierarchy) {
         this.mapLevelToMembers =
             new SmartMemberListCache<RolapLevel, List<RolapMember>>();
         this.mapKeyToMember =
-            new SoftSmartCache<Object, RolapMember>();
+            new SoftSmartCache<Pair<RolapLevel, Object>, RolapMember>();
         this.mapMemberToChildren =
             new SmartMemberListCache<RolapMember, List<RolapMember>>();
 
@@ -70,36 +70,30 @@ public MemberCacheHelper(RolapHierarchy rolapHierarchy) {
     // implement MemberCache
     // synchronization: Must synchronize, because uses mapKeyToMember
     public synchronized RolapMember getMember(
+        RolapLevel level,
         Object key,
         boolean mustCheckCacheStatus)
     {
         if (mustCheckCacheStatus) {
             checkCacheStatus();
         }
-        return mapKeyToMember.get(key);
+        return mapKeyToMember.get(Pair.of(level, key));
     }
 
 
     // implement MemberCache
     // synchronization: Must synchronize, because modifies mapKeyToMember
-    public synchronized Object putMember(Object key, RolapMember value) {
-        return mapKeyToMember.put(key, value);
-    }
-
-    // implement MemberCache
-    public Object makeKey(RolapMember parent, Object[] key) {
-        return new MemberKey(parent, key);
-    }
-
-    // implement MemberCache
-    public Object makeKey(RolapMember parent, List<Object> key) {
-        return new MemberKey(parent, key);
+    public synchronized Object putMember(
+        RolapLevel level,
+        Object key,
+        RolapMember value)
+    {
+        return mapKeyToMember.put(Pair.of(level, key), value);
     }
 
     // implement MemberCache
-    // synchronization: Must synchronize, because modifies mapKeyToMember
-    public synchronized RolapMember getMember(Object key) {
-        return getMember(key, true);
+    public final RolapMember getMember(RolapLevel level, Object key) {
+        return getMember(level, key, true);
     }
 
     public synchronized void checkCacheStatus() {
@@ -178,9 +172,9 @@ public boolean isMutable()
         return true;
     }
 
-    public synchronized RolapMember removeMember(Object key)
+    public synchronized RolapMember removeMember(RolapLevel level, Object key)
     {
-        RolapMember member = getMember(key);
+        RolapMember member = getMember(level, key);
         if (member == null) {
             // not in cache
             return null;
@@ -191,9 +185,8 @@ public synchronized RolapMember removeMember(Object key)
         // cache value is a list of RolapMember.
         // For each cache key whose level matches, remove from the list,
         // regardless of the constraint.
-        RolapLevel level = member.getLevel();
-        for (Map.Entry<Pair<RolapLevel, Object>,
-            List<RolapMember>> entry : mapLevelToMembers.getCache())
+        for (Map.Entry<Pair<RolapLevel, Object>, List<RolapMember>> entry
+            : mapLevelToMembers.getCache())
         {
             if (entry.getKey().left.equals(level)) {
                 List<RolapMember> peers = entry.getValue();
@@ -236,10 +229,13 @@ public synchronized RolapMember removeMember(Object key)
         }
 
         // drop it from the lookup-cache
-        return mapKeyToMember.put(key, null);
+        return mapKeyToMember.put(Pair.of(level, key), null);
     }
 
-    public synchronized RolapMember removeMemberAndDescendants(Object key) {
+    public RolapMember removeMemberAndDescendants(
+        RolapLevel level,
+        Object key)
+    {
         // Can use mapMemberToChildren recursively. No need to update inferior
         // lists of children. Do need to update inferior lists of level-peers.
         return null; // STUB

src/main/mondrian/rolap/MemberNoCacheHelper.java

@@ -11,12 +11,9 @@
 */
 package mondrian.rolap;
 
-import mondrian.rolap.cache.SmartCache;
-import mondrian.rolap.cache.SoftSmartCache;
 import mondrian.rolap.sql.MemberChildrenConstraint;
 import mondrian.rolap.sql.TupleConstraint;
 import mondrian.spi.DataSourceChangeListener;
-import mondrian.olap.Util;
 
 import java.util.*;
 
@@ -35,6 +32,7 @@ public MemberNoCacheHelper() {
 
     // implement MemberCache
     public RolapMember getMember(
+        RolapLevel level,
         Object key,
         boolean mustCheckCacheStatus)
     {
@@ -43,16 +41,10 @@ public RolapMember getMember(
 
 
     // implement MemberCache
-    public Object putMember(Object key, RolapMember value) {
+    public Object putMember(RolapLevel level, Object key, RolapMember value) {
         return value;
     }
 
-    // implement MemberCache
-    // synchronization: Must synchronize, because modifies mapKeyToMember
-    public synchronized RolapMember getMember(Object key) {
-        return getMember(key, true);
-    }
-
     public void checkCacheStatus() {
     }
 
@@ -103,11 +95,7 @@ public boolean isMutable() {
         return true;
     }
 
-    public synchronized RolapMember removeMember(Object key) {
-        return null;
-    }
-
-    public synchronized RolapMember removeMemberAndDescendants(Object key) {
+    public synchronized RolapMember removeMember(RolapLevel level, Object key) {
         return null;
     }
 }