SLING-10903 : Remove duplicate code from value map implementations
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMap.java b/src/main/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMap.java
index 37467ab..445edb1 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMap.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMap.java
@@ -18,413 +18,30 @@
  */
 package org.apache.sling.jcr.resource.internal;
 
-import java.util.Calendar;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Date;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
 import java.util.Map;
-import java.util.Set;
 
 import javax.jcr.Node;
-import javax.jcr.Property;
-import javax.jcr.PropertyIterator;
 import javax.jcr.RepositoryException;
 import javax.jcr.Value;
 
-import org.apache.jackrabbit.util.ISO9075;
-import org.apache.jackrabbit.util.Text;
 import org.apache.sling.api.resource.ModifiableValueMap;
 import org.apache.sling.jcr.resource.internal.helper.JcrPropertyMapCacheEntry;
 
 /**
- * This implementation of the value map allows to change
- * the properties.
- *
- * TODO : This adds a lot of duplicate code - we should consolidate.
+ * Modifiable value map implementation leveraging the base class
  */
-public final class JcrModifiableValueMap
+public class JcrModifiableValueMap
+    extends JcrValueMap
     implements ModifiableValueMap {
 
-    /** The underlying node. */
-    private final Node node;
-
-    /** A cache for the properties. */
-    private final Map<String, JcrPropertyMapCacheEntry> cache;
-
-    /** A cache for the values. */
-    private final Map<String, Object> valueCache;
-
-    /** Has the node been read completely? */
-    private boolean fullyRead;
-
-    private final HelperData helper;
-
     /**
      * Constructor
      * @param node The underlying node.
      * @param helper Helper data object
      */
     public JcrModifiableValueMap(final Node node, final HelperData helper) {
-        this.node = node;
-        this.cache = new LinkedHashMap<String, JcrPropertyMapCacheEntry>();
-        this.valueCache = new LinkedHashMap<String, Object>();
-        this.fullyRead = false;
-        this.helper = helper;
-    }
-
-    // ---------- ValueMap
-
-    private String checkKey(final String key) {
-        if ( key == null ) {
-            throw new NullPointerException("Key must not be null.");
-        }
-        if ( key.startsWith("./") ) {
-            return key.substring(2);
-        }
-        return key;
-    }
-
-    /**
-     * @see org.apache.sling.api.resource.ValueMap#get(java.lang.String, java.lang.Class)
-     */
-    @Override
-    @SuppressWarnings("unchecked")
-    public <T> T get(final String aKey, final Class<T> type) {
-        final String key = checkKey(aKey);
-        if (type == null) {
-            return (T) get(key);
-        }
-
-        final JcrPropertyMapCacheEntry entry = this.read(key);
-        if ( entry == null ) {
-            return null;
-        }
-        return entry.convertToType(type, node, helper.getDynamicClassLoader());
-    }
-
-    /**
-     * @see org.apache.sling.api.resource.ValueMap#get(java.lang.String, java.lang.Object)
-     */
-    @Override
-    @SuppressWarnings("unchecked")
-    public <T> T get(final String aKey,final T defaultValue) {
-        final String key = checkKey(aKey);
-        if (defaultValue == null) {
-            return (T) get(key);
-        }
-
-        // special handling in case the default value implements one
-        // of the interface types supported by the convertToType method
-        Class<T> type = (Class<T>) normalizeClass(defaultValue.getClass());
-
-        T value = get(key, type);
-        if (value == null) {
-            value = defaultValue;
-        }
-
-        return value;
-    }
-
-    // ---------- Map
-
-    /**
-     * @see java.util.Map#get(java.lang.Object)
-     */
-    @Override
-    public Object get(final Object aKey) {
-        final String key = checkKey(aKey.toString());
-        final JcrPropertyMapCacheEntry entry = this.read(key);
-        final Object value = (entry == null ? null : entry.getPropertyValueOrNull());
-        return value;
-    }
-
-    /**
-     * @see java.util.Map#containsKey(java.lang.Object)
-     */
-    @Override
-    public boolean containsKey(final Object key) {
-        return get(key) != null;
-    }
-
-    /**
-     * @see java.util.Map#containsValue(java.lang.Object)
-     */
-    @Override
-    public boolean containsValue(final Object value) {
-        readFully();
-        return valueCache.containsValue(value);
-    }
-
-    /**
-     * @see java.util.Map#isEmpty()
-     */
-    @Override
-    public boolean isEmpty() {
-        return size() == 0;
-    }
-
-    /**
-     * @see java.util.Map#size()
-     */
-    @Override
-    public int size() {
-        readFully();
-        return cache.size();
-    }
-
-    /**
-     * @see java.util.Map#entrySet()
-     */
-    @Override
-    public Set<java.util.Map.Entry<String, Object>> entrySet() {
-        readFully();
-        final Map<String, Object> sourceMap;
-        if (cache.size() == valueCache.size()) {
-            sourceMap = valueCache;
-        } else {
-            sourceMap = transformEntries(cache);
-        }
-        return Collections.unmodifiableSet(sourceMap.entrySet());
-    }
-
-    /**
-     * @see java.util.Map#keySet()
-     */
-    @Override
-    public Set<String> keySet() {
-        readFully();
-        return Collections.unmodifiableSet(cache.keySet());
-    }
-
-    /**
-     * @see java.util.Map#values()
-     */
-    @Override
-    public Collection<Object> values() {
-        readFully();
-        final Map<String, Object> sourceMap;
-        if (cache.size() == valueCache.size()) {
-            sourceMap = valueCache;
-        } else {
-            sourceMap = transformEntries(cache);
-        }
-        return Collections.unmodifiableCollection(sourceMap.values());
-    }
-
-    /**
-     * Return the path of the current node.
-     *
-     * @return the path
-     * @throws IllegalStateException If a repository exception occurs
-     */
-    public String getPath() {
-        try {
-            return node.getPath();
-        } catch (final RepositoryException e) {
-            throw new IllegalStateException(e);
-        }
-    }
-
-    // ---------- Helpers to access the node's property ------------------------
-
-    /**
-     * Put a single property into the cache
-     * @param prop
-     * @return
-     * @throws IllegalArgumentException if a repository exception occurs
-     */
-    private JcrPropertyMapCacheEntry cacheProperty(final Property prop) {
-        try {
-            // calculate the key
-            final String name = prop.getName();
-            String key = null;
-            if ( name.indexOf("_x") != -1 ) {
-                // for compatibility with older versions we use the (wrong)
-                // ISO9075 path encoding
-                key = ISO9075.decode(name);
-                if ( key.equals(name) ) {
-                    key = null;
-                }
-            }
-            if ( key == null ) {
-                key = Text.unescapeIllegalJcrChars(name);
-            }
-            JcrPropertyMapCacheEntry entry = cache.get(key);
-            if ( entry == null ) {
-                entry = new JcrPropertyMapCacheEntry(prop);
-                cache.put(key, entry);
-
-                final Object defaultValue = entry.getPropertyValue();
-                if (defaultValue != null) {
-                    valueCache.put(key, entry.getPropertyValue());
-                }
-            }
-            return entry;
-        } catch (final RepositoryException re) {
-            throw new IllegalArgumentException(re);
-        }
-    }
-
-    /**
-     * Read a single property.
-     * @throws IllegalArgumentException if a repository exception occurs
-     */
-    JcrPropertyMapCacheEntry read(final String name) {
-        // check for empty key
-        if ( name.length() == 0 ) {
-            return null;
-        }
-        // if the name is a path, we should handle this differently
-        if ( name.indexOf('/') != -1 ) {
-            // first a compatibility check with the old (wrong) ISO9075
-            // encoding
-            final String path = ISO9075.encodePath(name);
-            try {
-                if ( node.hasProperty(path) ) {
-                    return new JcrPropertyMapCacheEntry(node.getProperty(path));
-                }
-            } catch (final RepositoryException re) {
-                throw new IllegalArgumentException(re);
-            }
-            // now we do a proper segment by segment encoding
-            final StringBuilder sb = new StringBuilder();
-            int pos = 0;
-            int lastPos = -1;
-            while ( pos < name.length() ) {
-                if ( name.charAt(pos) == '/' ) {
-                    if ( lastPos + 1 < pos ) {
-                        sb.append(Text.escapeIllegalJcrChars(name.substring(lastPos + 1, pos)));
-                    }
-                    sb.append('/');
-                    lastPos = pos;
-                }
-                pos++;
-            }
-            if ( lastPos + 1 < pos ) {
-                sb.append(Text.escapeIllegalJcrChars(name.substring(lastPos + 1)));
-            }
-            final String newPath = sb.toString();
-            try {
-                if ( node.hasProperty(newPath) ) {
-                    return new JcrPropertyMapCacheEntry(node.getProperty(newPath));
-                }
-            } catch (final RepositoryException re) {
-                throw new IllegalArgumentException(re);
-            }
-
-            return null;
-        }
-
-        // check cache
-        JcrPropertyMapCacheEntry cachedValued = cache.get(name);
-        if ( fullyRead || cachedValued != null ) {
-            return cachedValued;
-        }
-
-        try {
-            final String key = escapeKeyName(name);
-            if (node.hasProperty(key)) {
-                final Property prop = node.getProperty(key);
-                return cacheProperty(prop);
-            }
-        } catch (final RepositoryException re) {
-            throw new IllegalArgumentException(re);
-        }
-
-        try {
-            // for compatibility with older versions we use the (wrong) ISO9075 path
-            // encoding
-            final String oldKey = ISO9075.encodePath(name);
-            if (node.hasProperty(oldKey)) {
-                final Property prop = node.getProperty(oldKey);
-                return cacheProperty(prop);
-            }
-        } catch (final RepositoryException re) {
-            // we ignore this
-        }
-
-        // property not found
-        return null;
-    }
-
-    /**
-     * Handles key name escaping by taking into consideration if it contains a
-     * registered prefix
-     *
-     * @param key the key to escape
-     * @return escaped key name
-     * @throws RepositoryException if the repository's namespace prefixes cannot be retrieved
-     */
-    protected String escapeKeyName(final String key) throws RepositoryException {
-        final int indexOfPrefix = key.indexOf(':');
-        // check if colon is neither the first nor the last character
-        if (indexOfPrefix > 0 && key.length() > indexOfPrefix + 1) {
-            final String prefix = key.substring(0, indexOfPrefix);
-            for (final String existingPrefix : this.helper.getNamespacePrefixes(this.node.getSession())) {
-                if (existingPrefix.equals(prefix)) {
-                    return prefix
-                            + ":"
-                            + Text.escapeIllegalJcrChars(key
-                                    .substring(indexOfPrefix + 1));
-                }
-            }
-        }
-        return Text.escapeIllegalJcrChars(key);
-    }
-
-    /**
-     * Read all properties.
-     * @throws IllegalArgumentException if a repository exception occurs
-     */
-    void readFully() {
-        if (!fullyRead) {
-            try {
-                final PropertyIterator pi = node.getProperties();
-                while (pi.hasNext()) {
-                    final Property prop = pi.nextProperty();
-                    this.cacheProperty(prop);
-                }
-                fullyRead = true;
-            } catch (final RepositoryException re) {
-                throw new IllegalArgumentException(re);
-            }
-        }
-    }
-
-    // ---------- Implementation helper
-
-    private Class<?> normalizeClass(Class<?> type) {
-        if (Calendar.class.isAssignableFrom(type)) {
-            type = Calendar.class;
-        } else if (Date.class.isAssignableFrom(type)) {
-            type = Date.class;
-        } else if (Value.class.isAssignableFrom(type)) {
-            type = Value.class;
-        } else if (Property.class.isAssignableFrom(type)) {
-            type = Property.class;
-        }
-        return type;
-    }
-
-    private Map<String, Object> transformEntries(final Map<String, JcrPropertyMapCacheEntry> map) {
-
-        final Map<String, Object> transformedEntries = new LinkedHashMap<String, Object>(map.size());
-        for ( final Map.Entry<String, JcrPropertyMapCacheEntry> entry : map.entrySet() )
-            transformedEntries.put(entry.getKey(), entry.getValue().getPropertyValueOrNull());
-
-        return transformedEntries;
-    }
-
-    // ---------- Map
-
-    /**
-     * @see java.util.Map#clear()
-     */
-    @Override
-    public void clear() {
-        throw new UnsupportedOperationException("clear");
+        super(node, helper);
     }
 
     /**
@@ -455,21 +72,13 @@
                 node.setProperty(name, entry.convertToType(Value.class, node, this.helper.getDynamicClassLoader()));
             }
         } catch (final RepositoryException re) {
-            throw new IllegalArgumentException("Value '"+ value + "' for property '" + key + "' can't be put into node '" + getNodePath(node) + "'.", re);
+            throw new IllegalArgumentException("Value '"+ value + "' for property '" + key + "' can't be put into node '" + getPath() + "'.", re);
         }
         this.valueCache.put(key, value);
 
         return oldValue;
     }
 
-    static String getNodePath(Node node) {
-        try {
-            return node.getPath();
-        } catch (RepositoryException e) {
-            return "Could not get node path: "+ e.getMessage();
-        }
-    }
-
     /**
      * @see java.util.Map#putAll(java.util.Map)
      */
@@ -500,7 +109,7 @@
                 node.getProperty(name).remove();
             }
         } catch (final RepositoryException re) {
-            throw new IllegalArgumentException("Property '" + key + "' can't be removed from node '" + getNodePath(node) + "'.", re);
+            throw new IllegalArgumentException("Property '" + key + "' can't be removed from node '" + getPath() + "'.", re);
         }
 
         return oldValue;
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java b/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java
index afe3c57..ac9dc78 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java
@@ -35,49 +35,45 @@
 
 import org.apache.jackrabbit.util.ISO9075;
 import org.apache.jackrabbit.util.Text;
+import org.apache.sling.api.resource.ModifiableValueMap;
 import org.apache.sling.api.resource.ValueMap;
 import org.apache.sling.jcr.resource.internal.helper.JcrPropertyMapCacheEntry;
 
-public class JcrValueMap implements ValueMap {
-
-    private final HelperData helper;
+/**
+ * This implementation of the value map allows to change
+ * the properties.
+ */
+public class JcrValueMap
+    implements ValueMap {
 
     /** The underlying node. */
-    private final Node node;
+    protected final Node node;
 
     /** A cache for the properties. */
-    final Map<String, JcrPropertyMapCacheEntry> cache;
+    protected final Map<String, JcrPropertyMapCacheEntry> cache = new LinkedHashMap<String, JcrPropertyMapCacheEntry>();
 
     /** A cache for the values. */
-    final Map<String, Object> valueCache;
+    protected final Map<String, Object> valueCache = new LinkedHashMap<String, Object>();
 
     /** Has the node been read completely? */
-    boolean fullyRead;
+    private boolean fullyRead = false;
+
+    /** Helper data object */
+    protected final HelperData helper;
 
     /**
-     * Create a new JCR property map based on a node.
+     * Constructor
      * @param node The underlying node.
+     * @param helper Helper data object
      */
     public JcrValueMap(final Node node, final HelperData helper) {
         this.node = node;
-        this.cache = new LinkedHashMap<String, JcrPropertyMapCacheEntry>();
-        this.valueCache = new LinkedHashMap<String, Object>();
-        this.fullyRead = false;
         this.helper = helper;
     }
 
-    /**
-     * Get the node.
-     *
-     * @return the node
-     */
-    protected Node getNode() {
-        return node;
-    }
-
     // ---------- ValueMap
 
-    String checkKey(final String key) {
+    protected String checkKey(final String key) {
         if ( key == null ) {
             throw new NullPointerException("Key must not be null.");
         }
@@ -102,7 +98,7 @@
         if ( entry == null ) {
             return null;
         }
-        return entry.convertToType(type, this.node, this.getDynamicClassLoader());
+        return entry.convertToType(type, node, helper.getDynamicClassLoader());
     }
 
     /**
@@ -196,7 +192,7 @@
     @Override
     public Set<String> keySet() {
         readFully();
-        return cache.keySet();
+        return Collections.unmodifiableSet(cache.keySet());
     }
 
     /**
@@ -219,9 +215,7 @@
      *
      * @return the path
      * @throws IllegalStateException If a repository exception occurs
-     * @deprecated
      */
-    @Deprecated
     public String getPath() {
         try {
             return node.getPath();
@@ -235,7 +229,7 @@
     /**
      * Put a single property into the cache
      * @param prop
-     * @return the cached property
+     * @return
      * @throws IllegalArgumentException if a repository exception occurs
      */
     private JcrPropertyMapCacheEntry cacheProperty(final Property prop) {
@@ -326,9 +320,8 @@
             return cachedValued;
         }
 
-        final String key;
         try {
-            key = escapeKeyName(name);
+            final String key = escapeKeyName(name);
             if (node.hasProperty(key)) {
                 final Property prop = node.getProperty(key);
                 return cacheProperty(prop);
@@ -341,7 +334,7 @@
             // for compatibility with older versions we use the (wrong) ISO9075 path
             // encoding
             final String oldKey = ISO9075.encodePath(name);
-            if (!oldKey.equals(key) && node.hasProperty(oldKey)) {
+            if (node.hasProperty(oldKey)) {
                 final Property prop = node.getProperty(oldKey);
                 return cacheProperty(prop);
             }
@@ -359,14 +352,14 @@
      *
      * @param key the key to escape
      * @return escaped key name
-     * @throws RepositoryException if the repository's namespaced prefixes cannot be retrieved
+     * @throws RepositoryException if the repository's namespace prefixes cannot be retrieved
      */
     protected String escapeKeyName(final String key) throws RepositoryException {
         final int indexOfPrefix = key.indexOf(':');
         // check if colon is neither the first nor the last character
         if (indexOfPrefix > 0 && key.length() > indexOfPrefix + 1) {
             final String prefix = key.substring(0, indexOfPrefix);
-            for (final String existingPrefix : getNamespacePrefixes()) {
+            for (final String existingPrefix : this.helper.getNamespacePrefixes(this.node.getSession())) {
                 if (existingPrefix.equals(prefix)) {
                     return prefix
                             + ":"
@@ -397,28 +390,6 @@
         }
     }
 
-    // ---------- Unsupported Modification methods
-
-    @Override
-    public void clear() {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public Object put(String key, Object value) {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public void putAll(Map<? extends String, ? extends Object> t) {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public Object remove(Object key) {
-        throw new UnsupportedOperationException();
-    }
-
     // ---------- Implementation helper
 
     private Class<?> normalizeClass(Class<?> type) {
@@ -434,19 +405,58 @@
         return type;
     }
 
-    private Map<String, Object> transformEntries( Map<String, JcrPropertyMapCacheEntry> map) {
+    private Map<String, Object> transformEntries(final Map<String, JcrPropertyMapCacheEntry> map) {
 
-        Map<String, Object> transformedEntries = new LinkedHashMap<String, Object>(map.size());
-        for ( Map.Entry<String, JcrPropertyMapCacheEntry> entry : map.entrySet() )
+        final Map<String, Object> transformedEntries = new LinkedHashMap<String, Object>(map.size());
+        for ( final Map.Entry<String, JcrPropertyMapCacheEntry> entry : map.entrySet() )
             transformedEntries.put(entry.getKey(), entry.getValue().getPropertyValueOrNull());
 
         return transformedEntries;
     }
 
+    // ---------- Map
+
+    /**
+     * @see java.util.Map#clear()
+     */
+    @Override
+    public void clear() {
+        throw new UnsupportedOperationException("clear");
+    }
+
+    /**
+     * @see java.util.Map#put(java.lang.Object, java.lang.Object)
+     */
+    @Override
+    public Object put(final String aKey, final Object value) {
+        throw new UnsupportedOperationException();
+    }
+
+    /**
+     * @see java.util.Map#putAll(java.util.Map)
+     */
+    @Override
+    public void putAll(final Map<? extends String, ? extends Object> t) {
+        throw new UnsupportedOperationException();
+    }
+
+    /**
+     * @see java.util.Map#remove(java.lang.Object)
+     */
+    @Override
+    public Object remove(final Object aKey) {
+        throw new UnsupportedOperationException();
+    }
 
     @Override
     public String toString() {
-        final StringBuilder sb = new StringBuilder("JcrPropertyMap [node=");
+        final StringBuilder sb = new StringBuilder();
+        if ( this instanceof ModifiableValueMap ) {
+            sb.append("JcrModifiablePropertyMap");
+        } else {
+            sb.append("JcrPropertyMap");
+        }
+        sb.append(" [node=");
         sb.append(this.node);
         sb.append(", values={");
         final Iterator<Map.Entry<String, Object>> iter = this.entrySet().iterator();
@@ -465,12 +475,4 @@
         sb.append("}]");
         return sb.toString();
     }
-
-    private String[] getNamespacePrefixes() throws RepositoryException {
-        return this.helper.getNamespacePrefixes(this.getNode().getSession());
-    }
-
-    private ClassLoader getDynamicClassLoader() {
-        return helper.getDynamicClassLoader();
-    }
 }
diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResource.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResource.java
index 70596ad..4ff50af 100644
--- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResource.java
+++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResource.java
@@ -133,7 +133,7 @@
         } else if (type == InputStream.class) {
             return (Type) getInputStream(); // unchecked cast
         } else if (type == Map.class || type == ValueMap.class) {
-            return (Type) new JcrValueMap(getNode(), this.helper); // unchecked cast
+            return (Type) new JcrValueMap(getNode(), this.helper);
         } else if (type == ModifiableValueMap.class ) {
             // check write
             try {
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java
index ac9e476..073e7e8 100644
--- a/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java
+++ b/src/test/java/org/apache/sling/jcr/resource/internal/JcrModifiableValueMapTest.java
@@ -151,8 +151,8 @@
         InputStream stream = new ByteArrayInputStream(TEST_BYTE_ARRAY);
         pvm.put("binary", stream);
         getSession().save();
-        final ModifiableValueMap modifiableValueMap2 = new JcrModifiableValueMap(this.rootNode, getHelperData());
-        assertTrue("The read stream is not what we wrote.", IOUtils.toString(modifiableValueMap2.get("binary", InputStream.class)).equals
+        final ValueMap valueMap2 = new JcrValueMap(this.rootNode, getHelperData());
+        assertTrue("The read stream is not what we wrote.", IOUtils.toString(valueMap2.get("binary", InputStream.class)).equals
                 (TEST_BYTE_ARRAY_TO_STRING));
     }
 
@@ -175,7 +175,7 @@
         getSession().save();
         assertContains(pvm, currentlyStored);
 
-        final ModifiableValueMap pvm2 = new JcrModifiableValueMap(this.rootNode, getHelperData());
+        final ValueMap pvm2 = new JcrValueMap(this.rootNode, getHelperData());
         assertContains(pvm2, currentlyStored);
     }
 
@@ -215,7 +215,7 @@
 
         getSession().save();
 
-        final ModifiableValueMap pvm2 = new JcrModifiableValueMap(this.rootNode, getHelperData());
+        final ValueMap pvm2 = new JcrValueMap(this.rootNode, getHelperData());
         // check if we get the list again
         @SuppressWarnings("unchecked")
         final List<String> strings3 = (List<String>) pvm2.get("something", Serializable.class);
@@ -298,7 +298,7 @@
         getSession().save();
 
         // read with property map
-        final ValueMap vm = new JcrModifiableValueMap(testNode, getHelperData());
+        final ValueMap vm = new JcrValueMap(testNode, getHelperData());
         assertEquals(VALUE, vm.get(TEST_PATH));
         assertEquals(VALUE1, vm.get(PROP1));
         assertEquals(VALUE2, vm.get(PROP2));
@@ -329,7 +329,7 @@
         getSession().save();
 
         // read with property map
-        final ValueMap vm = new JcrModifiableValueMap(testNode, getHelperData());
+        final ValueMap vm = new JcrValueMap(testNode, getHelperData());
         assertEquals(VALUE3, vm.get(PROP3));
         assertEquals(VALUE3, vm.get("jcr:a:b"));
         assertEquals(VALUE3, vm.get("jcr:"));
@@ -383,7 +383,7 @@
         getSession().save();
 
         // read with property map
-        final ValueMap vm = new JcrModifiableValueMap(testNode, getHelperData());
+        final ValueMap vm = new JcrValueMap(testNode, getHelperData());
         assertEquals(dateValue1, vm.get(PROP1, Date.class));
         assertEqualsCalendar(calendarValue1, vm.get(PROP1, Calendar.class));
         assertEquals(dateValue2, vm.get(PROP2, Date.class));
@@ -425,7 +425,7 @@
         getSession().save();
 
         // read with property map
-        final ValueMap vm = new JcrModifiableValueMap(testNode, getHelperData());
+        final ValueMap vm = new JcrValueMap(testNode, getHelperData());
 
         // check types
         assertTrue(vm.get(PROP1) instanceof Calendar);
diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResourceTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResourceTest.java
index e576915..56f30ca 100644
--- a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResourceTest.java
+++ b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResourceTest.java
@@ -20,6 +20,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -30,8 +31,10 @@
 import javax.jcr.Session;
 
 import org.apache.jackrabbit.JcrConstants;
+import org.apache.sling.api.resource.ModifiableValueMap;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceMetadata;
+import org.apache.sling.api.resource.ValueMap;
 import org.apache.sling.api.resource.external.URIProvider;
 import org.apache.sling.commons.classloader.DynamicClassLoaderManager;
 import org.apache.sling.jcr.resource.api.JcrResourceConstants;
@@ -285,4 +288,70 @@
         assertEquals(TEST_TYPE, rm.getContentType());
         assertEquals(TEST_ENCODING, rm.getCharacterEncoding());
     }
+
+    public void testAdaptToValueMap() throws Exception {
+        final String name = "adaptablevm";
+        Node res = rootNode.addNode(name, JcrConstants.NT_UNSTRUCTURED);
+        setupResource(res);
+        getSession().save();
+
+        res = rootNode.getNode(name);
+        JcrNodeResource jnr = new JcrNodeResource(null, res.getPath(), null, res, getHelperData());
+
+        final ValueMap props = jnr.adaptTo(ValueMap.class);
+        assertFalse(props instanceof ModifiableValueMap);
+        assertNotNull(props);
+        assertFalse(props.isEmpty());
+
+        // assert all properties set up
+        assertEquals(TEST_MODIFIED, props.get(JcrConstants.JCR_LASTMODIFIED));
+        assertEquals(TEST_TYPE, props.get(JcrConstants.JCR_MIMETYPE));
+        assertEquals(TEST_ENCODING, props.get(JcrConstants.JCR_ENCODING));
+        assertEquals(TEST_DATA, (InputStream) props.get(JcrConstants.JCR_DATA));
+
+        try {
+            props.remove(JcrConstants.JCR_MIMETYPE);
+            fail();
+        } catch ( final UnsupportedOperationException uoe) {
+            // expected
+        }
+
+        try {
+            props.put(JcrConstants.JCR_MIMETYPE, "all");
+            fail();
+        } catch ( final UnsupportedOperationException uoe) {
+            // expected
+        }
+
+        try {
+            props.putAll(Collections.singletonMap(JcrConstants.JCR_MIMETYPE, "value"));
+            fail();
+        } catch ( final UnsupportedOperationException uoe) {
+            // expected
+        }
+    }
+
+    public void testAdaptToModifiableValueMap() throws Exception {
+        final String name = "adaptablemvm";
+        Node res = rootNode.addNode(name, JcrConstants.NT_UNSTRUCTURED);
+        setupResource(res);
+        getSession().save();
+
+        res = rootNode.getNode(name);
+        JcrNodeResource jnr = new JcrNodeResource(null, res.getPath(), null, res, getHelperData());
+
+        final ModifiableValueMap props = jnr.adaptTo(ModifiableValueMap.class);
+        assertNotNull(props);
+        assertFalse(props.isEmpty());
+
+        // assert all properties set up
+        assertEquals(TEST_MODIFIED, props.get(JcrConstants.JCR_LASTMODIFIED));
+        assertEquals(TEST_TYPE, props.get(JcrConstants.JCR_MIMETYPE));
+        assertEquals(TEST_ENCODING, props.get(JcrConstants.JCR_ENCODING));
+        assertEquals(TEST_DATA, (InputStream) props.get(JcrConstants.JCR_DATA));
+
+        props.remove(JcrConstants.JCR_MIMETYPE);
+        props.put(JcrConstants.JCR_MIMETYPE, "all");
+        props.putAll(Collections.singletonMap(JcrConstants.JCR_MIMETYPE, "value"));
+    }
 }