Author: kiwiwings Date: Sun Jan 26 19:50:40 2020 New Revision: 1873187 URL: http://svn.apache.org/viewvc?rev=1873187&view=rev Log: #64036 - Replace reflection calls in factories for Java 9+ - Escher factories
Removed: poi/trunk/src/java/org/apache/poi/ddf/EscherPictBlip.java Modified: poi/trunk/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java poi/trunk/src/java/org/apache/poi/ddf/EscherClientDataRecord.java poi/trunk/src/java/org/apache/poi/ddf/EscherRecordTypes.java poi/trunk/src/java/org/apache/poi/hssf/record/ContinueRecord.java poi/trunk/src/java/org/apache/poi/hssf/record/DrawingRecord.java poi/trunk/src/java/org/apache/poi/hssf/record/EscherAggregate.java poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java poi/trunk/src/scratchpad/src/org/apache/poi/hslf/record/HSLFEscherRecordFactory.java poi/trunk/src/testcases/org/apache/poi/hssf/model/TestDrawingAggregate.java Modified: poi/trunk/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java (original) +++ poi/trunk/src/java/org/apache/poi/ddf/DefaultEscherRecordFactory.java Sun Jan 26 19:50:40 2020 @@ -17,11 +17,12 @@ package org.apache.poi.ddf; -import java.lang.reflect.Constructor; -import java.util.HashMap; -import java.util.Map; +import java.util.function.Supplier; +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, - 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 ); + private static final BitField IS_CONTAINER = BitFieldFactory.getInstance(0xF); /** * Creates an instance of the escher record factory @@ -51,86 +45,41 @@ public class DefaultEscherRecordFactory short recordId = LittleEndian.getShort( data, offset + 2 ); // int remainingBytes = LittleEndian.getInt( data, offset + 4 ); + final EscherRecord escherRecord = getConstructor(options, recordId).get(); + escherRecord.setRecordId(recordId); + escherRecord.setOptions(options); + return escherRecord; + } + + protected Supplier<? extends EscherRecord> getConstructor(short options, short recordId) { + EscherRecordTypes recordTypes = EscherRecordTypes.forTypeID(recordId); + // Options of 0x000F means container record - // 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; + // However, EscherTextboxRecord are containers of records for the host application, + // not of other Escher records, but those are returned by the above anyway + if (recordTypes == EscherRecordTypes.UNKNOWN && IS_CONTAINER.isAllSet(options)) { + return EscherContainerRecord::new; } - 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; + if (recordTypes.constructor != null) { + return recordTypes.constructor; } - Constructor<? extends EscherRecord> recordConstructor = recordsMap.get(Short.valueOf(recordId)); - final EscherRecord escherRecord; - if (recordConstructor == null) { - return new UnknownEscherRecord(); + // handle unknown blip records + if (EscherBlipRecord.RECORD_ID_START <= recordId && recordId <= EscherBlipRecord.RECORD_ID_END) { + return EscherBlipRecord::new; } - try { - escherRecord = recordConstructor.newInstance(); - } catch (Exception e) { - return new UnknownEscherRecord(); - } - escherRecord.setRecordId(recordId); - escherRecord.setOptions(options); - return escherRecord; + + // catch all + return UnknownEscherRecord::new; } + /** - * Converts from a list of classes into a map that contains the record id as the key and - * 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. + * @deprecated this method is not used anymore to identify container records */ - 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) { - @SuppressWarnings("unchecked") - Class<? extends EscherRecord> recCls = (Class<? extends EscherRecord>) recClass; - short sid; - try { - 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; - } - + @Deprecated + @Removal(version = "5.0.0") public static boolean isContainer(short options, short recordId){ if(recordId >= EscherContainerRecord.DGG_CONTAINER && recordId <= EscherContainerRecord.SOLVER_CONTAINER){ Modified: poi/trunk/src/java/org/apache/poi/ddf/EscherClientDataRecord.java URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ddf/EscherClientDataRecord.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/java/org/apache/poi/ddf/EscherClientDataRecord.java (original) +++ poi/trunk/src/java/org/apache/poi/ddf/EscherClientDataRecord.java Sun Jan 26 19:50:40 2020 @@ -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 exte 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 exte 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() ); Modified: poi/trunk/src/java/org/apache/poi/ddf/EscherRecordTypes.java URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ddf/EscherRecordTypes.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/java/org/apache/poi/ddf/EscherRecordTypes.java (original) +++ poi/trunk/src/java/org/apache/poi/ddf/EscherRecordTypes.java Sun Jan 26 19:50:40 2020 @@ -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 - DGG_CONTAINER(0xF000, "DggContainer", null), - BSTORE_CONTAINER(0xf001, "BStoreContainer", null), - DG_CONTAINER(0xf002, "DgContainer", null), - SPGR_CONTAINER(0xf003, "SpgrContainer", null), - SP_CONTAINER(0xf004, "SpContainer", null), - SOLVER_CONTAINER(0xf005, "SolverContainer", null), - DGG(0xf006, "Dgg", "MsofbtDgg"), - BSE(0xf007, "BSE", "MsofbtBSE"), - DG(0xf008, "Dg", "MsofbtDg"), - SPGR(0xf009, "Spgr", "MsofbtSpgr"), - SP(0xf00a, "Sp", "MsofbtSp"), - OPT(0xf00b, "Opt", "msofbtOPT"), - TEXTBOX(0xf00c, null, null), - CLIENT_TEXTBOX(0xf00d, "ClientTextbox", "msofbtClientTextbox"), - ANCHOR(0xf00e, null, null), - CHILD_ANCHOR(0xf00f, "ChildAnchor", "MsofbtChildAnchor"), - CLIENT_ANCHOR(0xf010, "ClientAnchor", "MsofbtClientAnchor"), - CLIENT_DATA(0xf011, "ClientData", "MsofbtClientData"), - CONNECTOR_RULE(0xf012, null, null), - ALIGN_RULE(0xf013, null, null), - ARC_RULE(0xf014, null, null), - CLIENT_RULE(0xf015, null, null), - CLSID(0xf016, null, null), - CALLOUT_RULE(0xf017, null, null), - BLIP_START(0xf018, "Blip", "msofbtBlip"), - BLIP_EMF(0xf018 + 2, "BlipEmf", null), - BLIP_WMF(0xf018 + 3, "BlipWmf", null), - BLIP_PICT(0xf018 + 4, "BlipPict", null), - BLIP_JPEG(0xf018 + 5, "BlipJpeg", null), - BLIP_PNG(0xf018 + 6, "BlipPng", null), - BLIP_DIB(0xf018 + 7, "BlipDib", null), - BLIP_END(0xf117, "Blip", "msofbtBlip"), - REGROUP_ITEMS(0xf118, null, null), - SELECTION(0xf119, null, null), - COLOR_MRU(0xf11a, null, null), - DELETED_PSPL(0xf11d, null, null), - SPLIT_MENU_COLORS(0xf11e, "SplitMenuColors", "MsofbtSplitMenuColors"), - OLE_OBJECT(0xf11f, null, null), - COLOR_SCHEME(0xf120, null, null), + // records greater then 0xF000 belong to Microsoft Office Drawing format also known as Escher + DGG_CONTAINER(0xF000, "DggContainer", null, EscherContainerRecord::new), + BSTORE_CONTAINER(0xf001, "BStoreContainer", null, EscherContainerRecord::new), + DG_CONTAINER(0xf002, "DgContainer", null, EscherContainerRecord::new), + SPGR_CONTAINER(0xf003, "SpgrContainer", null, EscherContainerRecord::new), + SP_CONTAINER(0xf004, "SpContainer", null, EscherContainerRecord::new), + SOLVER_CONTAINER(0xf005, "SolverContainer", null, EscherContainerRecord::new), + DGG(0xf006, "Dgg", "MsofbtDgg", EscherDggRecord::new), + BSE(0xf007, "BSE", "MsofbtBSE", EscherBSERecord::new), + DG(0xf008, "Dg", "MsofbtDg", EscherDgRecord::new), + SPGR(0xf009, "Spgr", "MsofbtSpgr", EscherSpgrRecord::new), + SP(0xf00a, "Sp", "MsofbtSp", EscherSpRecord::new), + OPT(0xf00b, "Opt", "msofbtOPT", EscherOptRecord::new), + TEXTBOX(0xf00c, null, null, EscherTextboxRecord::new), + CLIENT_TEXTBOX(0xf00d, "ClientTextbox", "msofbtClientTextbox", EscherTextboxRecord::new), + ANCHOR(0xf00e, null, null, null), + CHILD_ANCHOR(0xf00f, "ChildAnchor", "MsofbtChildAnchor", EscherChildAnchorRecord::new), + CLIENT_ANCHOR(0xf010, "ClientAnchor", "MsofbtClientAnchor", EscherClientAnchorRecord::new), + CLIENT_DATA(0xf011, "ClientData", "MsofbtClientData", EscherClientDataRecord::new), + CONNECTOR_RULE(0xf012, null, null, null), + ALIGN_RULE(0xf013, null, null, null), + ARC_RULE(0xf014, null, null, null), + CLIENT_RULE(0xf015, null, null, null), + CLSID(0xf016, null, null, null), + CALLOUT_RULE(0xf017, null, null, null), + BLIP_START(0xf018, "Blip", "msofbtBlip", null), + BLIP_EMF(0xf018 + 2, "BlipEmf", null, EscherMetafileBlip::new), + BLIP_WMF(0xf018 + 3, "BlipWmf", null, EscherMetafileBlip::new), + BLIP_PICT(0xf018 + 4, "BlipPict", null, EscherMetafileBlip::new), + BLIP_JPEG(0xf018 + 5, "BlipJpeg", null, EscherBitmapBlip::new), + BLIP_PNG(0xf018 + 6, "BlipPng", null, EscherBitmapBlip::new), + BLIP_DIB(0xf018 + 7, "BlipDib", null, EscherBitmapBlip::new), + BLIP_END(0xf117, "Blip", "msofbtBlip", null), + REGROUP_ITEMS(0xf118, null, null, null), + SELECTION(0xf119, null, null, null), + COLOR_MRU(0xf11a, null, null, null), + DELETED_PSPL(0xf11d, null, null, null), + SPLIT_MENU_COLORS(0xf11e, "SplitMenuColors", "MsofbtSplitMenuColors", EscherSplitMenuColorsRecord::new), + OLE_OBJECT(0xf11f, null, null, null), + COLOR_SCHEME(0xf120, null, null, null), // same as EscherTertiaryOptRecord.RECORD_ID - USER_DEFINED(0xf122, "TertiaryOpt", null), - UNKNOWN(0xffff, "unknown", "unknown"); + USER_DEFINED(0xf122, "TertiaryOpt", null, EscherTertiaryOptRecord::new), + 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() { Modified: poi/trunk/src/java/org/apache/poi/hssf/record/ContinueRecord.java URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/ContinueRecord.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/java/org/apache/poi/hssf/record/ContinueRecord.java (original) +++ poi/trunk/src/java/org/apache/poi/hssf/record/ContinueRecord.java Sun Jan 26 19:50:40 2020 @@ -31,7 +31,7 @@ public final class ContinueRecord extend private byte[] _data; public ContinueRecord(byte[] data) { - _data = data; + _data = data.clone(); } public ContinueRecord(ContinueRecord other) { Modified: poi/trunk/src/java/org/apache/poi/hssf/record/DrawingRecord.java URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/DrawingRecord.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/java/org/apache/poi/hssf/record/DrawingRecord.java (original) +++ poi/trunk/src/java/org/apache/poi/hssf/record/DrawingRecord.java Sun Jan 26 19:50:40 2020 @@ -43,6 +43,10 @@ public final class DrawingRecord extends recordData = in.readRemainder(); } + public DrawingRecord(byte[] data) { + recordData = data.clone(); + } + /** * @deprecated POI 3.9 */ Modified: poi/trunk/src/java/org/apache/poi/hssf/record/EscherAggregate.java URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/EscherAggregate.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/java/org/apache/poi/hssf/record/EscherAggregate.java (original) +++ poi/trunk/src/java/org/apache/poi/hssf/record/EscherAggregate.java Sun Jan 26 19:50:40 2020 @@ -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.EscherClientDa 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.RecordFormatE */ public final class EscherAggregate extends AbstractEscherHolderRecord { - public static final short sid = 9876; // not a real sid - dummy value - private static final POILogger log = POILogFactory.getLogger(EscherAggregate.class); + // not a real sid - dummy value + public static final short sid = 9876; //arbitrarily selected; may need to increase private static final int MAX_RECORD_LENGTH = 100_000_000; @@ -365,17 +366,6 @@ public final class EscherAggregate exten } /** - * @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, * create Escher tree from byte array, create map <EscherRecord, Record> @@ -384,82 +374,83 @@ public final class EscherAggregate exten * @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) { - // 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(); + public static EscherAggregate createAggregate(final List<RecordBase> records, final int locFirstDrawingRecord) { EscherAggregate agg = new EscherAggregate(false); - int loc = locFirstDrawingRecord; - while (loc + 1 < records.size() - && (isDrawingLayerRecord(sid(records, loc)))) { - try { - if (!(sid(records, loc) == DrawingRecord.sid || sid(records, loc) == ContinueRecord.sid)) { - loc++; + + ShapeCollector recordFactory = new ShapeCollector(); + List<Record> objectRecords = new ArrayList<>(); + + int nextIdx = locFirstDrawingRecord; + for (RecordBase rb : records.subList(locFirstDrawingRecord, records.size())) { + nextIdx++; + switch (sid(rb)) { + case DrawingRecord.sid: + recordFactory.addBytes(((DrawingRecord)rb).getRecordData()); continue; - } - if (sid(records, loc) == DrawingRecord.sid) { - buffer.write(((DrawingRecord) records.get(loc)).getRecordData()); - } else { - buffer.write(((ContinueRecord) records.get(loc)).getData()); - } - } catch (IOException e) { - throw new RuntimeException("Couldn't get data from drawing/continue records", e); + case ContinueRecord.sid: + recordFactory.addBytes(((ContinueRecord)rb).getData()); + continue; + case ObjRecord.sid: + case TextObjectRecord.sid: + objectRecords.add((org.apache.poi.hssf.record.Record)rb); + continue; + 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; } - // Decode the shapes - // agg.escherRecords = new ArrayList(); - 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; + // replace drawing block with the created EscherAggregate + records.set(locFirstDrawingRecord, agg); + if (locFirstDrawingRecord+1 <= nextIdx) { + records.subList(locFirstDrawingRecord + 1, nextIdx).clear(); } + // Decode the shapes + Iterator<EscherRecord> shapeIter = recordFactory.parse(agg).iterator(); + // Associate the object records with the shapes - loc = locFirstDrawingRecord + 1; - int shapeIndex = 0; - while (loc < records.size() - && (isDrawingLayerRecord(sid(records, loc)))) { - if (!isObjectRecord(records, loc)) { - loc++; - continue; + objectRecords.forEach(or -> agg.shapeToObj.put(shapeIter.next(), or)); + + 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); } - 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; + 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); } - loc++; + return r; } - int locLastDrawingRecord = loc; - // replace drawing block with the created EscherAggregate - records.subList(locFirstDrawingRecord, locLastDrawingRecord).clear(); - records.add(locFirstDrawingRecord, agg); - return agg; + 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; + } } /** @@ -470,7 +461,7 @@ public final class EscherAggregate exten * @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 exten // the first one because it's the patriach). pos = offset; int writtenEscherBytes = 0; - int i; - for (i = 1; i < shapes.size(); i++) { - int endOffset = spEndingOffsets.get(i) - 1; - int startOffset; - if (i == 1) - startOffset = 0; - else - startOffset = spEndingOffsets.get(i - 1); - - byte[] drawingData = new byte[endOffset - startOffset + 1]; - System.arraycopy(buffer, startOffset, drawingData, 0, drawingData.length); - pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i); + boolean isFirst = true; + int endOffset = 0; + for (int i = 1; i < shapes.size(); i++) { + int startOffset = endOffset; + endOffset = spEndingOffsets.get(i); + + byte[] drawingData = Arrays.copyOfRange(buffer, startOffset, endOffset); + pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, isFirst); writtenEscherBytes += drawingData.length; @@ -520,24 +507,22 @@ public final class EscherAggregate exten Record obj = shapeToObj.get(shapes.get(i)); pos += obj.serialize(pos, data); - if (i == shapes.size() - 1 && endOffset < buffer.length - 1) { - drawingData = new byte[buffer.length - endOffset - 1]; - System.arraycopy(buffer, endOffset + 1, drawingData, 0, drawingData.length); - pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i); - } + isFirst = false; } - if ((pos - offset) < buffer.length - 1) { - byte[] drawingData = new byte[buffer.length - (pos - offset)]; - System.arraycopy(buffer, (pos - offset), drawingData, 0, drawingData.length); - pos += writeDataIntoDrawingRecord(drawingData, writtenEscherBytes, pos, data, i); + + if (endOffset < buffer.length - 1) { + byte[] drawingData = Arrays.copyOfRange(buffer, endOffset, buffer.length); + 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 exten * 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) { - 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)]; - 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); - } - } 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); - } - } + boolean useDrawingRecord = isFirst || (writtenEscherBytes + drawingData.length) <= MAX_RECORD_DATA_SIZE; + + for (int j = 0; j < drawingData.length; j += MAX_RECORD_DATA_SIZE) { + byte[] buf = Arrays.copyOfRange(drawingData, j, Math.min(j+MAX_RECORD_DATA_SIZE, drawingData.length)); + Record drawing = (useDrawingRecord) ? new DrawingRecord(buf) : new ContinueRecord(buf); + temp += drawing.serialize(pos + temp, data); + useDrawingRecord = false; } return temp; } @@ -624,14 +594,15 @@ public final class EscherAggregate exten 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; @@ -672,16 +643,6 @@ public final class EscherAggregate exten // =============== 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 * -EscherSpgrContainer @@ -741,7 +702,9 @@ public final class EscherAggregate exten 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 exten */ public void setMainSpRecordId(int shapeId) { EscherContainerRecord dgContainer = getEscherContainer(); - EscherContainerRecord spgrConatiner = dgContainer.getChildById(EscherContainerRecord.SPGR_CONTAINER); - EscherContainerRecord spContainer = (EscherContainerRecord) spgrConatiner.getChild(0); - EscherSpRecord sp = spContainer.getChildById(EscherSpRecord.RECORD_ID); - sp.setShapeId(shapeId); + EscherContainerRecord spgrContainer = dgContainer.getChildById(EscherContainerRecord.SPGR_CONTAINER); + if (spgrContainer != null) { + EscherContainerRecord spContainer = (EscherContainerRecord) spgrContainer.getChild(0); + EscherSpRecord sp = spContainer.getChildById(EscherSpRecord.RECORD_ID); + if (sp != null) { + sp.setShapeId(shapeId); + } + } } /** - * @param records list of records to look into - * @param loc - location of the record which sid must be returned - * @return sid of the record with selected location - */ - private static short sid(List<RecordBase> records, int loc) { - RecordBase record = records.get(loc); - if (record instanceof Record) { - return ((org.apache.poi.hssf.record.Record)record).getSid(); - } else { - // Aggregates don't have a sid - // We could step into them, but for these needs we don't care - return -1; - } + * @param record the record to look into + * @return sid of the record + */ + private static short sid(RecordBase record) { + // Aggregates don't have a sid + // We could step into them, but for these needs we don't care + return (record instanceof org.apache.poi.hssf.record.Record) + ? ((org.apache.poi.hssf.record.Record)record).getSid() + : -1; } /** Modified: poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java (original) +++ poi/trunk/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java Sun Jan 26 19:50:40 2020 @@ -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++) { - EscherContainerRecord spContainer = children.get(i); - if (i != 0) { - createShapeTree(spContainer, agg, group, root); - } + if (children.size() > 1) { + children.subList(1, children.size()).forEach(c -> createShapeTree(c, agg, group, root)); } out.addShape(group); } else if (container.getRecordId() == EscherContainerRecord.SP_CONTAINER) { Modified: poi/trunk/src/scratchpad/src/org/apache/poi/hslf/record/HSLFEscherRecordFactory.java URL: http://svn.apache.org/viewvc/poi/trunk/src/scratchpad/src/org/apache/poi/hslf/record/HSLFEscherRecordFactory.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/scratchpad/src/org/apache/poi/hslf/record/HSLFEscherRecordFactory.java (original) +++ poi/trunk/src/scratchpad/src/org/apache/poi/hslf/record/HSLFEscherRecordFactory.java Sun Jan 26 19:50:40 2020 @@ -17,11 +17,11 @@ package org.apache.poi.hslf.record; -import java.lang.reflect.Constructor; -import java.util.Map; +import java.util.function.Supplier; -import org.apache.poi.ddf.*; -import org.apache.poi.util.LittleEndian; +import org.apache.poi.ddf.DefaultEscherRecordFactory; +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)); - if (recordConstructor == null) { - return super.createRecord(data, offset); - } - EscherRecord escherRecord = null; - 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); + @Override + protected Supplier<? extends EscherRecord> getConstructor(short options, short recordId) { + if (recordId == EscherPlaceholder.RECORD_ID) { + return EscherPlaceholder::new; + } else if (recordId == HSLFEscherClientDataRecord.RECORD_ID) { + return HSLFEscherClientDataRecord::new; } - - return escherRecord; + return super.getConstructor(options, recordId); } } Modified: poi/trunk/src/testcases/org/apache/poi/hssf/model/TestDrawingAggregate.java URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/model/TestDrawingAggregate.java?rev=1873187&r1=1873186&r2=1873187&view=diff ============================================================================== --- poi/trunk/src/testcases/org/apache/poi/hssf/model/TestDrawingAggregate.java (original) +++ poi/trunk/src/testcases/org/apache/poi/hssf/model/TestDrawingAggregate.java Sun Jan 26 19:50:40 2020 @@ -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 = { - DrawingRecord.sid, - ObjRecord.sid, - DrawingRecord.sid, - TextObjectRecord.sid, - DrawingRecord.sid, - ObjRecord.sid, - DrawingRecord.sid, - TextObjectRecord.sid, - DrawingRecord.sid, - ObjRecord.sid, - DrawingRecord.sid, - TextObjectRecord.sid, - DrawingRecord.sid, - ObjRecord.sid, - DrawingRecord.sid, - TextObjectRecord.sid, - ContinueRecord.sid, - ObjRecord.sid, - ContinueRecord.sid, - TextObjectRecord.sid + int[] expectedSids = { + DrawingRecord.sid, ObjRecord.sid, + DrawingRecord.sid, TextObjectRecord.sid, + DrawingRecord.sid, ObjRecord.sid, + DrawingRecord.sid, TextObjectRecord.sid, + DrawingRecord.sid, ObjRecord.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 = { - DrawingRecord.sid, - ObjRecord.sid, - DrawingRecord.sid, - ObjRecord.sid, - DrawingRecord.sid, - ObjRecord.sid, - DrawingRecord.sid, - ObjRecord.sid, - ContinueRecord.sid, - ObjRecord.sid, - ContinueRecord.sid, - ObjRecord.sid, - ContinueRecord.sid, - ObjRecord.sid + int[] expectedSids = { + DrawingRecord.sid, ObjRecord.sid, + DrawingRecord.sid, ObjRecord.sid, + DrawingRecord.sid, ObjRecord.sid, + DrawingRecord.sid, ObjRecord.sid, + ContinueRecord.sid, ObjRecord.sid, + ContinueRecord.sid, ObjRecord.sid, + ContinueRecord.sid, ObjRecord.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 --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@poi.apache.org For additional commands, e-mail: commits-h...@poi.apache.org