Based on reading of the new file format docs, start to tweak how fixed length vs variable length / multi-valued properties are handled

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1593861 13f79535-47bb-0310-9956-ffa450edef68

src/scratchpad/src/org/apache/poi/hsmf/datatypes/Chunks.java

@@ -90,6 +90,13 @@ public final class Chunks implements ChunkGroupWithProperties {
       }
       else return Collections.emptyMap();
    }
+   public Map<MAPIProperty, PropertyValue> getRawProperties() {
+      if (messageProperties != null) {
+         return messageProperties.getRawProperties();
+      }
+      else return Collections.emptyMap();
+   }
+   
    public Map<MAPIProperty,List<Chunk>> getAll() {
       return allChunks;
    }

src/scratchpad/src/org/apache/poi/hsmf/datatypes/PropertiesChunk.java

@@ -20,7 +20,7 @@ package org.apache.poi.hsmf.datatypes;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -42,9 +42,10 @@ import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
 
 /**
- * A Chunk which holds fixed-length properties, and pointer
+ * <p>A Chunk which holds (single) fixed-length properties, and pointer
- *  to the variable length ones (which get their own chunk).
+ *  to the variable length ones / multi-valued ones (which get their 
- * There are two kinds of PropertiesChunks, which differ only in 
+ *  own chunk).
+ * <p>There are two kinds of PropertiesChunks, which differ only in 
  *  their headers.
  */
 public abstract class PropertiesChunk extends Chunk {
@@ -53,16 +54,16 @@ public abstract class PropertiesChunk extends Chunk {
    /** For logging problems we spot with the file */
    private POILogger logger = POILogFactory.getLogger(PropertiesChunk.class);
 
-   
    /**
-    * Holds properties, indexed by type. Properties can be multi-valued
+    * Holds properties, indexed by type. If a property is multi-valued,
+    *  or variable length, it will be held via a {@link ChunkBasedPropertyValue}.
     */
-   private Map<MAPIProperty, List<PropertyValue>> properties = 
+   private Map<MAPIProperty, PropertyValue> properties = 
-         new HashMap<MAPIProperty, List<PropertyValue>>();
+         new HashMap<MAPIProperty, PropertyValue>();
 
    /**
     * The ChunkGroup that these properties apply to. Used when
-    *  matching chunks to variable sized properties
+    *  matching chunks to variable sized and multi-valued properties
     */
    private ChunkGroup parentGroup;
    
@@ -80,29 +81,51 @@ public abstract class PropertiesChunk extends Chunk {
    }
 	
    /**
-    * Returns all the properties in the chunk
+    * Returns all the properties in the chunk, without
+    *  looking up any chunk-based values
     */
-   public Map<MAPIProperty, List<PropertyValue>> getProperties() {
+   public Map<MAPIProperty, PropertyValue> getRawProperties() {
       return properties;
    }
 
    /**
-    * Returns all values for the given property, of null if none exist
+    * <p>Returns all the properties in the chunk, along with their
+    *  values.
+    * <p>Any chunk-based values will be looked up and returned as such
     */
-   public List<PropertyValue> getValues(MAPIProperty property) {
+   public Map<MAPIProperty, List<PropertyValue>> getProperties() {
-      return properties.get(property);
+       Map<MAPIProperty, List<PropertyValue>> props = 
+               new HashMap<MAPIProperty, List<PropertyValue>>(properties.size());
+       for (MAPIProperty prop : properties.keySet()) {
+           props.put(prop, getValues(prop));
+       }
+       return props;
    }
 
    /**
-    * Returns the (first/only) value for the given property, or
+    * Returns all values for the given property, looking up chunk based
-    *  null if none exist
+    *  ones as required, of null if none exist
     */
-   public PropertyValue getValue(MAPIProperty property) {
+   public List<PropertyValue> getValues(MAPIProperty property) {
-      List<PropertyValue> values = properties.get(property);
+       PropertyValue val = properties.get(property);
-      if (values != null && values.size() > 0) {
+       if (val == null) {
-         return values.get(0);
+           return null;
-      }
+       }
-      return null;
+       if (val instanceof ChunkBasedPropertyValue) {
+           ChunkBasedPropertyValue cval = (ChunkBasedPropertyValue)val;
+           // TODO Lookup
+           return Collections.emptyList();
+       } else {
+           return Collections.singletonList(val);
+       }
+   }
+
+   /**
+    * Returns the value / pointer to the value chunk of 
+    *  the property, or null if none exists
+    */
+   public PropertyValue getRawValue(MAPIProperty property) {
+       return properties.get(property);
    }
 	
    /**
@@ -118,23 +141,19 @@ public abstract class PropertiesChunk extends Chunk {
       }
       
       // Loop over our values, looking for chunk based ones
-      for (List<PropertyValue> vals : properties.values()) {
+      for (PropertyValue val : properties.values()) {
-         if (vals != null) {
+           if (val instanceof ChunkBasedPropertyValue) {
-            for (PropertyValue val : vals) {
+              ChunkBasedPropertyValue cVal = (ChunkBasedPropertyValue)val;
-               if (val instanceof ChunkBasedPropertyValue) {
+              Chunk chunk = chunks.get(cVal.getProperty().id);
-                  ChunkBasedPropertyValue cVal = (ChunkBasedPropertyValue)val;
+//System.err.println(cVal.getProperty() + " = " + cVal + " -> " + HexDump.toHex(cVal.data));                  
-                  Chunk chunk = chunks.get(cVal.getProperty().id);
-//System.err.println(cVal + " -> " + HexDump.toHex(cVal.data));                  
                   
-                  // TODO Make sense of the raw offset value
+              // TODO Make sense of the raw offset value
               
-                  if (chunk != null) {
+              if (chunk != null) {
-                     cVal.setValue(chunk);
+                 cVal.setValue(chunk);
-                  } else {
+              } else {
-                     logger.log(POILogger.WARN, "No chunk found matching Property " + cVal);
+                 logger.log(POILogger.WARN, "No chunk found matching Property " + cVal);
-                  }
+              }
-               }
-            }
          }
       }
    }
@@ -183,6 +202,10 @@ public abstract class PropertiesChunk extends Chunk {
                 }
             }
             
+            // TODO Detect if it is multi-valued, since if it is
+            //  then even fixed-length strings store their multiple
+            //  values in another chunk (much as variable length ones)
+            
             // Work out how long the "data" is
             // This might be the actual data, or just a pointer
             //  to another chunk which holds the data itself
@@ -241,10 +264,10 @@ public abstract class PropertiesChunk extends Chunk {
                 propVal = new PropertyValue(prop, flags, data);
             }
 
-            if (properties.get(prop) == null) {
+            if (properties.get(prop) != null) {
-                properties.put(prop, new ArrayList<PropertyValue>());
+                logger.log(POILogger.WARN, "Duplicate values found for " + prop);
             }
-            properties.get(prop).add(propVal);
+            properties.put(prop, propVal);
          } catch (BufferUnderrunException e) {
             // Invalid property, ended short
             going = false;

src/scratchpad/testcases/org/apache/poi/hsmf/TestFixedSizedProperties.java

@@ -31,6 +31,7 @@ import java.util.TimeZone;
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.POITestCase;
 import org.apache.poi.hsmf.datatypes.ChunkBasedPropertyValue;
+import org.apache.poi.hsmf.datatypes.Chunks;
 import org.apache.poi.hsmf.datatypes.MAPIProperty;
 import org.apache.poi.hsmf.datatypes.PropertyValue;
 import org.apache.poi.hsmf.datatypes.PropertyValue.LongPropertyValue;
@@ -86,9 +87,12 @@ public final class TestFixedSizedProperties extends POITestCase {
     * Check we find properties of a variety of different types
     */
    public void testPropertyValueTypes() throws Exception {
-       Map<MAPIProperty,List<PropertyValue>> props =
+       Chunks mainChunks = mapiMessageSucceeds.getMainChunks();
-               mapiMessageSucceeds.getMainChunks().getProperties();
+       
-       HashSet<Class<? extends PropertyValue>> seenTypes = new HashSet<Class<? extends PropertyValue>>();
+       // Ask to have the values looked up
+       Map<MAPIProperty,List<PropertyValue>> props = mainChunks.getProperties();
+       HashSet<Class<? extends PropertyValue>> seenTypes = 
+               new HashSet<Class<? extends PropertyValue>>();
        for (List<PropertyValue> pvs : props.values()) {
            for (PropertyValue pv : pvs) {
                seenTypes.add(pv.getClass());
@@ -97,6 +101,16 @@ public final class TestFixedSizedProperties extends POITestCase {
        assertTrue(seenTypes.toString(), seenTypes.size() > 3);
        assertTrue(seenTypes.toString(), seenTypes.contains(LongPropertyValue.class));
        assertTrue(seenTypes.toString(), seenTypes.contains(TimePropertyValue.class));
+       assertFalse(seenTypes.toString(), seenTypes.contains(ChunkBasedPropertyValue.class));
+       
+       // Ask for the raw values
+       seenTypes.clear();
+       for (PropertyValue pv : mainChunks.getRawProperties().values()) {
+           seenTypes.add(pv.getClass());
+       }
+       assertTrue(seenTypes.toString(), seenTypes.size() > 3);
+       assertTrue(seenTypes.toString(), seenTypes.contains(LongPropertyValue.class));
+       assertTrue(seenTypes.toString(), seenTypes.contains(TimePropertyValue.class));
        assertTrue(seenTypes.toString(), seenTypes.contains(ChunkBasedPropertyValue.class));
    }