Re: [PR] Adjusting XSLFDrawing's slide shape tree scan in constructor to prevent shape ID warnings. [poi]

pjfanning commented on code in PR #1187:
URL: https://github.com/apache/poi/pull/1187#discussion_r3615753934


##########
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFDrawing.java:
##########
@@ -38,17 +38,24 @@ public class XSLFDrawing {
     /*package*/ XSLFDrawing(XSLFSheet sheet, CTGroupShape spTree){
         _sheet = sheet;
         _spTree = spTree;
-        XmlObject[] cNvPr = sheet.getSpTree().selectPath(
-                "declare namespace p='" + NS_PRESENTATIONML + "' .//*/p:cNvPr");
-        for(XmlObject o : cNvPr) {
-            // powerpoint generates AlternateContent elements which cNvPr elements aren't recognized
-            // ignore them for now
-            if (o instanceof CTNonVisualDrawingProps) {
-                CTNonVisualDrawingProps p = (CTNonVisualDrawingProps)o;
-                try {
-                    sheet.registerShapeId(Math.toIntExact(p.getId()));
-                } catch (ArithmeticException e) {
-                    throw new IllegalStateException("Int overflow with drawing id " + p.getId());
+
+        // Only performs the shape id scan when this is the sheet's own top-level
+        // drawing, since that scan is already recursive and covers every shape in
+        // the sheet. This avoids unnecessary re-scanning and the annoying
+        // "shape id _ has been already used" warning. See https://github.com/apache/poi/issues/924.
+        if (spTree == sheet.getSpTree()) {
+            XmlObject[] cNvPr = _spTree.selectPath(
+                    "declare namespace p='" + NS_PRESENTATIONML + "' .//*/p:cNvPr");
+            for(XmlObject o : cNvPr) {
+                // powerpoint generates AlternateContent elements which cNvPr elements aren't recognized
+                // ignore them for now
+                if (o instanceof CTNonVisualDrawingProps) {

Review Comment:
   we support Java 17 min - so use `if (o instanceof CTNonVisualDrawingProps p)` - avoid the need for additional cast



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]