From 0f31a07f43e9942e1dbe1cd15558aa256d74b931 Mon Sep 17 00:00:00 2001 From: Carlos Ruiz Date: Wed, 10 Nov 2021 02:41:30 +0100 Subject: [PATCH] IDEMPIERE-5034 NPE and other problems when a file is removed from store attachment filesystem (#971) --- .../compiere/model/AttachmentFileSystem.java | 124 ++++++++++++------ .../src/org/compiere/model/MAttachment.java | 7 +- .../adempiere/webui/panel/WAttachment.java | 4 +- 3 files changed, 94 insertions(+), 41 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/model/AttachmentFileSystem.java b/org.adempiere.base/src/org/compiere/model/AttachmentFileSystem.java index 37caf997b8..4603060bc9 100644 --- a/org.adempiere.base/src/org/compiere/model/AttachmentFileSystem.java +++ b/org.adempiere.base/src/org/compiere/model/AttachmentFileSystem.java @@ -33,6 +33,7 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; +import org.adempiere.exceptions.AdempiereException; import org.compiere.util.CLogger; import org.compiere.util.Util; import org.w3c.dom.Document; @@ -52,6 +53,9 @@ public class AttachmentFileSystem implements IAttachmentStore { private final CLogger log = CLogger.getCLogger(getClass()); + /** + * + */ @Override public boolean save(MAttachment attach,MStorageProvider prov) { String attachmentPathRoot = getAttachmentPathRoot(prov); @@ -59,6 +63,13 @@ public class AttachmentFileSystem implements IAttachmentStore { attach.setBinaryData(null); return true; } + + // get list of old entries + byte[] data = (byte[]) attach.get_ValueOld(MAttachment.COLUMNNAME_BinaryData); + NodeList xmlEntries = null; + if (data != null && data.length > 0) + xmlEntries = getEntriesFromXML(data); + final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); try { final DocumentBuilder builder = factory.newDocumentBuilder(); @@ -70,6 +81,31 @@ public class AttachmentFileSystem implements IAttachmentStore { for (int i = 0; i < attach.m_items.size(); i++) { if (log.isLoggable(Level.FINE)) log.fine(attach.m_items.get(i).toString()); File entryFile = attach.m_items.get(i).getFile(); + if (entryFile == null) { + String itemName = attach.m_items.get(i).getName(); + if (itemName.startsWith("~") && itemName.endsWith("~")) { + itemName = itemName.substring(1, itemName.length()-1); + if (xmlEntries != null) { + for (int x = 0; x < xmlEntries.getLength(); x++) { + final Node entryNode = xmlEntries.item(x); + final NamedNodeMap attributes = entryNode.getAttributes(); + final Node fileNode = attributes.getNamedItem("file"); + final Node nameNode = attributes.getNamedItem("name"); + if (itemName.equals(nameNode.getNodeValue())) { + // file was not found but we preserve the old location just in case is temporary + final Element entry = document.createElement("entry"); + entry.setAttribute("name", itemName); + entry.setAttribute("file", fileNode.getNodeValue()); + root.appendChild(entry); + break; + } + } + } + continue; + } + else + throw new AdempiereException("Attachment file not found: " + itemName); + } final String path = entryFile.getAbsolutePath(); // if local file - copy to central attachment folder if (log.isLoggable(Level.FINE)) log.fine(path + " - " + attachmentPathRoot); @@ -171,48 +207,59 @@ public class AttachmentFileSystem implements IAttachmentStore { if (data.length == 0) return true; + NodeList entries = getEntriesFromXML(data); + for (int i = 0; i < entries.getLength(); i++) { + final Node entryNode = entries.item(i); + final NamedNodeMap attributes = entryNode.getAttributes(); + final Node fileNode = attributes.getNamedItem("file"); + final Node nameNode = attributes.getNamedItem("name"); + if(fileNode==null || nameNode==null){ + log.severe("no filename for entry " + i); + attach.m_items = null; + return false; + } + if (log.isLoggable(Level.FINE)) log.fine("name: " + nameNode.getNodeValue()); + String filePath = fileNode.getNodeValue(); + if (log.isLoggable(Level.FINE)) log.fine("filePath: " + filePath); + if(filePath!=null){ + filePath = filePath.replaceFirst(attach.ATTACHMENT_FOLDER_PLACEHOLDER, attachmentPathRoot.replaceAll("\\\\","\\\\\\\\")); + //just to be shure... + String replaceSeparator = File.separator; + if(!replaceSeparator.equals("/")){ + replaceSeparator = "\\\\"; + } + filePath = filePath.replaceAll("/", replaceSeparator); + filePath = filePath.replaceAll("\\\\", replaceSeparator); + } + if (log.isLoggable(Level.FINE)) log.fine("filePath: " + filePath); + final File file = new File(filePath); + if (file.exists()) { + // file data read delayed + IAttachmentLazyDataSource ds = new AttachmentFileLazyDataSource(file); + final MAttachmentEntry entry = new MAttachmentEntry(file.getName(), attach.m_items.size() + 1, ds); + attach.m_items.add(entry); + } else { + log.severe("file not found: " + file.getAbsolutePath()); + attach.m_items.add(new MAttachmentEntry("~" + file.getName() + "~", "".getBytes(), attach.m_items.size() + 1)); + } + } + + return true; + } + + /** + * Get the entries from the XML + * @param data + * @return + */ + private NodeList getEntriesFromXML(byte[] data) { + NodeList entries = null; try { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); final DocumentBuilder builder = factory.newDocumentBuilder(); final Document document = builder.parse(new ByteArrayInputStream(data)); - final NodeList entries = document.getElementsByTagName("entry"); - for (int i = 0; i < entries.getLength(); i++) { - final Node entryNode = entries.item(i); - final NamedNodeMap attributes = entryNode.getAttributes(); - final Node fileNode = attributes.getNamedItem("file"); - final Node nameNode = attributes.getNamedItem("name"); - if(fileNode==null || nameNode==null){ - log.severe("no filename for entry " + i); - attach.m_items = null; - return false; - } - if (log.isLoggable(Level.FINE)) log.fine("name: " + nameNode.getNodeValue()); - String filePath = fileNode.getNodeValue(); - if (log.isLoggable(Level.FINE)) log.fine("filePath: " + filePath); - if(filePath!=null){ - filePath = filePath.replaceFirst(attach.ATTACHMENT_FOLDER_PLACEHOLDER, attachmentPathRoot.replaceAll("\\\\","\\\\\\\\")); - //just to be shure... - String replaceSeparator = File.separator; - if(!replaceSeparator.equals("/")){ - replaceSeparator = "\\\\"; - } - filePath = filePath.replaceAll("/", replaceSeparator); - filePath = filePath.replaceAll("\\\\", replaceSeparator); - } - if (log.isLoggable(Level.FINE)) log.fine("filePath: " + filePath); - final File file = new File(filePath); - if (file.exists()) { - // file data read delayed - IAttachmentLazyDataSource ds = new AttachmentFileLazyDataSource(file); - final MAttachmentEntry entry = new MAttachmentEntry(file.getName(), attach.m_items.size() + 1, ds); - attach.m_items.add(entry); - } else { - log.severe("file not found: " + file.getAbsolutePath()); - attach.m_items.add(new MAttachmentEntry("~" + file.getName() + "~", "".getBytes(), attach.m_items.size() + 1)); - } - } - + entries = document.getElementsByTagName("entry"); } catch (SAXException sxe) { // Error generated during parsing) Exception x = sxe; @@ -231,8 +278,7 @@ public class AttachmentFileSystem implements IAttachmentStore { ioe.printStackTrace(); log.severe(ioe.getMessage()); } - - return true; + return entries; } /** diff --git a/org.adempiere.base/src/org/compiere/model/MAttachment.java b/org.adempiere.base/src/org/compiere/model/MAttachment.java index 877b2c5d4e..d74ab07837 100644 --- a/org.adempiere.base/src/org/compiere/model/MAttachment.java +++ b/org.adempiere.base/src/org/compiere/model/MAttachment.java @@ -322,7 +322,12 @@ public class MAttachment extends X_AD_Attachment if (m_items == null) loadLOBData(); for (int i = 0; i < m_items.size(); i++) { - if (m_items.get(i).getName().equals(item.getName()) ) { + String itemName = m_items.get(i).getName(); + // Filesystem (and store other plugins can) mark not found files surrounding it with ~ + // avoid duplicating the file in this case + if (itemName.startsWith("~") && itemName.endsWith("~")) + itemName = itemName.substring(1, itemName.length()-1); + if (itemName.equals(item.getName()) ) { m_items.set(i, item); replaced = true; } diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/panel/WAttachment.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/panel/WAttachment.java index 190e1b023d..db05bf5d90 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/panel/WAttachment.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/panel/WAttachment.java @@ -116,6 +116,7 @@ public class WAttachment extends Window implements EventListener private Button bDeleteAll = new Button(); private Button bLoad = new Button(); private Button bCancel = ButtonFactory.createNamedButton(ConfirmPanel.A_CANCEL, false, true); + private boolean bCancelClicked = false; private Button bOk = ButtonFactory.createNamedButton(ConfirmPanel.A_OK, false, true); private Button bPreview = new Button(); private Button bEmail = new Button(); @@ -613,7 +614,7 @@ public class WAttachment extends Window implements EventListener } clearPreview(); autoPreview (cbContent.getSelectedIndex(), false); - } else if (e.getTarget() == bOk || DialogEvents.ON_WINDOW_CLOSE.equals(e.getName())) { + } else if (e.getTarget() == bOk || (!bCancelClicked && DialogEvents.ON_WINDOW_CLOSE.equals(e.getName()))) { if (m_attachment != null) { String newText = text.getText(); if (newText == null) @@ -640,6 +641,7 @@ public class WAttachment extends Window implements EventListener dispose(); } } else if (e.getTarget() == bCancel) { + bCancelClicked = true; onCancel(); } else if (e.getTarget() == bDeleteAll) { // Delete Attachment