[infinispan-commits] Infinispan SVN: r2560 - branches/4.2.x/core/src/main/java/org/infinispan/commands/read and 6 other directories.

infinispan-commits at lists.jboss.org infinispan-commits at lists.jboss.org
Thu Oct 21 13:16:33 EDT 2010


Author: trustin
Date: 2010-10-21 13:16:32 -0400 (Thu, 21 Oct 2010)
New Revision: 2560

Modified:
   branches/4.2.x/core/src/main/java/org/infinispan/CacheDelegate.java
   branches/4.2.x/core/src/main/java/org/infinispan/commands/read/AbstractLocalCommand.java
   branches/4.2.x/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java
   branches/4.2.x/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java
   branches/4.2.x/core/src/main/java/org/infinispan/commands/read/SizeCommand.java
   branches/4.2.x/core/src/main/java/org/infinispan/commands/read/ValuesCommand.java
   branches/4.2.x/core/src/main/java/org/infinispan/container/DefaultDataContainer.java
   branches/4.2.x/core/src/test/java/org/infinispan/api/CacheAPITest.java
   trunk/core/src/main/java/org/infinispan/CacheDelegate.java
   trunk/core/src/main/java/org/infinispan/commands/read/AbstractLocalCommand.java
   trunk/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java
   trunk/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java
   trunk/core/src/main/java/org/infinispan/commands/read/SizeCommand.java
   trunk/core/src/main/java/org/infinispan/commands/read/ValuesCommand.java
   trunk/core/src/main/java/org/infinispan/container/DefaultDataContainer.java
   trunk/core/src/test/java/org/infinispan/api/CacheAPITest.java
Log:
Fixed issue: ISPN-708 (cache.entrySet does not work correctly within a transaction)
* Reimplemented SizeCommand, KeySetCommand, ValuesCommand, EntrySetCommand
* Implemented DefaultDataContainer.keySet/entrySet.contains() properly so that a user don't get an unexpected result
* Fixed CacheAPITest.rollback*() tests which had incorrect assertions
* Removed AbstractLocalCommand.getKeySetWithinTransaction() which is not used anymore

Modified: branches/4.2.x/core/src/main/java/org/infinispan/CacheDelegate.java
===================================================================
--- branches/4.2.x/core/src/main/java/org/infinispan/CacheDelegate.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ branches/4.2.x/core/src/main/java/org/infinispan/CacheDelegate.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -21,6 +21,21 @@
  */
 package org.infinispan;
 
+import static org.infinispan.context.Flag.*;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import javax.transaction.Transaction;
+import javax.transaction.TransactionManager;
+
 import org.infinispan.batch.BatchContainer;
 import org.infinispan.commands.CommandsFactory;
 import org.infinispan.commands.control.LockControlCommand;
@@ -37,7 +52,6 @@
 import org.infinispan.commands.write.ReplaceCommand;
 import org.infinispan.config.Configuration;
 import org.infinispan.config.ConfigurationException;
-import org.infinispan.config.ConfigurationValidatingVisitor;
 import org.infinispan.container.DataContainer;
 import org.infinispan.container.entries.InternalCacheEntry;
 import org.infinispan.context.Flag;
@@ -73,20 +87,6 @@
 import org.rhq.helpers.pluginAnnotations.agent.Metric;
 import org.rhq.helpers.pluginAnnotations.agent.Operation;
 
-import javax.transaction.Transaction;
-import javax.transaction.TransactionManager;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.EnumSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
-
-import static org.infinispan.context.Flag.*;
-
 /**
  * @author Mircea.Markus at jboss.com
  * @author Galder Zamarreño
@@ -106,7 +106,7 @@
    protected TransactionManager transactionManager;
    protected RpcManager rpcManager;
    protected StreamingMarshaller marshaller;
-   private String name;
+   private final String name;
    private EvictionManager evictionManager;
    private DataContainer dataContainer;
    private static final Log log = LogFactory.getLog(CacheDelegate.class);
@@ -116,7 +116,7 @@
    // as above for ResponseGenerator
    private ResponseGenerator responseGenerator;
    private DistributionManager distributionManager;
-   private ThreadLocal<PreInvocationContext> flagHolder = new ThreadLocal<PreInvocationContext>();
+   private final ThreadLocal<PreInvocationContext> flagHolder = new ThreadLocal<PreInvocationContext>();
 
    public CacheDelegate(String name) {
       this.name = name;
@@ -151,17 +151,24 @@
       this.responseGenerator = responseGenerator;
       this.stateTransferManager = stateTransferManager;
       this.icc = icc;
-      this.distributionManager = distributionManager; 
+      this.distributionManager = distributionManager;
    }
 
    private void assertKeyNotNull(Object key) {
-      if (key == null) throw new NullPointerException("Null keys are not supported!");
+      if (key == null) {
+         throw new NullPointerException("Null keys are not supported!");
+      }
    }
 
    private void assertKeysNotNull(Map<?, ?> data) {
-      if (data == null) throw new NullPointerException("Expected map cannot be null");
-      for (Object key: data.keySet())
-         if (key == null) throw new NullPointerException("Null keys are not supported!");
+      if (data == null) {
+         throw new NullPointerException("Expected map cannot be null");
+      }
+      for (Object key: data.keySet()) {
+         if (key == null) {
+            throw new NullPointerException("Null keys are not supported!");
+         }
+      }
    }
 
    public final boolean remove(Object key, Object value) {
@@ -229,7 +236,7 @@
    @SuppressWarnings("unchecked")
    public Set<Map.Entry<K, V>> entrySet() {
       EntrySetCommand command = commandsFactory.buildEntrySetCommand();
-      return (Set<Map.Entry<K, V>>) invoker.invoke(icc.createNonTxInvocationContext(), command);
+      return (Set<Map.Entry<K, V>>) invoker.invoke(getInvocationContext(false), command);
    }
 
    public final void putForExternalRead(K key, V value) {
@@ -242,11 +249,15 @@
          withFlags(FAIL_SILENTLY, FORCE_ASYNCHRONOUS, ZERO_LOCK_ACQUISITION_TIMEOUT, PUT_FOR_EXTERNAL_READ).putIfAbsent(key, value);
       }
       catch (Exception e) {
-         if (log.isDebugEnabled()) log.debug("Caught exception while doing putForExternalRead()", e);
+         if (log.isDebugEnabled()) {
+            log.debug("Caught exception while doing putForExternalRead()", e);
+         }
       }
       finally {
          try {
-            if (ongoingTransaction != null) transactionManager.resume(ongoingTransaction);
+            if (ongoingTransaction != null) {
+               transactionManager.resume(ongoingTransaction);
+            }
          }
          catch (Exception e) {
             log.debug("Had problems trying to resume a transaction after putForExternalRead()", e);
@@ -280,7 +291,9 @@
    private InvocationContext getInvocationContext(boolean forceNonTransactional) {
       InvocationContext ctx = forceNonTransactional ? icc.createNonTxInvocationContext() : icc.createInvocationContext();
       PreInvocationContext pic = flagHolder.get();
-      if (pic != null && !pic.flags.isEmpty()) ctx.setFlags(pic.flags);
+      if (pic != null && !pic.flags.isEmpty()) {
+         ctx.setFlags(pic.flags);
+      }
       flagHolder.remove();
       return ctx;
    }
@@ -291,8 +304,9 @@
    }
 
    public boolean lock(Collection<? extends K> keys) {
-      if (keys == null || keys.isEmpty())
+      if (keys == null || keys.isEmpty()) {
          throw new IllegalArgumentException("Cannot lock empty list of keys");
+      }
       LockControlCommand command = commandsFactory.buildLockControlCommand(keys, false);
       return (Boolean) invoker.invoke(getInvocationContext(false), command);
    }
@@ -362,14 +376,16 @@
    }
 
    public boolean startBatch() {
-      if (!config.isInvocationBatchingEnabled())
+      if (!config.isInvocationBatchingEnabled()) {
          throw new ConfigurationException("Invocation batching not enabled in current configuration!  Please use the <invocationBatching /> element.");
+      }
       return batchContainer.startBatch();
    }
 
    public void endBatch(boolean successful) {
-      if (!config.isInvocationBatchingEnabled())
+      if (!config.isInvocationBatchingEnabled()) {
          throw new ConfigurationException("Invocation batching not enabled in current configuration!  Please use the <invocationBatching /> element.");
+      }
       batchContainer.endBatch(successful);
    }
 
@@ -554,8 +570,12 @@
 
    public void compact() {
       for (InternalCacheEntry e : dataContainer) {
-         if (e.getKey() instanceof MarshalledValue) ((MarshalledValue) e.getKey()).compact(true, true);
-         if (e.getValue() instanceof MarshalledValue) ((MarshalledValue) e.getValue()).compact(true, true);
+         if (e.getKey() instanceof MarshalledValue) {
+            ((MarshalledValue) e.getKey()).compact(true, true);
+         }
+         if (e.getValue() instanceof MarshalledValue) {
+            ((MarshalledValue) e.getValue()).compact(true, true);
+         }
       }
    }
 
@@ -566,10 +586,11 @@
    public AdvancedCache<K, V> withFlags(Flag... flags) {
       if (flags != null && flags.length > 0) {
          PreInvocationContext pic = flagHolder.get();
-         if (pic == null)
+         if (pic == null) {
             flagHolder.set(new PreInvocationContext(flags));
-         else
+         } else {
             flagHolder.set(pic.add(flags));
+         }
       }
       return this;
    }
@@ -582,7 +603,9 @@
       }
 
       private PreInvocationContext add(Flag[] newFlags) {
-         if (newFlags != null && newFlags.length > 0) flags.addAll(Arrays.asList(newFlags));
+         if (newFlags != null && newFlags.length > 0) {
+            flags.addAll(Arrays.asList(newFlags));
+         }
          return this;
       }
    }

Modified: branches/4.2.x/core/src/main/java/org/infinispan/commands/read/AbstractLocalCommand.java
===================================================================
--- branches/4.2.x/core/src/main/java/org/infinispan/commands/read/AbstractLocalCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ branches/4.2.x/core/src/main/java/org/infinispan/commands/read/AbstractLocalCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -1,14 +1,9 @@
 package org.infinispan.commands.read;
 
 import org.infinispan.commands.LocalCommand;
-import org.infinispan.container.DataContainer;
-import org.infinispan.container.entries.CacheEntry;
 import org.infinispan.context.InvocationContext;
 import org.infinispan.context.impl.TxInvocationContext;
 
-import java.util.HashSet;
-import java.util.Set;
-
 /**
  * Abstract class
  *
@@ -18,7 +13,7 @@
  */
 public class AbstractLocalCommand implements LocalCommand {
    private static final Object[] EMPTY_ARRAY = new Object[0];
-   
+
    public byte getCommandId() {
       return 0;  // no-op
    }
@@ -38,20 +33,4 @@
    protected boolean noTxModifications(InvocationContext ctx) {
       return !ctx.isInTxScope() || !((TxInvocationContext)ctx).hasModifications();
    }
-
-   protected Set<Object> getKeySetWithinTransaction(InvocationContext ctx, DataContainer container) {
-      Set<Object> objects = container.keySet();
-      Set<Object> result = new HashSet<Object>();
-      result.addAll(objects);
-      for (CacheEntry ce : ctx.getLookedUpEntries().values()) {
-         if (ce.isRemoved()) {
-            result.remove(ce.getKey());
-         } else {
-            if (ce.isCreated()) {
-               result.add(ce.getKey());
-            }
-         }
-      }
-      return result;
-   }
 }

Modified: branches/4.2.x/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java
===================================================================
--- branches/4.2.x/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ branches/4.2.x/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -21,33 +21,49 @@
  */
 package org.infinispan.commands.read;
 
+import java.util.AbstractSet;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Set;
+
 import org.infinispan.commands.VisitableCommand;
 import org.infinispan.commands.Visitor;
 import org.infinispan.container.DataContainer;
+import org.infinispan.container.entries.CacheEntry;
+import org.infinispan.container.entries.InternalCacheEntry;
+import org.infinispan.container.entries.InternalEntryFactory;
 import org.infinispan.context.InvocationContext;
+import org.infinispan.util.BidirectionalMap;
 import org.infinispan.util.Immutables;
 
-import java.util.Set;
-
 /**
  * EntrySetCommand.
  *
  * @author Galder Zamarreño
  * @since 4.0
  */
-public class EntrySetCommand extends AbstractLocalCommand implements VisitableCommand {   
+public class EntrySetCommand extends AbstractLocalCommand implements VisitableCommand {
    private final DataContainer container;
 
    public EntrySetCommand(DataContainer container) {
       this.container = container;
    }
 
+   @Override
    public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {
       return visitor.visitEntrySetCommand(ctx, this);
    }
 
-   public Set perform(InvocationContext ctx) throws Throwable {
-      return Immutables.immutableSetWrap(container.entrySet());
+   @Override
+   public Set<InternalCacheEntry> perform(InvocationContext ctx) throws Throwable {
+      Set<InternalCacheEntry> entries = container.entrySet();
+      if (noTxModifications(ctx)) {
+         return Immutables.immutableSetWrap(entries);
+      }
+
+      return new FilteredEntrySet(entries, ctx.getLookedUpEntries());
    }
 
    @Override
@@ -56,4 +72,165 @@
             "set=" + container.entrySet() +
             '}';
    }
+
+   private static class FilteredEntrySet extends AbstractSet<InternalCacheEntry> {
+      final Set<InternalCacheEntry> entrySet;
+      final BidirectionalMap<Object, CacheEntry> lookedUpEntries;
+
+      FilteredEntrySet(Set<InternalCacheEntry> entrySet, BidirectionalMap<Object, CacheEntry> lookedUpEntries) {
+         this.entrySet = entrySet;
+         this.lookedUpEntries = lookedUpEntries;
+      }
+
+      @Override
+      public int size() {
+         int size = entrySet.size();
+         for (CacheEntry e: lookedUpEntries.values()) {
+            if (e.isCreated()) {
+               size ++;
+            } else if (e.isRemoved()) {
+               size --;
+            }
+         }
+         return Math.max(size, 0);
+      }
+
+      @Override
+      public boolean contains(Object o) {
+         if (!(o instanceof Map.Entry)) {
+            return false;
+         }
+
+         @SuppressWarnings("rawtypes")
+         Map.Entry e = (Map.Entry) o;
+         CacheEntry ce = lookedUpEntries.get(e.getKey());
+         if (ce.isRemoved()) {
+            return false;
+         }
+         if (ce.isChanged() || ce.isCreated()) {
+            return ce.getValue().equals(e.getValue());
+         }
+
+         return entrySet.contains(o);
+      }
+
+      @Override
+      public Iterator<InternalCacheEntry> iterator() {
+         return new Itr();
+      }
+
+      @Override
+      public boolean add(InternalCacheEntry e) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean remove(Object o) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean addAll(Collection<? extends InternalCacheEntry> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean retainAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean removeAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public void clear() {
+         throw new UnsupportedOperationException();
+      }
+
+      private class Itr implements Iterator<InternalCacheEntry> {
+
+         private final Iterator<CacheEntry> it1 = lookedUpEntries.values().iterator();
+         private final Iterator<InternalCacheEntry> it2 = entrySet.iterator();
+         private boolean atIt1 = true;
+         private InternalCacheEntry next;
+
+         Itr() {
+            fetchNext();
+         }
+
+         private void fetchNext() {
+            if (atIt1) {
+               boolean found = false;
+               while (it1.hasNext()) {
+                  CacheEntry e = it1.next();
+                  if (e.isCreated()) {
+                     next = InternalEntryFactory.create(e.getKey(), e.getValue());
+                     found = true;
+                     break;
+                  }
+               }
+
+               if (!found) {
+                  atIt1 = false;
+               }
+            }
+
+            if (!atIt1) {
+               boolean found = false;
+               while (it2.hasNext()) {
+                  InternalCacheEntry ice = it2.next();
+                  Object key = ice.getKey();
+                  CacheEntry e = lookedUpEntries.get(key);
+                  if (e == null) {
+                     next = ice;
+                     found = true;
+                     break;
+                  }
+                  if (e.isChanged()) {
+                     next = InternalEntryFactory.create(key, e.getValue());
+                     found = true;
+                     break;
+                  }
+                  if (e.isRemoved()) {
+                     continue;
+                  }
+               }
+
+               if (!found) {
+                  next = null;
+               }
+            }
+         }
+
+         @Override
+         public boolean hasNext() {
+            if (next == null) {
+               fetchNext();
+            }
+            return next != null;
+         }
+
+         @Override
+         public InternalCacheEntry next() {
+            if (next == null) {
+               fetchNext();
+            }
+
+            if (next == null) {
+               throw new NoSuchElementException();
+            }
+
+            InternalCacheEntry ret = next;
+            next = null;
+            return ret;
+         }
+
+         @Override
+         public void remove() {
+            throw new UnsupportedOperationException();
+         }
+      }
+   }
 }

Modified: branches/4.2.x/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java
===================================================================
--- branches/4.2.x/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ branches/4.2.x/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -21,14 +21,20 @@
  */
 package org.infinispan.commands.read;
 
+import java.util.AbstractSet;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import java.util.Set;
+
 import org.infinispan.commands.VisitableCommand;
 import org.infinispan.commands.Visitor;
 import org.infinispan.container.DataContainer;
+import org.infinispan.container.entries.CacheEntry;
 import org.infinispan.context.InvocationContext;
+import org.infinispan.util.BidirectionalMap;
 import org.infinispan.util.Immutables;
 
-import java.util.Set;
-
 /**
  * KeySetCommand.
  *
@@ -43,17 +49,19 @@
       this.container = container;
    }
 
+   @Override
    public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {
       return visitor.visitKeySetCommand(ctx, this);
    }
 
-   public Set perform(InvocationContext ctx) throws Throwable {
+   @Override
+   public Set<Object> perform(InvocationContext ctx) throws Throwable {
       Set<Object> objects = container.keySet();
       if (noTxModifications(ctx)) {
          return Immutables.immutableSetWrap(objects);
       }
-      Set<Object> result = getKeySetWithinTransaction(ctx, container);
-      return Immutables.immutableSetWrap(result);
+
+      return new FilteredKeySet(objects, ctx.getLookedUpEntries());
    }
 
    @Override
@@ -62,4 +70,148 @@
             "set=" + container.keySet() +
             '}';
    }
+
+   private static class FilteredKeySet extends AbstractSet<Object> {
+      final Set<Object> keySet;
+      final BidirectionalMap<Object, CacheEntry> lookedUpEntries;
+
+      FilteredKeySet(Set<Object> keySet, BidirectionalMap<Object, CacheEntry> lookedUpEntries) {
+         this.keySet = keySet;
+         this.lookedUpEntries = lookedUpEntries;
+      }
+
+      @Override
+      public int size() {
+         int size = keySet.size();
+         for (CacheEntry e: lookedUpEntries.values()) {
+            if (e.isCreated()) {
+               size ++;
+            } else if (e.isRemoved()) {
+               size --;
+            }
+         }
+         return Math.max(size, 0);
+      }
+
+      @Override
+      public boolean contains(Object o) {
+         CacheEntry e = lookedUpEntries.get(o);
+         if (e.isRemoved()) {
+            return false;
+         } else if (e.isChanged() || e.isCreated()) {
+            return true;
+         }
+         return keySet.contains(o);
+      }
+
+      @Override
+      public Iterator<Object> iterator() {
+         return new Itr();
+      }
+
+      @Override
+      public boolean add(Object e) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean remove(Object o) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean addAll(Collection<? extends Object> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean retainAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean removeAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public void clear() {
+         throw new UnsupportedOperationException();
+      }
+
+      private class Itr implements Iterator<Object> {
+
+         private final Iterator<CacheEntry> it1 = lookedUpEntries.values().iterator();
+         private final Iterator<Object> it2 = keySet.iterator();
+         private boolean atIt1 = true;
+         private Object next;
+
+         Itr() {
+            fetchNext();
+         }
+
+         private void fetchNext() {
+            if (atIt1) {
+               boolean found = false;
+               while (it1.hasNext()) {
+                  CacheEntry e = it1.next();
+                  if (e.isCreated()) {
+                     next = e.getKey();
+                     found = true;
+                     break;
+                  }
+               }
+
+               if (!found) {
+                  atIt1 = false;
+               }
+            }
+
+            if (!atIt1) {
+               boolean found = false;
+               while (it2.hasNext()) {
+                  Object k = it2.next();
+                  CacheEntry e = lookedUpEntries.get(k);
+                  if (e == null || !e.isRemoved()) {
+                     next = k;
+                     found = true;
+                     break;
+                  }
+               }
+
+               if (!found) {
+                  next = null;
+               }
+            }
+         }
+
+         @Override
+         public boolean hasNext() {
+            if (next == null) {
+               fetchNext();
+            }
+            return next != null;
+         }
+
+         @Override
+         public Object next() {
+            if (next == null) {
+               fetchNext();
+            }
+
+            if (next == null) {
+               throw new NoSuchElementException();
+            }
+
+            Object ret = next;
+            next = null;
+            return ret;
+         }
+
+         @Override
+         public void remove() {
+            throw new UnsupportedOperationException();
+         }
+      }
+   }
 }

Modified: branches/4.2.x/core/src/main/java/org/infinispan/commands/read/SizeCommand.java
===================================================================
--- branches/4.2.x/core/src/main/java/org/infinispan/commands/read/SizeCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ branches/4.2.x/core/src/main/java/org/infinispan/commands/read/SizeCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -24,6 +24,7 @@
 import org.infinispan.commands.VisitableCommand;
 import org.infinispan.commands.Visitor;
 import org.infinispan.container.DataContainer;
+import org.infinispan.container.entries.CacheEntry;
 import org.infinispan.context.InvocationContext;
 
 /**
@@ -40,15 +41,27 @@
       this.container = container;
    }
 
+   @Override
    public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {
       return visitor.visitSizeCommand(ctx, this);
    }
 
+   @Override
    public Integer perform(InvocationContext ctx) throws Throwable {
       if (noTxModifications(ctx)) {
          return container.size();
       }
-      return super.getKeySetWithinTransaction(ctx, container).size();
+
+      int size = container.size();
+      for (CacheEntry e: ctx.getLookedUpEntries().values()) {
+         if (e.isCreated()) {
+            size ++;
+         } else  if (e.isRemoved()) {
+            size --;
+         }
+      }
+
+      return Math.max(size, 0);
    }
 
    @Override

Modified: branches/4.2.x/core/src/main/java/org/infinispan/commands/read/ValuesCommand.java
===================================================================
--- branches/4.2.x/core/src/main/java/org/infinispan/commands/read/ValuesCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ branches/4.2.x/core/src/main/java/org/infinispan/commands/read/ValuesCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -21,18 +21,21 @@
  */
 package org.infinispan.commands.read;
 
+import java.util.AbstractCollection;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import java.util.Set;
+
 import org.infinispan.commands.VisitableCommand;
 import org.infinispan.commands.Visitor;
 import org.infinispan.container.DataContainer;
 import org.infinispan.container.entries.CacheEntry;
 import org.infinispan.container.entries.InternalCacheEntry;
 import org.infinispan.context.InvocationContext;
+import org.infinispan.util.BidirectionalMap;
 import org.infinispan.util.Immutables;
 
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
-
 /**
  * ValuesCommand.
  *
@@ -47,29 +50,18 @@
       this.container = container;
    }
 
+   @Override
    public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {
       return visitor.visitValuesCommand(ctx, this);
    }
 
-   public Collection perform(InvocationContext ctx) throws Throwable {
+   @Override
+   public Collection<Object> perform(InvocationContext ctx) throws Throwable {
       if (noTxModifications(ctx)) {
          return Immutables.immutableCollectionWrap(container.values());
       }
-      Map result = new HashMap();
-      for (InternalCacheEntry ice : container.entrySet()) {
-         result.put(ice.getKey(), ice.getValue());
-      }      
-      for (CacheEntry ce : ctx.getLookedUpEntries().values()) {
-         if (ce.isRemoved()) {
-            result.remove(ce.getKey());
-         } else {
-            if (ce.isCreated() || ce.isChanged()) {
-               result.put(ce.getKey(), ce.getValue());
-            }
-         }
-      }
-      return Immutables.immutableCollectionWrap(result.values());
 
+      return new FilteredValues(container, ctx.getLookedUpEntries());
    }
 
    @Override
@@ -78,4 +70,163 @@
             "values=" + container.values() +
             '}';
    }
+
+   private static class FilteredValues extends AbstractCollection<Object> {
+      final Collection<Object> values;
+      final Set<InternalCacheEntry> entrySet;
+      final BidirectionalMap<Object, CacheEntry> lookedUpEntries;
+
+      FilteredValues(DataContainer container, BidirectionalMap<Object, CacheEntry> lookedUpEntries) {
+         values = container.values();
+         entrySet = container.entrySet();
+         this.lookedUpEntries = lookedUpEntries;
+      }
+
+      @Override
+      public int size() {
+         int size = entrySet.size();
+         for (CacheEntry e: lookedUpEntries.values()) {
+            if (e.isCreated()) {
+               size ++;
+            } else if (e.isRemoved()) {
+               size --;
+            }
+         }
+         return Math.max(size, 0);
+      }
+
+      @Override
+      public boolean contains(Object o) {
+         for (CacheEntry e: lookedUpEntries.values()) {
+            if (o.equals(e.getValue())) {
+               if (e.isRemoved()) {
+                  return false;
+               } else {
+                  return true;
+               }
+            }
+         }
+
+         return values.contains(o);
+      }
+
+      @Override
+      public Iterator<Object> iterator() {
+         return new Itr();
+      }
+
+      @Override
+      public boolean add(Object e) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean remove(Object o) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean addAll(Collection<? extends Object> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean retainAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean removeAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public void clear() {
+         throw new UnsupportedOperationException();
+      }
+
+      private class Itr implements Iterator<Object> {
+
+         private final Iterator<CacheEntry> it1 = lookedUpEntries.values().iterator();
+         private final Iterator<InternalCacheEntry> it2 = entrySet.iterator();
+         private boolean atIt1 = true;
+         private Object next;
+
+         Itr() {
+            fetchNext();
+         }
+
+         private void fetchNext() {
+            if (atIt1) {
+               boolean found = false;
+               while (it1.hasNext()) {
+                  CacheEntry e = it1.next();
+                  if (e.isCreated()) {
+                     next = e.getValue();
+                     found = true;
+                     break;
+                  }
+               }
+
+               if (!found) {
+                  atIt1 = false;
+               }
+            }
+
+            if (!atIt1) {
+               boolean found = false;
+               while (it2.hasNext()) {
+                  InternalCacheEntry ice = it2.next();
+                  Object key = ice.getKey();
+                  CacheEntry e = lookedUpEntries.get(key);
+                  if (e == null) {
+                     next = ice.getValue();
+                     found = true;
+                     break;
+                  }
+                  if (e.isChanged()) {
+                     next = e.getValue();
+                     found = true;
+                     break;
+                  }
+                  if (e.isRemoved()) {
+                     continue;
+                  }
+               }
+
+               if (!found) {
+                  next = null;
+               }
+            }
+         }
+
+         @Override
+         public boolean hasNext() {
+            if (next == null) {
+               fetchNext();
+            }
+            return next != null;
+         }
+
+         @Override
+         public Object next() {
+            if (next == null) {
+               fetchNext();
+            }
+
+            if (next == null) {
+               throw new NoSuchElementException();
+            }
+
+            Object ret = next;
+            next = null;
+            return ret;
+         }
+
+         @Override
+         public void remove() {
+            throw new UnsupportedOperationException();
+         }
+      }
+   }
 }

Modified: branches/4.2.x/core/src/main/java/org/infinispan/container/DefaultDataContainer.java
===================================================================
--- branches/4.2.x/core/src/main/java/org/infinispan/container/DefaultDataContainer.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ branches/4.2.x/core/src/main/java/org/infinispan/container/DefaultDataContainer.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -5,6 +5,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -216,7 +217,23 @@
     *
     */
    private class EntrySet extends AbstractSet<InternalCacheEntry> {
+
       @Override
+      public boolean contains(Object o) {
+         if (!(o instanceof Map.Entry)) {
+            return false;
+         }
+
+         @SuppressWarnings("rawtypes")
+         Map.Entry e = (Map.Entry) o;
+         InternalCacheEntry ice = entries.get(e.getKey());
+         if (ice == null) {
+            return false;
+         }
+         return ice.getValue().equals(e.getValue());
+      }
+
+      @Override
       public Iterator<InternalCacheEntry> iterator() {
          return new ImmutableEntryIterator(entries.values().iterator());
       }

Modified: branches/4.2.x/core/src/test/java/org/infinispan/api/CacheAPITest.java
===================================================================
--- branches/4.2.x/core/src/test/java/org/infinispan/api/CacheAPITest.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ branches/4.2.x/core/src/test/java/org/infinispan/api/CacheAPITest.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -1,5 +1,16 @@
 package org.infinispan.api;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.transaction.NotSupportedException;
+import javax.transaction.SystemException;
+
 import org.infinispan.config.Configuration;
 import org.infinispan.config.ConfigurationException;
 import org.infinispan.lifecycle.ComponentStatus;
@@ -11,16 +22,6 @@
 import org.infinispan.util.concurrent.IsolationLevel;
 import org.testng.annotations.Test;
 
-import javax.transaction.NotSupportedException;
-import javax.transaction.SystemException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 /**
  * Tests the {@link org.infinispan.Cache} public API at a high level
  *
@@ -29,6 +30,7 @@
 @Test(groups = "functional")
 public abstract class CacheAPITest extends SingleCacheManagerTest {
 
+   @Override
    protected EmbeddedCacheManager createCacheManager() throws Exception {
       // start a single cache instance
       Configuration c = getDefaultStandaloneConfig(true);
@@ -181,10 +183,14 @@
       TestingUtil.getTransactionManager(cache).begin();
       cache.put(key2, value2);
       assert cache.get(key2).equals(value2);
-      assert !cache.keySet().contains(key2);
-      size = 1;
-      assert size == cache.size() && size == cache.keySet().size() && size == cache.values().size() && size == cache.entrySet().size();
-      assert !cache.values().contains(value2);
+      assert cache.keySet().contains(key2);
+      size = 2;
+      System.out.println(cache.size());
+      assert size == cache.size();
+      assert size == cache.keySet().size();
+      assert size == cache.values().size();
+      assert size == cache.entrySet().size();
+      assert cache.values().contains(value2);
       TestingUtil.getTransactionManager(cache).rollback();
 
       assert cache.get(key).equals(value);
@@ -210,7 +216,7 @@
       size = 1;
       assert size == cache.size() && size == cache.keySet().size() && size == cache.values().size() && size == cache.entrySet().size();
       assert cache.keySet().contains(key);
-      assert !cache.values().contains(value2);
+      assert cache.values().contains(value2);
       TestingUtil.getTransactionManager(cache).rollback();
 
       assert cache.get(key).equals(value);
@@ -233,7 +239,7 @@
       TestingUtil.getTransactionManager(cache).begin();
       cache.remove(key);
       assert cache.get(key) == null;
-      size = 1;
+      size = 0;
       assert size == cache.size() && size == cache.keySet().size() && size == cache.values().size() && size == cache.entrySet().size();
       TestingUtil.getTransactionManager(cache).rollback();
 
@@ -257,7 +263,7 @@
       TestingUtil.getTransactionManager(cache).begin();
       cache.clear();
       assert cache.get(key) == null;
-      size = 1;
+      size = 0;
       assert size == cache.size() && size == cache.keySet().size() && size == cache.values().size() && size == cache.entrySet().size();
       TestingUtil.getTransactionManager(cache).rollback();
 
@@ -368,11 +374,15 @@
       Set expValueEntries = ObjectDuplicator.duplicateSet(expValues);
 
       Set<Object> keys = cache.keySet();
-      for (Object key : keys) assert expKeys.remove(key);
+      for (Object key : keys) {
+         assert expKeys.remove(key);
+      }
       assert expKeys.isEmpty() : "Did not see keys " + expKeys + " in iterator!";
 
       Collection<Object> values = cache.values();
-      for (Object value : values) assert expValues.remove(value);
+      for (Object value : values) {
+         assert expValues.remove(value);
+      }
       assert expValues.isEmpty() : "Did not see keys " + expValues + " in iterator!";
 
       Set<Map.Entry<Object, Object>> entries = cache.entrySet();

Modified: trunk/core/src/main/java/org/infinispan/CacheDelegate.java
===================================================================
--- trunk/core/src/main/java/org/infinispan/CacheDelegate.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ trunk/core/src/main/java/org/infinispan/CacheDelegate.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -21,6 +21,21 @@
  */
 package org.infinispan;
 
+import static org.infinispan.context.Flag.*;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import javax.transaction.Transaction;
+import javax.transaction.TransactionManager;
+
 import org.infinispan.batch.BatchContainer;
 import org.infinispan.commands.CommandsFactory;
 import org.infinispan.commands.control.LockControlCommand;
@@ -37,7 +52,6 @@
 import org.infinispan.commands.write.ReplaceCommand;
 import org.infinispan.config.Configuration;
 import org.infinispan.config.ConfigurationException;
-import org.infinispan.config.ConfigurationValidatingVisitor;
 import org.infinispan.container.DataContainer;
 import org.infinispan.container.entries.InternalCacheEntry;
 import org.infinispan.context.Flag;
@@ -73,20 +87,6 @@
 import org.rhq.helpers.pluginAnnotations.agent.Metric;
 import org.rhq.helpers.pluginAnnotations.agent.Operation;
 
-import javax.transaction.Transaction;
-import javax.transaction.TransactionManager;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.EnumSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
-
-import static org.infinispan.context.Flag.*;
-
 /**
  * @author Mircea.Markus at jboss.com
  * @author Galder Zamarreño
@@ -106,7 +106,7 @@
    protected TransactionManager transactionManager;
    protected RpcManager rpcManager;
    protected StreamingMarshaller marshaller;
-   private String name;
+   private final String name;
    private EvictionManager evictionManager;
    private DataContainer dataContainer;
    private static final Log log = LogFactory.getLog(CacheDelegate.class);
@@ -116,7 +116,7 @@
    // as above for ResponseGenerator
    private ResponseGenerator responseGenerator;
    private DistributionManager distributionManager;
-   private ThreadLocal<PreInvocationContext> flagHolder = new ThreadLocal<PreInvocationContext>();
+   private final ThreadLocal<PreInvocationContext> flagHolder = new ThreadLocal<PreInvocationContext>();
 
    public CacheDelegate(String name) {
       this.name = name;
@@ -151,17 +151,24 @@
       this.responseGenerator = responseGenerator;
       this.stateTransferManager = stateTransferManager;
       this.icc = icc;
-      this.distributionManager = distributionManager; 
+      this.distributionManager = distributionManager;
    }
 
    private void assertKeyNotNull(Object key) {
-      if (key == null) throw new NullPointerException("Null keys are not supported!");
+      if (key == null) {
+         throw new NullPointerException("Null keys are not supported!");
+      }
    }
 
    private void assertKeysNotNull(Map<?, ?> data) {
-      if (data == null) throw new NullPointerException("Expected map cannot be null");
-      for (Object key: data.keySet())
-         if (key == null) throw new NullPointerException("Null keys are not supported!");
+      if (data == null) {
+         throw new NullPointerException("Expected map cannot be null");
+      }
+      for (Object key: data.keySet()) {
+         if (key == null) {
+            throw new NullPointerException("Null keys are not supported!");
+         }
+      }
    }
 
    public final boolean remove(Object key, Object value) {
@@ -229,7 +236,7 @@
    @SuppressWarnings("unchecked")
    public Set<Map.Entry<K, V>> entrySet() {
       EntrySetCommand command = commandsFactory.buildEntrySetCommand();
-      return (Set<Map.Entry<K, V>>) invoker.invoke(icc.createNonTxInvocationContext(), command);
+      return (Set<Map.Entry<K, V>>) invoker.invoke(getInvocationContext(false), command);
    }
 
    public final void putForExternalRead(K key, V value) {
@@ -242,11 +249,15 @@
          withFlags(FAIL_SILENTLY, FORCE_ASYNCHRONOUS, ZERO_LOCK_ACQUISITION_TIMEOUT, PUT_FOR_EXTERNAL_READ).putIfAbsent(key, value);
       }
       catch (Exception e) {
-         if (log.isDebugEnabled()) log.debug("Caught exception while doing putForExternalRead()", e);
+         if (log.isDebugEnabled()) {
+            log.debug("Caught exception while doing putForExternalRead()", e);
+         }
       }
       finally {
          try {
-            if (ongoingTransaction != null) transactionManager.resume(ongoingTransaction);
+            if (ongoingTransaction != null) {
+               transactionManager.resume(ongoingTransaction);
+            }
          }
          catch (Exception e) {
             log.debug("Had problems trying to resume a transaction after putForExternalRead()", e);
@@ -280,7 +291,9 @@
    private InvocationContext getInvocationContext(boolean forceNonTransactional) {
       InvocationContext ctx = forceNonTransactional ? icc.createNonTxInvocationContext() : icc.createInvocationContext();
       PreInvocationContext pic = flagHolder.get();
-      if (pic != null && !pic.flags.isEmpty()) ctx.setFlags(pic.flags);
+      if (pic != null && !pic.flags.isEmpty()) {
+         ctx.setFlags(pic.flags);
+      }
       flagHolder.remove();
       return ctx;
    }
@@ -291,8 +304,9 @@
    }
 
    public boolean lock(Collection<? extends K> keys) {
-      if (keys == null || keys.isEmpty())
+      if (keys == null || keys.isEmpty()) {
          throw new IllegalArgumentException("Cannot lock empty list of keys");
+      }
       LockControlCommand command = commandsFactory.buildLockControlCommand(keys, false);
       return (Boolean) invoker.invoke(getInvocationContext(false), command);
    }
@@ -362,14 +376,16 @@
    }
 
    public boolean startBatch() {
-      if (!config.isInvocationBatchingEnabled())
+      if (!config.isInvocationBatchingEnabled()) {
          throw new ConfigurationException("Invocation batching not enabled in current configuration!  Please use the <invocationBatching /> element.");
+      }
       return batchContainer.startBatch();
    }
 
    public void endBatch(boolean successful) {
-      if (!config.isInvocationBatchingEnabled())
+      if (!config.isInvocationBatchingEnabled()) {
          throw new ConfigurationException("Invocation batching not enabled in current configuration!  Please use the <invocationBatching /> element.");
+      }
       batchContainer.endBatch(successful);
    }
 
@@ -554,8 +570,12 @@
 
    public void compact() {
       for (InternalCacheEntry e : dataContainer) {
-         if (e.getKey() instanceof MarshalledValue) ((MarshalledValue) e.getKey()).compact(true, true);
-         if (e.getValue() instanceof MarshalledValue) ((MarshalledValue) e.getValue()).compact(true, true);
+         if (e.getKey() instanceof MarshalledValue) {
+            ((MarshalledValue) e.getKey()).compact(true, true);
+         }
+         if (e.getValue() instanceof MarshalledValue) {
+            ((MarshalledValue) e.getValue()).compact(true, true);
+         }
       }
    }
 
@@ -566,10 +586,11 @@
    public AdvancedCache<K, V> withFlags(Flag... flags) {
       if (flags != null && flags.length > 0) {
          PreInvocationContext pic = flagHolder.get();
-         if (pic == null)
+         if (pic == null) {
             flagHolder.set(new PreInvocationContext(flags));
-         else
+         } else {
             flagHolder.set(pic.add(flags));
+         }
       }
       return this;
    }
@@ -582,7 +603,9 @@
       }
 
       private PreInvocationContext add(Flag[] newFlags) {
-         if (newFlags != null && newFlags.length > 0) flags.addAll(Arrays.asList(newFlags));
+         if (newFlags != null && newFlags.length > 0) {
+            flags.addAll(Arrays.asList(newFlags));
+         }
          return this;
       }
    }

Modified: trunk/core/src/main/java/org/infinispan/commands/read/AbstractLocalCommand.java
===================================================================
--- trunk/core/src/main/java/org/infinispan/commands/read/AbstractLocalCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ trunk/core/src/main/java/org/infinispan/commands/read/AbstractLocalCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -1,14 +1,9 @@
 package org.infinispan.commands.read;
 
 import org.infinispan.commands.LocalCommand;
-import org.infinispan.container.DataContainer;
-import org.infinispan.container.entries.CacheEntry;
 import org.infinispan.context.InvocationContext;
 import org.infinispan.context.impl.TxInvocationContext;
 
-import java.util.HashSet;
-import java.util.Set;
-
 /**
  * Abstract class
  *
@@ -18,7 +13,7 @@
  */
 public class AbstractLocalCommand implements LocalCommand {
    private static final Object[] EMPTY_ARRAY = new Object[0];
-   
+
    public byte getCommandId() {
       return 0;  // no-op
    }
@@ -38,20 +33,4 @@
    protected boolean noTxModifications(InvocationContext ctx) {
       return !ctx.isInTxScope() || !((TxInvocationContext)ctx).hasModifications();
    }
-
-   protected Set<Object> getKeySetWithinTransaction(InvocationContext ctx, DataContainer container) {
-      Set<Object> objects = container.keySet();
-      Set<Object> result = new HashSet<Object>();
-      result.addAll(objects);
-      for (CacheEntry ce : ctx.getLookedUpEntries().values()) {
-         if (ce.isRemoved()) {
-            result.remove(ce.getKey());
-         } else {
-            if (ce.isCreated()) {
-               result.add(ce.getKey());
-            }
-         }
-      }
-      return result;
-   }
 }

Modified: trunk/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java
===================================================================
--- trunk/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ trunk/core/src/main/java/org/infinispan/commands/read/EntrySetCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -21,33 +21,49 @@
  */
 package org.infinispan.commands.read;
 
+import java.util.AbstractSet;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Set;
+
 import org.infinispan.commands.VisitableCommand;
 import org.infinispan.commands.Visitor;
 import org.infinispan.container.DataContainer;
+import org.infinispan.container.entries.CacheEntry;
+import org.infinispan.container.entries.InternalCacheEntry;
+import org.infinispan.container.entries.InternalEntryFactory;
 import org.infinispan.context.InvocationContext;
+import org.infinispan.util.BidirectionalMap;
 import org.infinispan.util.Immutables;
 
-import java.util.Set;
-
 /**
  * EntrySetCommand.
  *
  * @author Galder Zamarreño
  * @since 4.0
  */
-public class EntrySetCommand extends AbstractLocalCommand implements VisitableCommand {   
+public class EntrySetCommand extends AbstractLocalCommand implements VisitableCommand {
    private final DataContainer container;
 
    public EntrySetCommand(DataContainer container) {
       this.container = container;
    }
 
+   @Override
    public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {
       return visitor.visitEntrySetCommand(ctx, this);
    }
 
-   public Set perform(InvocationContext ctx) throws Throwable {
-      return Immutables.immutableSetWrap(container.entrySet());
+   @Override
+   public Set<InternalCacheEntry> perform(InvocationContext ctx) throws Throwable {
+      Set<InternalCacheEntry> entries = container.entrySet();
+      if (noTxModifications(ctx)) {
+         return Immutables.immutableSetWrap(entries);
+      }
+
+      return new FilteredEntrySet(entries, ctx.getLookedUpEntries());
    }
 
    @Override
@@ -56,4 +72,165 @@
             "set=" + container.entrySet() +
             '}';
    }
+
+   private static class FilteredEntrySet extends AbstractSet<InternalCacheEntry> {
+      final Set<InternalCacheEntry> entrySet;
+      final BidirectionalMap<Object, CacheEntry> lookedUpEntries;
+
+      FilteredEntrySet(Set<InternalCacheEntry> entrySet, BidirectionalMap<Object, CacheEntry> lookedUpEntries) {
+         this.entrySet = entrySet;
+         this.lookedUpEntries = lookedUpEntries;
+      }
+
+      @Override
+      public int size() {
+         int size = entrySet.size();
+         for (CacheEntry e: lookedUpEntries.values()) {
+            if (e.isCreated()) {
+               size ++;
+            } else if (e.isRemoved()) {
+               size --;
+            }
+         }
+         return Math.max(size, 0);
+      }
+
+      @Override
+      public boolean contains(Object o) {
+         if (!(o instanceof Map.Entry)) {
+            return false;
+         }
+
+         @SuppressWarnings("rawtypes")
+         Map.Entry e = (Map.Entry) o;
+         CacheEntry ce = lookedUpEntries.get(e.getKey());
+         if (ce.isRemoved()) {
+            return false;
+         }
+         if (ce.isChanged() || ce.isCreated()) {
+            return ce.getValue().equals(e.getValue());
+         }
+
+         return entrySet.contains(o);
+      }
+
+      @Override
+      public Iterator<InternalCacheEntry> iterator() {
+         return new Itr();
+      }
+
+      @Override
+      public boolean add(InternalCacheEntry e) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean remove(Object o) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean addAll(Collection<? extends InternalCacheEntry> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean retainAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean removeAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public void clear() {
+         throw new UnsupportedOperationException();
+      }
+
+      private class Itr implements Iterator<InternalCacheEntry> {
+
+         private final Iterator<CacheEntry> it1 = lookedUpEntries.values().iterator();
+         private final Iterator<InternalCacheEntry> it2 = entrySet.iterator();
+         private boolean atIt1 = true;
+         private InternalCacheEntry next;
+
+         Itr() {
+            fetchNext();
+         }
+
+         private void fetchNext() {
+            if (atIt1) {
+               boolean found = false;
+               while (it1.hasNext()) {
+                  CacheEntry e = it1.next();
+                  if (e.isCreated()) {
+                     next = InternalEntryFactory.create(e.getKey(), e.getValue());
+                     found = true;
+                     break;
+                  }
+               }
+
+               if (!found) {
+                  atIt1 = false;
+               }
+            }
+
+            if (!atIt1) {
+               boolean found = false;
+               while (it2.hasNext()) {
+                  InternalCacheEntry ice = it2.next();
+                  Object key = ice.getKey();
+                  CacheEntry e = lookedUpEntries.get(key);
+                  if (e == null) {
+                     next = ice;
+                     found = true;
+                     break;
+                  }
+                  if (e.isChanged()) {
+                     next = InternalEntryFactory.create(key, e.getValue());
+                     found = true;
+                     break;
+                  }
+                  if (e.isRemoved()) {
+                     continue;
+                  }
+               }
+
+               if (!found) {
+                  next = null;
+               }
+            }
+         }
+
+         @Override
+         public boolean hasNext() {
+            if (next == null) {
+               fetchNext();
+            }
+            return next != null;
+         }
+
+         @Override
+         public InternalCacheEntry next() {
+            if (next == null) {
+               fetchNext();
+            }
+
+            if (next == null) {
+               throw new NoSuchElementException();
+            }
+
+            InternalCacheEntry ret = next;
+            next = null;
+            return ret;
+         }
+
+         @Override
+         public void remove() {
+            throw new UnsupportedOperationException();
+         }
+      }
+   }
 }

Modified: trunk/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java
===================================================================
--- trunk/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ trunk/core/src/main/java/org/infinispan/commands/read/KeySetCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -21,14 +21,20 @@
  */
 package org.infinispan.commands.read;
 
+import java.util.AbstractSet;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import java.util.Set;
+
 import org.infinispan.commands.VisitableCommand;
 import org.infinispan.commands.Visitor;
 import org.infinispan.container.DataContainer;
+import org.infinispan.container.entries.CacheEntry;
 import org.infinispan.context.InvocationContext;
+import org.infinispan.util.BidirectionalMap;
 import org.infinispan.util.Immutables;
 
-import java.util.Set;
-
 /**
  * KeySetCommand.
  *
@@ -43,17 +49,19 @@
       this.container = container;
    }
 
+   @Override
    public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {
       return visitor.visitKeySetCommand(ctx, this);
    }
 
-   public Set perform(InvocationContext ctx) throws Throwable {
+   @Override
+   public Set<Object> perform(InvocationContext ctx) throws Throwable {
       Set<Object> objects = container.keySet();
       if (noTxModifications(ctx)) {
          return Immutables.immutableSetWrap(objects);
       }
-      Set<Object> result = getKeySetWithinTransaction(ctx, container);
-      return Immutables.immutableSetWrap(result);
+
+      return new FilteredKeySet(objects, ctx.getLookedUpEntries());
    }
 
    @Override
@@ -62,4 +70,148 @@
             "set=" + container.keySet() +
             '}';
    }
+
+   private static class FilteredKeySet extends AbstractSet<Object> {
+      final Set<Object> keySet;
+      final BidirectionalMap<Object, CacheEntry> lookedUpEntries;
+
+      FilteredKeySet(Set<Object> keySet, BidirectionalMap<Object, CacheEntry> lookedUpEntries) {
+         this.keySet = keySet;
+         this.lookedUpEntries = lookedUpEntries;
+      }
+
+      @Override
+      public int size() {
+         int size = keySet.size();
+         for (CacheEntry e: lookedUpEntries.values()) {
+            if (e.isCreated()) {
+               size ++;
+            } else if (e.isRemoved()) {
+               size --;
+            }
+         }
+         return Math.max(size, 0);
+      }
+
+      @Override
+      public boolean contains(Object o) {
+         CacheEntry e = lookedUpEntries.get(o);
+         if (e.isRemoved()) {
+            return false;
+         } else if (e.isChanged() || e.isCreated()) {
+            return true;
+         }
+         return keySet.contains(o);
+      }
+
+      @Override
+      public Iterator<Object> iterator() {
+         return new Itr();
+      }
+
+      @Override
+      public boolean add(Object e) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean remove(Object o) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean addAll(Collection<? extends Object> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean retainAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean removeAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public void clear() {
+         throw new UnsupportedOperationException();
+      }
+
+      private class Itr implements Iterator<Object> {
+
+         private final Iterator<CacheEntry> it1 = lookedUpEntries.values().iterator();
+         private final Iterator<Object> it2 = keySet.iterator();
+         private boolean atIt1 = true;
+         private Object next;
+
+         Itr() {
+            fetchNext();
+         }
+
+         private void fetchNext() {
+            if (atIt1) {
+               boolean found = false;
+               while (it1.hasNext()) {
+                  CacheEntry e = it1.next();
+                  if (e.isCreated()) {
+                     next = e.getKey();
+                     found = true;
+                     break;
+                  }
+               }
+
+               if (!found) {
+                  atIt1 = false;
+               }
+            }
+
+            if (!atIt1) {
+               boolean found = false;
+               while (it2.hasNext()) {
+                  Object k = it2.next();
+                  CacheEntry e = lookedUpEntries.get(k);
+                  if (e == null || !e.isRemoved()) {
+                     next = k;
+                     found = true;
+                     break;
+                  }
+               }
+
+               if (!found) {
+                  next = null;
+               }
+            }
+         }
+
+         @Override
+         public boolean hasNext() {
+            if (next == null) {
+               fetchNext();
+            }
+            return next != null;
+         }
+
+         @Override
+         public Object next() {
+            if (next == null) {
+               fetchNext();
+            }
+
+            if (next == null) {
+               throw new NoSuchElementException();
+            }
+
+            Object ret = next;
+            next = null;
+            return ret;
+         }
+
+         @Override
+         public void remove() {
+            throw new UnsupportedOperationException();
+         }
+      }
+   }
 }

Modified: trunk/core/src/main/java/org/infinispan/commands/read/SizeCommand.java
===================================================================
--- trunk/core/src/main/java/org/infinispan/commands/read/SizeCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ trunk/core/src/main/java/org/infinispan/commands/read/SizeCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -24,6 +24,7 @@
 import org.infinispan.commands.VisitableCommand;
 import org.infinispan.commands.Visitor;
 import org.infinispan.container.DataContainer;
+import org.infinispan.container.entries.CacheEntry;
 import org.infinispan.context.InvocationContext;
 
 /**
@@ -40,15 +41,27 @@
       this.container = container;
    }
 
+   @Override
    public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {
       return visitor.visitSizeCommand(ctx, this);
    }
 
+   @Override
    public Integer perform(InvocationContext ctx) throws Throwable {
       if (noTxModifications(ctx)) {
          return container.size();
       }
-      return super.getKeySetWithinTransaction(ctx, container).size();
+
+      int size = container.size();
+      for (CacheEntry e: ctx.getLookedUpEntries().values()) {
+         if (e.isCreated()) {
+            size ++;
+         } else  if (e.isRemoved()) {
+            size --;
+         }
+      }
+
+      return Math.max(size, 0);
    }
 
    @Override

Modified: trunk/core/src/main/java/org/infinispan/commands/read/ValuesCommand.java
===================================================================
--- trunk/core/src/main/java/org/infinispan/commands/read/ValuesCommand.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ trunk/core/src/main/java/org/infinispan/commands/read/ValuesCommand.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -21,18 +21,21 @@
  */
 package org.infinispan.commands.read;
 
+import java.util.AbstractCollection;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import java.util.Set;
+
 import org.infinispan.commands.VisitableCommand;
 import org.infinispan.commands.Visitor;
 import org.infinispan.container.DataContainer;
 import org.infinispan.container.entries.CacheEntry;
 import org.infinispan.container.entries.InternalCacheEntry;
 import org.infinispan.context.InvocationContext;
+import org.infinispan.util.BidirectionalMap;
 import org.infinispan.util.Immutables;
 
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
-
 /**
  * ValuesCommand.
  *
@@ -47,29 +50,18 @@
       this.container = container;
    }
 
+   @Override
    public Object acceptVisitor(InvocationContext ctx, Visitor visitor) throws Throwable {
       return visitor.visitValuesCommand(ctx, this);
    }
 
-   public Collection perform(InvocationContext ctx) throws Throwable {
+   @Override
+   public Collection<Object> perform(InvocationContext ctx) throws Throwable {
       if (noTxModifications(ctx)) {
          return Immutables.immutableCollectionWrap(container.values());
       }
-      Map result = new HashMap();
-      for (InternalCacheEntry ice : container.entrySet()) {
-         result.put(ice.getKey(), ice.getValue());
-      }      
-      for (CacheEntry ce : ctx.getLookedUpEntries().values()) {
-         if (ce.isRemoved()) {
-            result.remove(ce.getKey());
-         } else {
-            if (ce.isCreated() || ce.isChanged()) {
-               result.put(ce.getKey(), ce.getValue());
-            }
-         }
-      }
-      return Immutables.immutableCollectionWrap(result.values());
 
+      return new FilteredValues(container, ctx.getLookedUpEntries());
    }
 
    @Override
@@ -78,4 +70,163 @@
             "values=" + container.values() +
             '}';
    }
+
+   private static class FilteredValues extends AbstractCollection<Object> {
+      final Collection<Object> values;
+      final Set<InternalCacheEntry> entrySet;
+      final BidirectionalMap<Object, CacheEntry> lookedUpEntries;
+
+      FilteredValues(DataContainer container, BidirectionalMap<Object, CacheEntry> lookedUpEntries) {
+         values = container.values();
+         entrySet = container.entrySet();
+         this.lookedUpEntries = lookedUpEntries;
+      }
+
+      @Override
+      public int size() {
+         int size = entrySet.size();
+         for (CacheEntry e: lookedUpEntries.values()) {
+            if (e.isCreated()) {
+               size ++;
+            } else if (e.isRemoved()) {
+               size --;
+            }
+         }
+         return Math.max(size, 0);
+      }
+
+      @Override
+      public boolean contains(Object o) {
+         for (CacheEntry e: lookedUpEntries.values()) {
+            if (o.equals(e.getValue())) {
+               if (e.isRemoved()) {
+                  return false;
+               } else {
+                  return true;
+               }
+            }
+         }
+
+         return values.contains(o);
+      }
+
+      @Override
+      public Iterator<Object> iterator() {
+         return new Itr();
+      }
+
+      @Override
+      public boolean add(Object e) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean remove(Object o) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean addAll(Collection<? extends Object> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean retainAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public boolean removeAll(Collection<?> c) {
+         throw new UnsupportedOperationException();
+      }
+
+      @Override
+      public void clear() {
+         throw new UnsupportedOperationException();
+      }
+
+      private class Itr implements Iterator<Object> {
+
+         private final Iterator<CacheEntry> it1 = lookedUpEntries.values().iterator();
+         private final Iterator<InternalCacheEntry> it2 = entrySet.iterator();
+         private boolean atIt1 = true;
+         private Object next;
+
+         Itr() {
+            fetchNext();
+         }
+
+         private void fetchNext() {
+            if (atIt1) {
+               boolean found = false;
+               while (it1.hasNext()) {
+                  CacheEntry e = it1.next();
+                  if (e.isCreated()) {
+                     next = e.getValue();
+                     found = true;
+                     break;
+                  }
+               }
+
+               if (!found) {
+                  atIt1 = false;
+               }
+            }
+
+            if (!atIt1) {
+               boolean found = false;
+               while (it2.hasNext()) {
+                  InternalCacheEntry ice = it2.next();
+                  Object key = ice.getKey();
+                  CacheEntry e = lookedUpEntries.get(key);
+                  if (e == null) {
+                     next = ice.getValue();
+                     found = true;
+                     break;
+                  }
+                  if (e.isChanged()) {
+                     next = e.getValue();
+                     found = true;
+                     break;
+                  }
+                  if (e.isRemoved()) {
+                     continue;
+                  }
+               }
+
+               if (!found) {
+                  next = null;
+               }
+            }
+         }
+
+         @Override
+         public boolean hasNext() {
+            if (next == null) {
+               fetchNext();
+            }
+            return next != null;
+         }
+
+         @Override
+         public Object next() {
+            if (next == null) {
+               fetchNext();
+            }
+
+            if (next == null) {
+               throw new NoSuchElementException();
+            }
+
+            Object ret = next;
+            next = null;
+            return ret;
+         }
+
+         @Override
+         public void remove() {
+            throw new UnsupportedOperationException();
+         }
+      }
+   }
 }

Modified: trunk/core/src/main/java/org/infinispan/container/DefaultDataContainer.java
===================================================================
--- trunk/core/src/main/java/org/infinispan/container/DefaultDataContainer.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ trunk/core/src/main/java/org/infinispan/container/DefaultDataContainer.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -5,6 +5,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -216,7 +217,23 @@
     *
     */
    private class EntrySet extends AbstractSet<InternalCacheEntry> {
+
       @Override
+      public boolean contains(Object o) {
+         if (!(o instanceof Map.Entry)) {
+            return false;
+         }
+
+         @SuppressWarnings("rawtypes")
+         Map.Entry e = (Map.Entry) o;
+         InternalCacheEntry ice = entries.get(e.getKey());
+         if (ice == null) {
+            return false;
+         }
+         return ice.getValue().equals(e.getValue());
+      }
+
+      @Override
       public Iterator<InternalCacheEntry> iterator() {
          return new ImmutableEntryIterator(entries.values().iterator());
       }

Modified: trunk/core/src/test/java/org/infinispan/api/CacheAPITest.java
===================================================================
--- trunk/core/src/test/java/org/infinispan/api/CacheAPITest.java	2010-10-21 15:34:46 UTC (rev 2559)
+++ trunk/core/src/test/java/org/infinispan/api/CacheAPITest.java	2010-10-21 17:16:32 UTC (rev 2560)
@@ -1,5 +1,16 @@
 package org.infinispan.api;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.transaction.NotSupportedException;
+import javax.transaction.SystemException;
+
 import org.infinispan.config.Configuration;
 import org.infinispan.config.ConfigurationException;
 import org.infinispan.lifecycle.ComponentStatus;
@@ -11,16 +22,6 @@
 import org.infinispan.util.concurrent.IsolationLevel;
 import org.testng.annotations.Test;
 
-import javax.transaction.NotSupportedException;
-import javax.transaction.SystemException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 /**
  * Tests the {@link org.infinispan.Cache} public API at a high level
  *
@@ -29,6 +30,7 @@
 @Test(groups = "functional")
 public abstract class CacheAPITest extends SingleCacheManagerTest {
 
+   @Override
    protected EmbeddedCacheManager createCacheManager() throws Exception {
       // start a single cache instance
       Configuration c = getDefaultStandaloneConfig(true);
@@ -181,10 +183,14 @@
       TestingUtil.getTransactionManager(cache).begin();
       cache.put(key2, value2);
       assert cache.get(key2).equals(value2);
-      assert !cache.keySet().contains(key2);
-      size = 1;
-      assert size == cache.size() && size == cache.keySet().size() && size == cache.values().size() && size == cache.entrySet().size();
-      assert !cache.values().contains(value2);
+      assert cache.keySet().contains(key2);
+      size = 2;
+      System.out.println(cache.size());
+      assert size == cache.size();
+      assert size == cache.keySet().size();
+      assert size == cache.values().size();
+      assert size == cache.entrySet().size();
+      assert cache.values().contains(value2);
       TestingUtil.getTransactionManager(cache).rollback();
 
       assert cache.get(key).equals(value);
@@ -210,7 +216,7 @@
       size = 1;
       assert size == cache.size() && size == cache.keySet().size() && size == cache.values().size() && size == cache.entrySet().size();
       assert cache.keySet().contains(key);
-      assert !cache.values().contains(value2);
+      assert cache.values().contains(value2);
       TestingUtil.getTransactionManager(cache).rollback();
 
       assert cache.get(key).equals(value);
@@ -233,7 +239,7 @@
       TestingUtil.getTransactionManager(cache).begin();
       cache.remove(key);
       assert cache.get(key) == null;
-      size = 1;
+      size = 0;
       assert size == cache.size() && size == cache.keySet().size() && size == cache.values().size() && size == cache.entrySet().size();
       TestingUtil.getTransactionManager(cache).rollback();
 
@@ -257,7 +263,7 @@
       TestingUtil.getTransactionManager(cache).begin();
       cache.clear();
       assert cache.get(key) == null;
-      size = 1;
+      size = 0;
       assert size == cache.size() && size == cache.keySet().size() && size == cache.values().size() && size == cache.entrySet().size();
       TestingUtil.getTransactionManager(cache).rollback();
 
@@ -368,11 +374,15 @@
       Set expValueEntries = ObjectDuplicator.duplicateSet(expValues);
 
       Set<Object> keys = cache.keySet();
-      for (Object key : keys) assert expKeys.remove(key);
+      for (Object key : keys) {
+         assert expKeys.remove(key);
+      }
       assert expKeys.isEmpty() : "Did not see keys " + expKeys + " in iterator!";
 
       Collection<Object> values = cache.values();
-      for (Object value : values) assert expValues.remove(value);
+      for (Object value : values) {
+         assert expValues.remove(value);
+      }
       assert expValues.isEmpty() : "Did not see keys " + expValues + " in iterator!";
 
       Set<Map.Entry<Object, Object>> entries = cache.entrySet();



More information about the infinispan-commits mailing list