Add documentation to PICFAndOfficeArtData

Adjust call sites based on defined behavior.

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

2 files changed, 30 insertions, 15 deletions

diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/PICFAndOfficeArtData.java b/src/scratchpad/src/org/apache/poi/hwpf/model/PICFAndOfficeArtData.java
index 85205b7bb1..fb34604169 100644
--- a/src/scratchpad/src/org/apache/poi/hwpf/model/PICFAndOfficeArtData.java
+++ b/src/scratchpad/src/org/apache/poi/hwpf/model/PICFAndOfficeArtData.java
@@ -20,8 +20,11 @@ import java.util.LinkedList;
 import java.util.List;
 
 import org.apache.poi.ddf.DefaultEscherRecordFactory;
+import org.apache.poi.ddf.EscherBSERecord;
+import org.apache.poi.ddf.EscherBlipRecord;
 import org.apache.poi.ddf.EscherContainerRecord;
 import org.apache.poi.ddf.EscherRecord;
+import org.apache.poi.ddf.EscherRecordTypes;
 import org.apache.poi.hwpf.model.types.PICFAbstractType;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.Internal;
@@ -34,13 +37,19 @@ public class PICFAndOfficeArtData
     //arbitrarily selected; may need to increase
     private static final int MAX_RECORD_LENGTH = 100_000;
 
-    private List<EscherRecord> _blipRecords;
+    /**
+     * Can contain either a {@link EscherBlipRecord} or a {@link EscherBSERecord}.
+     * <p>
+     * Should never contain more than 1 record.
+     */
+    private final List<EscherRecord> _blipRecords = new LinkedList<>();
 
-    private short _cchPicName;
+    private final PICF _picf;
 
-    private PICF _picf;
-
-    private EscherContainerRecord _shape;
+    /**
+     * A {@link EscherRecordTypes#SP_CONTAINER}.
+     */
+    private final EscherContainerRecord _shape = new EscherContainerRecord();
 
     private byte[] _stPicName;
 
@@ -53,7 +62,7 @@ public class PICFAndOfficeArtData
 
         if ( _picf.getMm() == 0x0066 )
         {
-            _cchPicName = LittleEndian.getUByte( dataStream, offset );
+            short _cchPicName = LittleEndian.getUByte(dataStream, offset);
             offset += 1;
 
             _stPicName = IOUtils.safelyClone(dataStream, offset, _cchPicName, MAX_RECORD_LENGTH);
@@ -61,12 +70,10 @@ public class PICFAndOfficeArtData
         }
 
         final DefaultEscherRecordFactory escherRecordFactory = new DefaultEscherRecordFactory();
-        _shape = new EscherContainerRecord();
         int recordSize = _shape.fillFields( dataStream, offset,
                 escherRecordFactory );
         offset += recordSize;
 
-        _blipRecords = new LinkedList<>();
         while ( ( offset - startOffset ) < _picf.getLcb() )
         {
             EscherRecord nextRecord = escherRecordFactory.createRecord(
@@ -81,9 +88,18 @@ public class PICFAndOfficeArtData
             offset += blipRecordSize;
 
             _blipRecords.add( nextRecord );
+
+            // [MS-ODRAW] allows for multiple records in a OfficeArtInlineSpContainer, which is what we're parsing here.
+            //   However, in the context of a HWPF document, there should be only 1.
+            assert _blipRecords.size() == 1;
         }
     }
 
+    /**
+     * Contains {@link EscherBlipRecord}s and {@link EscherBSERecord}s.
+     *
+     * @return List of BLIP records. Never {@code null}.
+     */
     public List<EscherRecord> getBlipRecords()
     {
         return _blipRecords;
@@ -94,6 +110,9 @@ public class PICFAndOfficeArtData
         return _picf;
     }
 
+    /**
+     * @return The {@link EscherRecordTypes#SP_CONTAINER}.
+     */
     public EscherContainerRecord getShape()
     {
         return _shape;
diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java
index 315e0be475..913b4950ae 100644
--- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java
+++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java
@@ -113,11 +113,7 @@ public final class Picture {
 
         this.dataBlockStartOfsset = dataBlockStartOfsset;
 
-        if ( _picfAndOfficeArtData.getBlipRecords() != null) {
-            _blipRecords = _picfAndOfficeArtData.getBlipRecords();
-        } else {
-            _blipRecords = Collections.emptyList();
-        }
+        _blipRecords = _picfAndOfficeArtData.getBlipRecords();
 
         if ( fillBytes ) {
             fillImageContent();
@@ -398,7 +394,7 @@ public final class Picture {
      */
     public byte[] getRawContent()
     {
-        if (_blipRecords == null || _blipRecords.size() != 1) {
+        if (_blipRecords.size() != 1) {
            return new byte[0];
         }
 
@@ -507,7 +503,7 @@ public final class Picture {
 
     public PictureType suggestPictureType()
     {
-        if (_blipRecords == null || _blipRecords.size() != 1 ) {
+        if (_blipRecords.size() != 1 ) {
             return PictureType.UNKNOWN;
         }