#64036 - Replace reflection calls in factories for Java 9+ - Escher factories

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

src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java

@@ -17,11 +17,12 @@
 
 package org.apache.poi.ddf;
 
-import java.lang.reflect.Constructor;
+import java.util.function.Supplier;
-import java.util.HashMap;
-import java.util.Map;
 
+import org.apache.poi.util.BitField;
+import org.apache.poi.util.BitFieldFactory;
 import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.Removal;
 
 /**
  * Generates escher records when provided the byte array containing those records.
@@ -29,14 +30,7 @@ import org.apache.poi.util.LittleEndian;
  * @see EscherRecordFactory
  */
 public class DefaultEscherRecordFactory implements EscherRecordFactory {
-    private static Class<?>[] escherRecordClasses = { EscherBSERecord.class,
+    private static final BitField IS_CONTAINER = BitFieldFactory.getInstance(0xF);
-            EscherOptRecord.class, EscherTertiaryOptRecord.class,
-            EscherClientAnchorRecord.class, EscherDgRecord.class,
-            EscherSpgrRecord.class, EscherSpRecord.class,
-            EscherClientDataRecord.class, EscherDggRecord.class,
-            EscherSplitMenuColorsRecord.class, EscherChildAnchorRecord.class,
-            EscherTextboxRecord.class };
-    private static Map<Short, Constructor<? extends EscherRecord>> recordsMap = recordsToMap( escherRecordClasses );
 
     /**
      * Creates an instance of the escher record factory
@@ -51,86 +45,41 @@ public class DefaultEscherRecordFactory implements EscherRecordFactory {
         short recordId = LittleEndian.getShort( data, offset + 2 );
         // int remainingBytes = LittleEndian.getInt( data, offset + 4 );
 
-        // Options of 0x000F means container record
+        final EscherRecord escherRecord = getConstructor(options, recordId).get();
-        // However, EscherTextboxRecord are containers of records for the
-        //  host application, not of other Escher records, so treat them
-        //  differently
-        if (isContainer(options, recordId)) {
-            EscherContainerRecord r = new EscherContainerRecord();
-            r.setRecordId( recordId );
-            r.setOptions( options );
-            return r;
-        }
-
-        if (recordId >= EscherBlipRecord.RECORD_ID_START
-                && recordId <= EscherBlipRecord.RECORD_ID_END) {
-            EscherBlipRecord r;
-            if (recordId == EscherBitmapBlip.RECORD_ID_DIB ||
-                    recordId == EscherBitmapBlip.RECORD_ID_JPEG ||
-                    recordId == EscherBitmapBlip.RECORD_ID_PNG)
-            {
-                r = new EscherBitmapBlip();
-            }
-            else if (recordId == EscherMetafileBlip.RECORD_ID_EMF ||
-                    recordId == EscherMetafileBlip.RECORD_ID_WMF ||
-                    recordId == EscherMetafileBlip.RECORD_ID_PICT)
-            {
-                r = new EscherMetafileBlip();
-            } else {
-                r = new EscherBlipRecord();
-            }
-            r.setRecordId( recordId );
-            r.setOptions( options );
-            return r;
-        }
-
-        Constructor<? extends EscherRecord> recordConstructor = recordsMap.get(Short.valueOf(recordId));
-        final EscherRecord escherRecord;
-        if (recordConstructor == null) {
-            return new UnknownEscherRecord();
-        }
-        try {
-            escherRecord = recordConstructor.newInstance();
-        } catch (Exception e) {
-            return new UnknownEscherRecord();
-        }
         escherRecord.setRecordId(recordId);
         escherRecord.setOptions(options);
         return escherRecord;
     }
 
-    /**
+    protected Supplier<? extends EscherRecord> getConstructor(short options, short recordId) {
-     * Converts from a list of classes into a map that contains the record id as the key and
+        EscherRecordTypes recordTypes = EscherRecordTypes.forTypeID(recordId);
-     * the Constructor in the value part of the map.  It does this by using reflection to look up
-     * the RECORD_ID field then using reflection again to find a reference to the constructor.
-     *
-     * @param recClasses The records to convert
-     * @return The map containing the id/constructor pairs.
-     */
-    protected static Map<Short, Constructor<? extends EscherRecord>> recordsToMap(Class<?>[] recClasses) {
-        Map<Short, Constructor<? extends EscherRecord>> result = new HashMap<>();
-        final Class<?>[] EMPTY_CLASS_ARRAY = new Class[0];
 
-        for (Class<?> recClass : recClasses) {
+        // Options of 0x000F means container record
-            @SuppressWarnings("unchecked")
+        // However, EscherTextboxRecord are containers of records for the host application,
-            Class<? extends EscherRecord> recCls = (Class<? extends EscherRecord>) recClass;
+        // not of other Escher records, but those are returned by the above anyway
-            short sid;
+        if (recordTypes == EscherRecordTypes.UNKNOWN && IS_CONTAINER.isAllSet(options)) {
-            try {
+            return EscherContainerRecord::new;
-                sid = recCls.getField("RECORD_ID").getShort(null);
-            } catch (IllegalArgumentException | NoSuchFieldException | IllegalAccessException e) {
-                throw new RuntimeException(e);
-            }
-            Constructor<? extends EscherRecord> constructor;
-            try {
-                constructor = recCls.getConstructor(EMPTY_CLASS_ARRAY);
-            } catch (NoSuchMethodException e) {
-                throw new RuntimeException(e);
-            }
-            result.put(Short.valueOf(sid), constructor);
         }
-        return result;
+
+        if (recordTypes.constructor != null) {
+            return recordTypes.constructor;
+        }
+
+        // handle unknown blip records
+        if (EscherBlipRecord.RECORD_ID_START <= recordId && recordId <= EscherBlipRecord.RECORD_ID_END) {
+            return EscherBlipRecord::new;
+        }
+
+        // catch all
+        return UnknownEscherRecord::new;
     }
 
+
+    /**
+     * @deprecated this method is not used anymore to identify container records
+     */
+    @Deprecated
+    @Removal(version = "5.0.0")
     public static boolean isContainer(short options, short recordId){
         if(recordId >= EscherContainerRecord.DGG_CONTAINER &&  recordId
                 <= EscherContainerRecord.SOLVER_CONTAINER){

src/java/org/apache/poi/ddf/EscherClientDataRecord.java

@@ -30,11 +30,13 @@ import org.apache.poi.util.LittleEndian;
  * shape within a container.
  */
 public class EscherClientDataRecord extends EscherRecord {
-    //arbitrarily selected; may need to increase
-    private static final int MAX_RECORD_LENGTH = 100_000;
 
     public static final short RECORD_ID = EscherRecordTypes.CLIENT_DATA.typeID;
 
+    //arbitrarily selected; may need to increase
+    private static final int MAX_RECORD_LENGTH = 100_000;
+    private static final byte[] EMPTY = {};
+
     private byte[] remainingData;
 
     public EscherClientDataRecord() {}
@@ -48,7 +50,7 @@ public class EscherClientDataRecord extends EscherRecord {
     public int fillFields(byte[] data, int offset, EscherRecordFactory recordFactory) {
         int bytesRemaining = readHeader( data, offset );
         int pos            = offset + 8;
-        remainingData = IOUtils.safelyAllocate(bytesRemaining, MAX_RECORD_LENGTH);
+        remainingData = (bytesRemaining == 0) ? EMPTY : IOUtils.safelyAllocate(bytesRemaining, MAX_RECORD_LENGTH);
         System.arraycopy( data, pos, remainingData, 0, bytesRemaining );
         return 8 + bytesRemaining;
     }
@@ -58,7 +60,7 @@ public class EscherClientDataRecord extends EscherRecord {
         listener.beforeRecordSerialize( offset, getRecordId(), this );
 
         if (remainingData == null) {
-            remainingData = new byte[0];
+            remainingData = EMPTY;
         }
         LittleEndian.putShort( data, offset, getOptions() );
         LittleEndian.putShort( data, offset + 2, getRecordId() );

src/java/org/apache/poi/ddf/EscherPictBlip.java

@@ -1,322 +0,0 @@
-/* ====================================================================
-   Licensed to the Apache Software Foundation (ASF) under one or more
-   contributor license agreements.  See the NOTICE file distributed with
-   this work for additional information regarding copyright ownership.
-   The ASF licenses this file to You under the Apache License, Version 2.0
-   (the "License"); you may not use this file except in compliance with
-   the License.  You may obtain a copy of the License at
-
-       http://www.apache.org/licenses/LICENSE-2.0
-
-   Unless required by applicable law or agreed to in writing, software
-   distributed under the License is distributed on an "AS IS" BASIS,
-   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-   See the License for the specific language governing permissions and
-   limitations under the License.
-==================================================================== */
-
-package org.apache.poi.ddf;
-
-import java.awt.Dimension;
-import java.awt.Rectangle;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.util.Collections;
-import java.util.LinkedHashMap;
-import java.util.Map;
-import java.util.function.Supplier;
-import java.util.zip.InflaterInputStream;
-
-import org.apache.poi.util.IOUtils;
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.POILogFactory;
-import org.apache.poi.util.POILogger;
-
-public final class EscherPictBlip extends EscherBlipRecord {
-    private static final POILogger log = POILogFactory.getLogger(EscherPictBlip.class);
-    //arbitrarily selected; may need to increase
-    private static final int MAX_RECORD_LENGTH = 100_000;
-
-    public static final short RECORD_ID_EMF = EscherRecordTypes.BLIP_EMF.typeID;
-    public static final short RECORD_ID_WMF = EscherRecordTypes.BLIP_WMF.typeID;
-    public static final short RECORD_ID_PICT = EscherRecordTypes.BLIP_PICT.typeID;
-
-    private static final int HEADER_SIZE = 8;
-
-    private final byte[] field_1_UID = new byte[16];
-    private int field_2_cb;
-    private int field_3_rcBounds_x1;
-    private int field_3_rcBounds_y1;
-    private int field_3_rcBounds_x2;
-    private int field_3_rcBounds_y2;
-    private int field_4_ptSize_w;
-    private int field_4_ptSize_h;
-    private int field_5_cbSave;
-    private byte field_6_fCompression;
-    private byte field_7_fFilter;
-
-    private byte[] raw_pictureData;
-
-    public EscherPictBlip() {}
-
-    public EscherPictBlip(EscherPictBlip other) {
-        super(other);
-        System.arraycopy(other.field_1_UID, 0, field_1_UID, 0, field_1_UID.length);
-        field_2_cb = other.field_2_cb;
-        field_3_rcBounds_x1 = other.field_3_rcBounds_x1;
-        field_3_rcBounds_y1 = other.field_3_rcBounds_y1;
-        field_3_rcBounds_x2 = other.field_3_rcBounds_x2;
-        field_3_rcBounds_y2 = other.field_3_rcBounds_y2;
-        field_4_ptSize_w = other.field_4_ptSize_w;
-        field_4_ptSize_h = other.field_4_ptSize_h;
-        field_5_cbSave = other.field_5_cbSave;
-        field_6_fCompression = other.field_6_fCompression;
-        field_7_fFilter = other.field_7_fFilter;
-        raw_pictureData = (other.raw_pictureData == null) ? null : other.raw_pictureData.clone();
-    }
-
-    @Override
-    public int fillFields(byte[] data, int offset, EscherRecordFactory recordFactory) {
-        int bytesAfterHeader = readHeader(data, offset);
-        int pos = offset + HEADER_SIZE;
-
-        System.arraycopy( data, pos, field_1_UID, 0, 16 ); pos += 16;
-        field_2_cb = LittleEndian.getInt( data, pos ); pos += 4;
-        field_3_rcBounds_x1 = LittleEndian.getInt( data, pos ); pos += 4;
-        field_3_rcBounds_y1 = LittleEndian.getInt( data, pos ); pos += 4;
-        field_3_rcBounds_x2 = LittleEndian.getInt( data, pos ); pos += 4;
-        field_3_rcBounds_y2 = LittleEndian.getInt( data, pos ); pos += 4;
-        field_4_ptSize_w = LittleEndian.getInt( data, pos ); pos += 4;
-        field_4_ptSize_h = LittleEndian.getInt( data, pos ); pos += 4;
-        field_5_cbSave = LittleEndian.getInt( data, pos ); pos += 4;
-        field_6_fCompression = data[pos]; pos++;
-        field_7_fFilter = data[pos]; pos++;
-
-        raw_pictureData = IOUtils.safelyAllocate(field_5_cbSave, MAX_RECORD_LENGTH);
-        System.arraycopy( data, pos, raw_pictureData, 0, field_5_cbSave );
-
-        // 0 means DEFLATE compression
-        // 0xFE means no compression
-        if (field_6_fCompression == 0)
-        {
-        	super.setPictureData(inflatePictureData(raw_pictureData));
-        }
-        else
-        {
-        	super.setPictureData(raw_pictureData);
-        }
-
-        return bytesAfterHeader + HEADER_SIZE;
-    }
-
-    @Override
-    public int serialize(int offset, byte[] data, EscherSerializationListener listener) {
-        listener.beforeRecordSerialize(offset, getRecordId(), this);
-
-        int pos = offset;
-        LittleEndian.putShort( data, pos, getOptions() ); pos += 2;
-        LittleEndian.putShort( data, pos, getRecordId() ); pos += 2;
-        LittleEndian.putInt( data, 0, getRecordSize() - HEADER_SIZE ); pos += 4;
-
-        System.arraycopy( field_1_UID, 0, data, pos, 16 ); pos += 16;
-        LittleEndian.putInt( data, pos, field_2_cb ); pos += 4;
-        LittleEndian.putInt( data, pos, field_3_rcBounds_x1 ); pos += 4;
-        LittleEndian.putInt( data, pos, field_3_rcBounds_y1 ); pos += 4;
-        LittleEndian.putInt( data, pos, field_3_rcBounds_x2 ); pos += 4;
-        LittleEndian.putInt( data, pos, field_3_rcBounds_y2 ); pos += 4;
-        LittleEndian.putInt( data, pos, field_4_ptSize_w ); pos += 4;
-        LittleEndian.putInt( data, pos, field_4_ptSize_h ); pos += 4;
-        LittleEndian.putInt( data, pos, field_5_cbSave ); pos += 4;
-        data[pos] = field_6_fCompression; pos++;
-        data[pos] = field_7_fFilter; pos++;
-
-        System.arraycopy( raw_pictureData, 0, data, pos, raw_pictureData.length );
-
-        listener.afterRecordSerialize(offset + getRecordSize(), getRecordId(), getRecordSize(), this);
-        return HEADER_SIZE + 16 + 1 + raw_pictureData.length;
-    }
-
-    /**
-     * Decompresses the provided data, returning the inflated result.
-     *
-     * @param data the deflated picture data.
-     * @return the inflated picture data.
-     */
-    private static byte[] inflatePictureData(byte[] data) {
-        try {
-            InflaterInputStream in = new InflaterInputStream(new ByteArrayInputStream(data));
-            ByteArrayOutputStream out = new ByteArrayOutputStream();
-            byte[] buf = new byte[4096];
-            int readBytes;
-            while ((readBytes = in.read(buf)) > 0) {
-                out.write(buf, 0, readBytes);
-            }
-            return out.toByteArray();
-        } catch (IOException e) {
-            log.log(POILogger.INFO, "Possibly corrupt compression or non-compressed data", e);
-            return data;
-        }
-    }
-
-    @Override
-    public int getRecordSize() {
-        return 8 + 50 + raw_pictureData.length;
-    }
-
-    /**
-     * Gets the first MD4, that specifies the unique identifier of the
-     * uncompressed blip data
-     *
-     * @return the first MD4
-     */
-    public byte[] getUID() {
-        return field_1_UID;
-    }
-
-    /**
-     * Sets the first MD4, that specifies the unique identifier of the
-     * uncompressed blip data
-     *
-     * @param uid the first MD4
-     */
-    public void setUID(byte[] uid) {
-        if (uid == null || uid.length != 16) {
-            throw new IllegalArgumentException("uid must be byte[16]");
-        }
-        System.arraycopy(uid, 0, field_1_UID, 0, field_1_UID.length);
-    }
-
-    /**
-     * Gets the uncompressed size (in bytes)
-     *
-     * @return the uncompressed size
-     */
-    public int getUncompressedSize() {
-        return field_2_cb;
-    }
-
-    /**
-     * Sets the uncompressed size (in bytes)
-     *
-     * @param uncompressedSize the uncompressed size
-     */
-    public void setUncompressedSize(int uncompressedSize) {
-        field_2_cb = uncompressedSize;
-    }
-
-    /**
-     * Get the clipping region of the pict file
-     *
-     * @return the clipping region
-     */
-    public Rectangle getBounds() {
-        return new Rectangle(field_3_rcBounds_x1,
-                             field_3_rcBounds_y1,
-                             field_3_rcBounds_x2 - field_3_rcBounds_x1,
-                             field_3_rcBounds_y2 - field_3_rcBounds_y1);
-    }
-
-    /**
-     * Sets the clipping region
-     *
-     * @param bounds the clipping region
-     */
-    public void setBounds(Rectangle bounds) {
-        field_3_rcBounds_x1 = bounds.x;
-        field_3_rcBounds_y1 = bounds.y;
-        field_3_rcBounds_x2 = bounds.x + bounds.width;
-        field_3_rcBounds_y2 = bounds.y + bounds.height;
-    }
-
-    /**
-     * Gets the dimensions of the metafile
-     *
-     * @return the dimensions of the metafile
-     */
-    public Dimension getSizeEMU() {
-        return new Dimension(field_4_ptSize_w, field_4_ptSize_h);
-    }
-
-    /**
-     * Gets the dimensions of the metafile
-     *
-     * @param sizeEMU the dimensions of the metafile
-     */
-    public void setSizeEMU(Dimension sizeEMU) {
-        field_4_ptSize_w = sizeEMU.width;
-        field_4_ptSize_h = sizeEMU.height;
-    }
-
-    /**
-     * Gets the compressed size of the metafile (in bytes)
-     *
-     * @return the compressed size
-     */
-    public int getCompressedSize() {
-        return field_5_cbSave;
-    }
-
-    /**
-     * Sets the compressed size of the metafile (in bytes)
-     *
-     * @param compressedSize the compressed size
-     */
-    public void setCompressedSize(int compressedSize) {
-        field_5_cbSave = compressedSize;
-    }
-
-    /**
-     * Gets the compression of the metafile
-     *
-     * @return true, if the metafile is compressed
-     */
-    public boolean isCompressed() {
-        return (field_6_fCompression == 0);
-    }
-
-    /**
-     * Sets the compression of the metafile
-     *
-     * @param compressed the compression state, true if it's compressed
-     */
-    public void setCompressed(boolean compressed) {
-        field_6_fCompression = compressed ? 0 : (byte)0xFE;
-    }
-
-    /**
-     * Gets the filter byte - this is usually 0xFE
-     *
-     * @return the filter byte
-     */
-    public byte getFilter() {
-        return field_7_fFilter;
-    }
-
-    /**
-     * Sets the filter byte - this is usually 0xFE
-     *
-     * @param filter the filter byte
-     */
-    public void setFilter(byte filter) {
-        field_7_fFilter = filter;
-    }
-
-    @Override
-    public Map<String, Supplier<?>> getGenericProperties() {
-        final Map<String, Supplier<?>> m = new LinkedHashMap<>(super.getGenericProperties());
-        m.put("uid", this::getUID);
-        m.put("uncompressedSize", this::getUncompressedSize);
-        m.put("bounds", this::getBounds);
-        m.put("sizeInEMU", this::getSizeEMU);
-        m.put("compressedSize", this::getCompressedSize);
-        m.put("isCompressed", this::isCompressed);
-        m.put("filter", this::getFilter);
-        return Collections.unmodifiableMap(m);
-    }
-
-    @Override
-    public EscherPictBlip copy() {
-        return new EscherPictBlip(this);
-    }
-}

src/java/org/apache/poi/ddf/EscherRecordTypes.java

@@ -19,62 +19,65 @@ package org.apache.poi.ddf;
 
 import java.util.Map;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 public enum EscherRecordTypes {
-    // records greater then 0xF000 belong to with Microsoft Office Drawing format also known as Escher
+    // records greater then 0xF000 belong to Microsoft Office Drawing format also known as Escher
-    DGG_CONTAINER(0xF000, "DggContainer", null),
+    DGG_CONTAINER(0xF000, "DggContainer", null, EscherContainerRecord::new),
-    BSTORE_CONTAINER(0xf001, "BStoreContainer", null),
+    BSTORE_CONTAINER(0xf001, "BStoreContainer", null, EscherContainerRecord::new),
-    DG_CONTAINER(0xf002, "DgContainer", null),
+    DG_CONTAINER(0xf002, "DgContainer", null, EscherContainerRecord::new),
-    SPGR_CONTAINER(0xf003, "SpgrContainer", null),
+    SPGR_CONTAINER(0xf003, "SpgrContainer", null, EscherContainerRecord::new),
-    SP_CONTAINER(0xf004, "SpContainer", null),
+    SP_CONTAINER(0xf004, "SpContainer", null, EscherContainerRecord::new),
-    SOLVER_CONTAINER(0xf005, "SolverContainer", null),
+    SOLVER_CONTAINER(0xf005, "SolverContainer", null, EscherContainerRecord::new),
-    DGG(0xf006, "Dgg", "MsofbtDgg"),
+    DGG(0xf006, "Dgg", "MsofbtDgg", EscherDggRecord::new),
-    BSE(0xf007, "BSE", "MsofbtBSE"),
+    BSE(0xf007, "BSE", "MsofbtBSE", EscherBSERecord::new),
-    DG(0xf008, "Dg", "MsofbtDg"),
+    DG(0xf008, "Dg", "MsofbtDg", EscherDgRecord::new),
-    SPGR(0xf009, "Spgr", "MsofbtSpgr"),
+    SPGR(0xf009, "Spgr", "MsofbtSpgr", EscherSpgrRecord::new),
-    SP(0xf00a, "Sp", "MsofbtSp"),
+    SP(0xf00a, "Sp", "MsofbtSp", EscherSpRecord::new),
-    OPT(0xf00b, "Opt", "msofbtOPT"),
+    OPT(0xf00b, "Opt", "msofbtOPT", EscherOptRecord::new),
-    TEXTBOX(0xf00c, null, null),
+    TEXTBOX(0xf00c, null, null, EscherTextboxRecord::new),
-    CLIENT_TEXTBOX(0xf00d, "ClientTextbox", "msofbtClientTextbox"),
+    CLIENT_TEXTBOX(0xf00d, "ClientTextbox", "msofbtClientTextbox", EscherTextboxRecord::new),
-    ANCHOR(0xf00e, null, null),
+    ANCHOR(0xf00e, null, null, null),
-    CHILD_ANCHOR(0xf00f, "ChildAnchor", "MsofbtChildAnchor"),
+    CHILD_ANCHOR(0xf00f, "ChildAnchor", "MsofbtChildAnchor", EscherChildAnchorRecord::new),
-    CLIENT_ANCHOR(0xf010, "ClientAnchor", "MsofbtClientAnchor"),
+    CLIENT_ANCHOR(0xf010, "ClientAnchor", "MsofbtClientAnchor", EscherClientAnchorRecord::new),
-    CLIENT_DATA(0xf011, "ClientData", "MsofbtClientData"),
+    CLIENT_DATA(0xf011, "ClientData", "MsofbtClientData", EscherClientDataRecord::new),
-    CONNECTOR_RULE(0xf012, null, null),
+    CONNECTOR_RULE(0xf012, null, null, null),
-    ALIGN_RULE(0xf013, null, null),
+    ALIGN_RULE(0xf013, null, null, null),
-    ARC_RULE(0xf014, null, null),
+    ARC_RULE(0xf014, null, null, null),
-    CLIENT_RULE(0xf015, null, null),
+    CLIENT_RULE(0xf015, null, null, null),
-    CLSID(0xf016, null, null),
+    CLSID(0xf016, null, null, null),
-    CALLOUT_RULE(0xf017, null, null),
+    CALLOUT_RULE(0xf017, null, null, null),
-    BLIP_START(0xf018, "Blip", "msofbtBlip"),
+    BLIP_START(0xf018, "Blip", "msofbtBlip", null),
-    BLIP_EMF(0xf018 + 2, "BlipEmf", null),
+    BLIP_EMF(0xf018 + 2, "BlipEmf", null, EscherMetafileBlip::new),
-    BLIP_WMF(0xf018 + 3, "BlipWmf", null),
+    BLIP_WMF(0xf018 + 3, "BlipWmf", null, EscherMetafileBlip::new),
-    BLIP_PICT(0xf018 + 4, "BlipPict", null),
+    BLIP_PICT(0xf018 + 4, "BlipPict", null, EscherMetafileBlip::new),
-    BLIP_JPEG(0xf018 + 5, "BlipJpeg", null),
+    BLIP_JPEG(0xf018 + 5, "BlipJpeg", null, EscherBitmapBlip::new),
-    BLIP_PNG(0xf018 + 6, "BlipPng", null),
+    BLIP_PNG(0xf018 + 6, "BlipPng", null, EscherBitmapBlip::new),
-    BLIP_DIB(0xf018 + 7, "BlipDib", null),
+    BLIP_DIB(0xf018 + 7, "BlipDib", null, EscherBitmapBlip::new),
-    BLIP_END(0xf117, "Blip", "msofbtBlip"),
+    BLIP_END(0xf117, "Blip", "msofbtBlip", null),
-    REGROUP_ITEMS(0xf118, null, null),
+    REGROUP_ITEMS(0xf118, null, null, null),
-    SELECTION(0xf119, null, null),
+    SELECTION(0xf119, null, null, null),
-    COLOR_MRU(0xf11a, null, null),
+    COLOR_MRU(0xf11a, null, null, null),
-    DELETED_PSPL(0xf11d, null, null),
+    DELETED_PSPL(0xf11d, null, null, null),
-    SPLIT_MENU_COLORS(0xf11e, "SplitMenuColors", "MsofbtSplitMenuColors"),
+    SPLIT_MENU_COLORS(0xf11e, "SplitMenuColors", "MsofbtSplitMenuColors", EscherSplitMenuColorsRecord::new),
-    OLE_OBJECT(0xf11f, null, null),
+    OLE_OBJECT(0xf11f, null, null, null),
-    COLOR_SCHEME(0xf120, null, null),
+    COLOR_SCHEME(0xf120, null, null, null),
     // same as EscherTertiaryOptRecord.RECORD_ID
-    USER_DEFINED(0xf122, "TertiaryOpt", null),
+    USER_DEFINED(0xf122, "TertiaryOpt", null, EscherTertiaryOptRecord::new),
-    UNKNOWN(0xffff, "unknown", "unknown");
+    UNKNOWN(0xffff, "unknown", "unknown", UnknownEscherRecord::new);
 
     public final short typeID;
     public final String recordName;
     public final String description;
+    public final Supplier<? extends EscherRecord> constructor;
 
-    EscherRecordTypes(int typeID, String recordName, String description) {
+    EscherRecordTypes(int typeID, String recordName, String description, Supplier<? extends EscherRecord> constructor) {
         this.typeID = (short) typeID;
         this.recordName = recordName;
         this.description = description;
+        this.constructor = constructor;
     }
 
     private Short getTypeId() {

src/java/org/apache/poi/hssf/record/ContinueRecord.java

@@ -31,7 +31,7 @@ public final class ContinueRecord extends StandardRecord {
     private byte[] _data;
 
     public ContinueRecord(byte[] data) {
-        _data = data;
+        _data = data.clone();
     }
 
     public ContinueRecord(ContinueRecord other) {

src/java/org/apache/poi/hssf/record/DrawingRecord.java

@@ -43,6 +43,10 @@ public final class DrawingRecord extends StandardRecord {
         recordData = in.readRemainder();
     }
 
+    public DrawingRecord(byte[] data) {
+        recordData = data.clone();
+    }
+
     /**
      * @deprecated POI 3.9
      */

src/java/org/apache/poi/hssf/record/EscherAggregate.java

@@ -17,11 +17,15 @@
 
 package org.apache.poi.hssf.record;
 
+import static org.apache.poi.hssf.record.RecordInputStream.MAX_RECORD_DATA_SIZE;
+
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -31,14 +35,11 @@ import org.apache.poi.ddf.EscherClientDataRecord;
 import org.apache.poi.ddf.EscherContainerRecord;
 import org.apache.poi.ddf.EscherDgRecord;
 import org.apache.poi.ddf.EscherRecord;
-import org.apache.poi.ddf.EscherRecordFactory;
 import org.apache.poi.ddf.EscherSerializationListener;
 import org.apache.poi.ddf.EscherSpRecord;
 import org.apache.poi.ddf.EscherSpgrRecord;
 import org.apache.poi.ddf.EscherTextboxRecord;
 import org.apache.poi.util.IOUtils;
-import org.apache.poi.util.POILogFactory;
-import org.apache.poi.util.POILogger;
 import org.apache.poi.util.RecordFormatException;
 
 /**
@@ -84,8 +85,8 @@ import org.apache.poi.util.RecordFormatException;
  */
 
 public final class EscherAggregate extends AbstractEscherHolderRecord {
-    public static final short sid = 9876; // not a real sid - dummy value
+    // not a real sid - dummy value
-    private static final POILogger log = POILogFactory.getLogger(EscherAggregate.class);
+    public static final short sid = 9876;
     //arbitrarily selected; may need to increase
     private static final int MAX_RECORD_LENGTH = 100_000_000;
 
@@ -364,17 +365,6 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         return builder.toString();
     }
 
-    /**
-     * @param sid - record sid we want to check if it belongs to drawing layer
-     * @return true if record is instance of DrawingRecord or ContinueRecord or ObjRecord or TextObjRecord
-     */
-    private static boolean isDrawingLayerRecord(final short sid) {
-        return sid == DrawingRecord.sid ||
-                sid == ContinueRecord.sid ||
-                sid == ObjRecord.sid ||
-                sid == TextObjectRecord.sid;
-    }
-
     /**
      * Collapses the drawing records into an aggregate.
      * read Drawing, Obj, TxtObj, Note and Continue records into single byte array,
@@ -384,84 +374,85 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
      * @param locFirstDrawingRecord - location of the first DrawingRecord inside sheet
      * @return new EscherAggregate create from all aggregated records which belong to drawing layer
      */
-    public static EscherAggregate createAggregate(List<RecordBase> records, int locFirstDrawingRecord) {
+    public static EscherAggregate createAggregate(final List<RecordBase> records, final int locFirstDrawingRecord) {
-        // Keep track of any shape records created so we can match them back to the object id's.
-        // Textbox objects are also treated as shape objects.
-        final List<EscherRecord> shapeRecords = new ArrayList<>();
-        EscherRecordFactory recordFactory = new DefaultEscherRecordFactory() {
-            public EscherRecord createRecord(byte[] data, int offset) {
-                EscherRecord r = super.createRecord(data, offset);
-                if (r.getRecordId() == EscherClientDataRecord.RECORD_ID || r.getRecordId() == EscherTextboxRecord.RECORD_ID) {
-                    shapeRecords.add(r);
-                }
-                return r;
-            }
-        };
-
-        // Create one big buffer
-        ByteArrayOutputStream buffer = new ByteArrayOutputStream();
         EscherAggregate agg = new EscherAggregate(false);
-        int loc = locFirstDrawingRecord;
+
-        while (loc + 1 < records.size()
+        ShapeCollector recordFactory = new ShapeCollector();
-                && (isDrawingLayerRecord(sid(records, loc)))) {
+        List<Record> objectRecords = new ArrayList<>();
-            try {
+
-                if (!(sid(records, loc) == DrawingRecord.sid || sid(records, loc) == ContinueRecord.sid)) {
+        int nextIdx = locFirstDrawingRecord;
-                    loc++;
+        for (RecordBase rb : records.subList(locFirstDrawingRecord, records.size())) {
+            nextIdx++;
+            switch (sid(rb)) {
+                case DrawingRecord.sid:
+                    recordFactory.addBytes(((DrawingRecord)rb).getRecordData());
                     continue;
-                }
+                case ContinueRecord.sid:
-                if (sid(records, loc) == DrawingRecord.sid) {
+                    recordFactory.addBytes(((ContinueRecord)rb).getData());
-                    buffer.write(((DrawingRecord) records.get(loc)).getRecordData());
+                    continue;
-                } else {
+                case ObjRecord.sid:
-                    buffer.write(((ContinueRecord) records.get(loc)).getData());
+                case TextObjectRecord.sid:
-                }
+                    objectRecords.add((org.apache.poi.hssf.record.Record)rb);
-            } catch (IOException e) {
+                    continue;
-                throw new RuntimeException("Couldn't get data from drawing/continue records", e);
+                case NoteRecord.sid:
+                    // any NoteRecords that follow the drawing block must be aggregated and saved in the tailRec collection
+                    NoteRecord r = (NoteRecord)rb;
+                    agg.tailRec.put(r.getShapeId(), r);
+                    continue;
+                default:
+                    nextIdx--;
+                    break;
             }
-            loc++;
+            break;
+        }
+
+        // replace drawing block with the created EscherAggregate
+        records.set(locFirstDrawingRecord, agg);
+        if (locFirstDrawingRecord+1 <= nextIdx) {
+            records.subList(locFirstDrawingRecord + 1, nextIdx).clear();
         }
 
         // Decode the shapes
-        // agg.escherRecords = new ArrayList();
+        Iterator<EscherRecord> shapeIter = recordFactory.parse(agg).iterator();
-        int pos = 0;
-        while (pos < buffer.size()) {
-            EscherRecord r = recordFactory.createRecord(buffer.toByteArray(), pos);
-            int bytesRead = r.fillFields(buffer.toByteArray(), pos, recordFactory);
-            agg.addEscherRecord(r);
-            pos += bytesRead;
-        }
 
         // Associate the object records with the shapes
-        loc = locFirstDrawingRecord + 1;
+        objectRecords.forEach(or -> agg.shapeToObj.put(shapeIter.next(), or));
-        int shapeIndex = 0;
-        while (loc < records.size()
-                && (isDrawingLayerRecord(sid(records, loc)))) {
-            if (!isObjectRecord(records, loc)) {
-                loc++;
-                continue;
-            }
-            Record objRecord = (org.apache.poi.hssf.record.Record) records.get(loc);
-            agg.shapeToObj.put(shapeRecords.get(shapeIndex++), objRecord);
-            loc++;
-        }
 
-        // any NoteRecords that follow the drawing block must be aggregated and and saved in the tailRec collection
-        while (loc < records.size()) {
-            if (sid(records, loc) == NoteRecord.sid) {
-                NoteRecord r = (NoteRecord) records.get(loc);
-                agg.tailRec.put(r.getShapeId(), r);
-            } else {
-                break;
-            }
-            loc++;
-        }
-
-        int locLastDrawingRecord = loc;
-        // replace drawing block with the created EscherAggregate
-        records.subList(locFirstDrawingRecord, locLastDrawingRecord).clear();
-        records.add(locFirstDrawingRecord, agg);
         return agg;
     }
 
+    private static class ShapeCollector extends DefaultEscherRecordFactory {
+        final List<EscherRecord> objShapes = new ArrayList<>();
+        final ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+
+        void addBytes(byte[] data) {
+            try {
+                buffer.write(data);
+            } catch (IOException e) {
+                throw new RuntimeException("Couldn't get data from drawing/continue records", e);
+            }
+        }
+
+        public EscherRecord createRecord(byte[] data, int offset) {
+            EscherRecord r = super.createRecord(data, offset);
+            short rid = r.getRecordId();
+            if (rid == EscherClientDataRecord.RECORD_ID || rid == EscherTextboxRecord.RECORD_ID) {
+                objShapes.add(r);
+            }
+            return r;
+        }
+
+        List<EscherRecord> parse(EscherAggregate agg) {
+            byte[] buf = buffer.toByteArray();
+            for (int pos = 0, bytesRead; pos < buf.length; pos += bytesRead) {
+                EscherRecord r = createRecord(buf, pos);
+                bytesRead = r.fillFields(buf, pos, this);
+                agg.addEscherRecord(r);
+            }
+            return objShapes;
+        }
+    }
+
     /**
      * Serializes this aggregate to a byte array.  Since this is an aggregate
      * record it will effectively serialize the aggregated records.
@@ -470,7 +461,7 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
      * @param data   The byte array to serialize to.
      * @return The number of bytes serialized.
      */
-    public int serialize(int offset, byte[] data) {
+    public int serialize(final int offset, final byte[] data) {
         // Determine buffer size
         List <EscherRecord>records = getEscherRecords();
         int size = getEscherRecordSize(records);
@@ -501,18 +492,14 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
         // the first one because it's the patriach).
         pos = offset;
         int writtenEscherBytes = 0;
-        int i;
+        boolean isFirst = true;
-        for (i = 1; i < shapes.size(); i++) {
+        int endOffset = 0;
-            int endOffset = spEndingOffsets.get(i) - 1;
+        for (int i = 1; i < shapes.size(); i++) {
-            int startOffset;
+            int startOffset = endOffset;
-            if (i == 1)
+            endOffset = spEndingOffsets.get(i);
-                startOffset = 0;
-            else
-                startOffset = spEndingOffsets.get(i - 1);
 
-            byte[] drawingData = new byte[endOffset - startOffset + 1];
+            byte[] drawingData = Arrays.copyOfRange(buffer, startOffset, endOffset);
-            System.arraycopy(buffer, startOffset, drawingData, 0, drawingData.length);
+            pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, isFirst);
-            pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i);
 
             writtenEscherBytes += drawingData.length;
 
@@ -520,24 +507,22 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
             Record obj = shapeToObj.get(shapes.get(i));
             pos += obj.serialize(pos, data);
 
-            if (i == shapes.size() - 1 && endOffset < buffer.length - 1) {
+            isFirst = false;
-                drawingData = new byte[buffer.length - endOffset - 1];
-                System.arraycopy(buffer, endOffset + 1, drawingData, 0, drawingData.length);
-                pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i);
-            }
         }
-        if ((pos - offset) < buffer.length - 1) {
+
-            byte[] drawingData = new byte[buffer.length - (pos - offset)];
+        if (endOffset < buffer.length - 1) {
-            System.arraycopy(buffer, (pos - offset), drawingData, 0, drawingData.length);
+            byte[] drawingData = Arrays.copyOfRange(buffer, endOffset, buffer.length);
-            pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i);
+            pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, isFirst);
         }
 
         for (NoteRecord noteRecord : tailRec.values()) {
             pos += noteRecord.serialize(pos, data);
         }
+
         int bytesWritten = pos - offset;
-        if (bytesWritten != getRecordSize())
+        if (bytesWritten != getRecordSize()) {
             throw new RecordFormatException(bytesWritten + " bytes written but getRecordSize() reports " + getRecordSize());
+        }
         return bytesWritten;
     }
 
@@ -547,34 +532,19 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
      *                           drawing or continue record)
      * @param pos current position of data array
      * @param data - array of bytes where drawing records must be serialized
-     * @param i - number of shape, saved into data array
+     * @param isFirst - is it the first shape, saved into data array
      * @return offset of data array after serialization
      */
-    private int writeDataIntoDrawingRecord(byte[] drawingData, int writtenEscherBytes, int pos, byte[] data, int i) {
+    private int writeDataIntoDrawingRecord(final byte[] drawingData, final int writtenEscherBytes, final int pos, final byte[] data, final boolean isFirst) {
         int temp = 0;
         //First record in drawing layer MUST be DrawingRecord
-        if (writtenEscherBytes + drawingData.length > RecordInputStream.MAX_RECORD_DATA_SIZE && i != 1) {
+        boolean useDrawingRecord = isFirst || (writtenEscherBytes + drawingData.length) <= MAX_RECORD_DATA_SIZE;
-            for (int j = 0; j < drawingData.length; j += RecordInputStream.MAX_RECORD_DATA_SIZE) {
+
-                byte[] buf = new byte[Math.min(RecordInputStream.MAX_RECORD_DATA_SIZE, drawingData.length - j)];
+        for (int j = 0; j < drawingData.length; j += MAX_RECORD_DATA_SIZE) {
-                System.arraycopy(drawingData, j, buf, 0, Math.min(RecordInputStream.MAX_RECORD_DATA_SIZE, drawingData.length - j));
+            byte[] buf = Arrays.copyOfRange(drawingData, j, Math.min(j+MAX_RECORD_DATA_SIZE, drawingData.length));
-                ContinueRecord drawing = new ContinueRecord(buf);
+            Record drawing = (useDrawingRecord) ? new DrawingRecord(buf) : new ContinueRecord(buf);
-                temp += drawing.serialize(pos + temp, data);
+            temp += drawing.serialize(pos + temp, data);
-            }
+            useDrawingRecord = false;
-        } else {
-            for (int j = 0; j < drawingData.length; j += RecordInputStream.MAX_RECORD_DATA_SIZE) {
-                if (j == 0) {
-                    DrawingRecord drawing = new DrawingRecord();
-                    byte[] buf = new byte[Math.min(RecordInputStream.MAX_RECORD_DATA_SIZE, drawingData.length - j)];
-                    System.arraycopy(drawingData, j, buf, 0, Math.min(RecordInputStream.MAX_RECORD_DATA_SIZE, drawingData.length - j));
-                    drawing.setData(buf);
-                    temp += drawing.serialize(pos + temp, data);
-                } else {
-                    byte[] buf = new byte[Math.min(RecordInputStream.MAX_RECORD_DATA_SIZE, drawingData.length - j)];
-                    System.arraycopy(drawingData, j, buf, 0, Math.min(RecordInputStream.MAX_RECORD_DATA_SIZE, drawingData.length - j));
-                    ContinueRecord drawing = new ContinueRecord(buf);
-                    temp += drawing.serialize(pos + temp, data);
-                }
-            }
         }
         return temp;
     }
@@ -624,14 +594,15 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
             if (i == spEndingOffsets.size() - 1 && spEndingOffsets.get(i) < pos) {
                 continueRecordsHeadersSize += 4;
             }
-            if (spEndingOffsets.get(i) - spEndingOffsets.get(i - 1) <= RecordInputStream.MAX_RECORD_DATA_SIZE) {
+            if (spEndingOffsets.get(i) - spEndingOffsets.get(i - 1) <= MAX_RECORD_DATA_SIZE) {
                 continue;
             }
-            continueRecordsHeadersSize += ((spEndingOffsets.get(i) - spEndingOffsets.get(i - 1)) / RecordInputStream.MAX_RECORD_DATA_SIZE) * 4;
+            continueRecordsHeadersSize += ((spEndingOffsets.get(i) - spEndingOffsets.get(i - 1)) / MAX_RECORD_DATA_SIZE) * 4;
         }
 
         int drawingRecordSize = rawEscherSize + (shapeToObj.size()) * 4;
-        if (rawEscherSize != 0 && spEndingOffsets.size() == 1/**EMPTY**/) {
+        if (rawEscherSize != 0 && spEndingOffsets.size() == 1) {
+            // EMPTY
             continueRecordsHeadersSize += 4;
         }
         int objRecordSize = 0;
@@ -671,16 +642,6 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
 
     // =============== Private methods ========================
 
-    /**
-     *
-     * @param records list of the record to look inside
-     * @param loc location of the checked record
-     * @return true if record is instance of ObjRecord or TextObjectRecord
-     */
-    private static boolean isObjectRecord(List <RecordBase>records, int loc) {
-        return sid(records, loc) == ObjRecord.sid || sid(records, loc) == TextObjectRecord.sid;
-    }
-
     /**
      * create base tree with such structure:
      * EscherDgContainer
@@ -741,7 +702,9 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
     public void setDgId(short dgId) {
         EscherContainerRecord dgContainer = getEscherContainer();
         EscherDgRecord dg = dgContainer.getChildById(EscherDgRecord.RECORD_ID);
-        dg.setOptions((short) (dgId << 4));
+        if (dg != null) {
+            dg.setOptions((short) (dgId << 4));
+        }
     }
 
     /**
@@ -757,26 +720,26 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
      */
     public void setMainSpRecordId(int shapeId) {
         EscherContainerRecord dgContainer = getEscherContainer();
-        EscherContainerRecord spgrConatiner = dgContainer.getChildById(EscherContainerRecord.SPGR_CONTAINER);
+        EscherContainerRecord spgrContainer = dgContainer.getChildById(EscherContainerRecord.SPGR_CONTAINER);
-        EscherContainerRecord spContainer = (EscherContainerRecord) spgrConatiner.getChild(0);
+        if (spgrContainer != null) {
-        EscherSpRecord sp = spContainer.getChildById(EscherSpRecord.RECORD_ID);
+            EscherContainerRecord spContainer = (EscherContainerRecord) spgrContainer.getChild(0);
-        sp.setShapeId(shapeId);
+            EscherSpRecord sp = spContainer.getChildById(EscherSpRecord.RECORD_ID);
+            if (sp != null) {
+                sp.setShapeId(shapeId);
+            }
+        }
     }
 
     /**
-     * @param records list of records to look into
+     * @param record the record to look into
-     * @param loc - location of the record which sid must be returned
+     * @return sid of the record
-     * @return sid of the record with selected location
      */
-    private static short sid(List<RecordBase> records, int loc) {
+    private static short sid(RecordBase record) {
-        RecordBase record = records.get(loc);
+        // Aggregates don't have a sid
-        if (record instanceof Record) {
+        // We could step into them, but for these needs we don't care
-            return ((org.apache.poi.hssf.record.Record)record).getSid();
+        return (record instanceof org.apache.poi.hssf.record.Record)
-        } else {
+            ? ((org.apache.poi.hssf.record.Record)record).getSid()
-            // Aggregates don't have a sid
+            : -1;
-            // We could step into them, but for these needs we don't care
-            return -1;
-        }
     }
 
     /**

src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java

@@ -58,11 +58,8 @@ public class HSSFShapeFactory {
             HSSFShapeGroup group = new HSSFShapeGroup(container, obj);
             List<EscherContainerRecord> children = container.getChildContainers();
             // skip the first child record, it is group descriptor
-            for (int i = 0; i < children.size(); i++) {
+            if (children.size() > 1) {
-                EscherContainerRecord spContainer = children.get(i);
+                children.subList(1, children.size()).forEach(c -> createShapeTree(c, agg, group, root));
-                if (i != 0) {
-                    createShapeTree(spContainer, agg, group, root);
-                }
             }
             out.addShape(group);
         } else if (container.getRecordId() == EscherContainerRecord.SP_CONTAINER) {

src/scratchpad/src/org/apache/poi/hslf/record/HSLFEscherRecordFactory.java

@@ -17,11 +17,11 @@
 
 package org.apache.poi.hslf.record;
 
-import java.lang.reflect.Constructor;
+import java.util.function.Supplier;
-import java.util.Map;
 
-import org.apache.poi.ddf.*;
+import org.apache.poi.ddf.DefaultEscherRecordFactory;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.ddf.EscherRecord;
+import org.apache.poi.ddf.EscherRecordFactory;
 
 /**
  * Generates escher records when provided the byte array containing those records.
@@ -29,39 +29,20 @@ import org.apache.poi.util.LittleEndian;
  * @see EscherRecordFactory
  */
 public class HSLFEscherRecordFactory extends DefaultEscherRecordFactory {
-    private static Class<?>[] escherRecordClasses = { EscherPlaceholder.class, HSLFEscherClientDataRecord.class };
-    private static Map<Short, Constructor<? extends EscherRecord>> recordsMap = recordsToMap( escherRecordClasses );
-
-    
     /**
      * Creates an instance of the escher record factory
      */
     public HSLFEscherRecordFactory() {
         // no instance initialisation
     }
-    
-    @Override
-    public EscherRecord createRecord(byte[] data, int offset) {
-        short options = LittleEndian.getShort( data, offset );
-        short recordId = LittleEndian.getShort( data, offset + 2 );
-        // int remainingBytes = LittleEndian.getInt( data, offset + 4 );
 
-        Constructor<? extends EscherRecord> recordConstructor = recordsMap.get(Short.valueOf(recordId));
+    @Override
-        if (recordConstructor == null) {
+    protected Supplier<? extends EscherRecord> getConstructor(short options, short recordId) {
-            return super.createRecord(data, offset);
+        if (recordId == EscherPlaceholder.RECORD_ID) {
+            return EscherPlaceholder::new;
+        } else if (recordId == HSLFEscherClientDataRecord.RECORD_ID) {
+            return HSLFEscherClientDataRecord::new;
         }
-        EscherRecord escherRecord = null;
+        return super.getConstructor(options, recordId);
-        try {
-            escherRecord = recordConstructor.newInstance(new Object[] {});
-        } catch (Exception e) {
-            return super.createRecord(data, offset);
-        }
-        escherRecord.setRecordId(recordId);
-        escherRecord.setOptions(options);
-        if (escherRecord instanceof EscherContainerRecord) {
-            escherRecord.fillFields(data, offset, this);
-        }
-        
-        return escherRecord;
     }
 }

src/testcases/org/apache/poi/hssf/model/TestDrawingAggregate.java

@@ -726,31 +726,22 @@ public class TestDrawingAggregate {
         List<org.apache.poi.hssf.record.Record> dgRecords = RecordFactory.createRecords(new ByteArrayInputStream(dgBytes));
         assertEquals(20, dgRecords.size());
 
-        short[] expectedSids = {
+        int[] expectedSids = {
-                DrawingRecord.sid,
+            DrawingRecord.sid, ObjRecord.sid,
-                ObjRecord.sid,
+            DrawingRecord.sid, TextObjectRecord.sid,
-                DrawingRecord.sid,
+            DrawingRecord.sid, ObjRecord.sid,
-                TextObjectRecord.sid,
+            DrawingRecord.sid, TextObjectRecord.sid,
-                DrawingRecord.sid,
+            DrawingRecord.sid, ObjRecord.sid,
-                ObjRecord.sid,
+            DrawingRecord.sid, TextObjectRecord.sid,
-                DrawingRecord.sid,
+            DrawingRecord.sid, ObjRecord.sid,
-                TextObjectRecord.sid,
+            DrawingRecord.sid, TextObjectRecord.sid,
-                DrawingRecord.sid,
+            ContinueRecord.sid, ObjRecord.sid,
-                ObjRecord.sid,
+            ContinueRecord.sid, TextObjectRecord.sid
-                DrawingRecord.sid,
-                TextObjectRecord.sid,
-                DrawingRecord.sid,
-                ObjRecord.sid,
-                DrawingRecord.sid,
-                TextObjectRecord.sid,
-                ContinueRecord.sid,
-                ObjRecord.sid,
-                ContinueRecord.sid,
-                TextObjectRecord.sid
         };
-        for (int i = 0; i < expectedSids.length; i++) {
+
-            assertEquals("unexpected record.sid and index[" + i + "]", expectedSids[i], dgRecords.get(i).getSid());
+        int[] actualSids = dgRecords.stream().mapToInt(Record::getSid).toArray();
-        }
+        assertArrayEquals("unexpected record.sid", expectedSids, actualSids);
+
         DrawingManager2 drawingManager = new DrawingManager2(new EscherDggRecord());
 
         // create a dummy sheet consisting of our test data
@@ -904,26 +895,19 @@ public class TestDrawingAggregate {
         List<org.apache.poi.hssf.record.Record> dgRecords = RecordFactory.createRecords(new ByteArrayInputStream(dgBytes));
         assertEquals(14, dgRecords.size());
 
-        short[] expectedSids = {
+        int[] expectedSids = {
-                DrawingRecord.sid,
+            DrawingRecord.sid, ObjRecord.sid,
-                ObjRecord.sid,
+            DrawingRecord.sid, ObjRecord.sid,
-                DrawingRecord.sid,
+            DrawingRecord.sid, ObjRecord.sid,
-                ObjRecord.sid,
+            DrawingRecord.sid, ObjRecord.sid,
-                DrawingRecord.sid,
+            ContinueRecord.sid, ObjRecord.sid,
-                ObjRecord.sid,
+            ContinueRecord.sid, ObjRecord.sid,
-                DrawingRecord.sid,
+            ContinueRecord.sid, ObjRecord.sid
-                ObjRecord.sid,
-                ContinueRecord.sid,
-                ObjRecord.sid,
-                ContinueRecord.sid,
-                ObjRecord.sid,
-                ContinueRecord.sid,
-                ObjRecord.sid
         };
 
-        for (int i = 0; i < expectedSids.length; i++) {
+        int[] actualSids = dgRecords.stream().mapToInt(Record::getSid).toArray();
-            assertEquals("unexpected record.sid and index[" + i + "]", expectedSids[i], dgRecords.get(i).getSid());
+        assertArrayEquals("unexpected record.sid", expectedSids, actualSids);
-        }
+
         DrawingManager2 drawingManager = new DrawingManager2(new EscherDggRecord());
 
         // create a dummy sheet consisting of our test data