IDEMPIERE-4190 Implement Image Storage Provider (1006528) (FHCA-1165)

Peer review and tests:
* PO -> implement postDelete method to be called after the database delete is committed - fix issue about file being deleted from filesystem and commit failing because of referential integrity (for example deleting an image that is associated to a user)
* MArchive, MAttachment, MImage -> change deletion of file from before/afterDelete to postDelete
* MImage -> change getImageStoragePath from clientID/orgID to AD_Image/clientID (this is to allow using the same storage that attachments and archives uses, and avoid issues when changing org)
* MBPartner, MUser, MPOSKey -> delete associated image with postDelete
* ArchiveFileSystem -> fix issue not deleting the file in the filesystem when deleting the archive (using get_ID() on a deleted record returns zero)
* WImageEditor -> save automatically when creating a new image
* AttachmentFileSystem -> avoid trying to save attachment after is deleted
* ImageDBStorageImpl, MSysConfig -> Implement SysConfig IMAGE_DB_STORAGE_SAVE_AS_ZIP to save images in DB zipped - defaults to false which is the actual behavior
* ImageFileStorageImpl -> do not save file with extension, to fix error where previous file was not deleted when uploading a new file with different extension

org.adempiere.base/src/org/compiere/model/ArchiveFileSystem.java

@@ -241,7 +241,7 @@ public class ArchiveFileSystem implements IArchiveStore {
 			throw new IllegalArgumentException("no attachmentPath defined");
 		}
 		StringBuilder msgfile = new StringBuilder().append(archivePathRoot)
-				.append(archive.getArchivePathSnippet()).append(archive.get_ID()).append(".pdf");
+				.append(archive.getArchivePathSnippet()).append(archive.getAD_Archive_ID()).append(".pdf");
 		
 		File file=new File(msgfile.toString());
 		if (file !=null && file.exists()) {

org.adempiere.base/src/org/compiere/model/AttachmentFileSystem.java

@@ -288,7 +288,8 @@ public class AttachmentFileSystem implements IAttachmentStore {
 			}
 		}
 		attach.m_items.remove(index);
-		attach.saveEx(); // must save here as the operation cannot be rolled back on filesystem
+		if (attach.get_ID() > 0) // the attachment has not been deleted
+			attach.saveEx(); // must save here as the operation cannot be rolled back on filesystem
 		if (log.isLoggable(Level.CONFIG)) log.config("Index=" + index + " - NewSize=" + attach.m_items.size());
 		return true;
 	}

org.adempiere.base/src/org/compiere/model/ImageDBStorageImpl.java

@@ -44,11 +44,16 @@ public class ImageDBStorageImpl implements IImageStore {
 
 		byte[] inflatedData = null;
 		try {
-			ByteArrayInputStream in = new ByteArrayInputStream(deflatedData);
-			ZipInputStream zip = new ZipInputStream(in);
-			ZipEntry entry = zip.getNextEntry();
-			if (entry != null) // just one entry
+			ZipInputStream zip = null;
+			ZipEntry entry = null;
+			if (MSysConfig.getBooleanValue(MSysConfig.IMAGE_DB_STORAGE_SAVE_AS_ZIP, false)) {
+				ByteArrayInputStream in = new ByteArrayInputStream(deflatedData);
+				zip = new ZipInputStream(in);
+				entry = zip.getNextEntry();
+			}
+			if (entry != null)
 			{
+				// just one entry per zip
 				ByteArrayOutputStream out = new ByteArrayOutputStream();
 				byte[] buffer = new byte[2048];
 				int length = zip.read(buffer);
@@ -62,7 +67,7 @@ public class ImageDBStorageImpl implements IImageStore {
 						+ "(" + entry.getSize() + ") "
 						+ (entry.getCompressedSize() * 100 / entry.getSize()) + "%");
 			} else {
-				//not zip stream, legacy data
+				//not zip stream, legacy data, or saving as raw
 				inflatedData = deflatedData;
 			}
 		} catch (Exception e) {
@@ -73,36 +78,39 @@ public class ImageDBStorageImpl implements IImageStore {
 	}
 
 	@Override
-	public void save(MImage image, MStorageProvider prov,byte[] inflatedData) {
+	public void save(MImage image, MStorageProvider prov, byte[] inflatedData) {
 		if (inflatedData == null || inflatedData.length == 0) {
 			image.setByteData(null);
 			return;
 		}
-		
-		ByteArrayOutputStream out = new ByteArrayOutputStream();
-		ZipOutputStream zip = new ZipOutputStream(out);
-		zip.setMethod(ZipOutputStream.DEFLATED);
-		zip.setLevel(Deflater.BEST_COMPRESSION);
-		zip.setComment("idempiere");
-		//
 		byte[] deflatedData = null;
-		try {
-			ZipEntry entry = new ZipEntry("IdempiereImage");
-			entry.setTime(System.currentTimeMillis());
-			entry.setMethod(ZipEntry.DEFLATED);
-			zip.putNextEntry(entry);
-			zip.write(inflatedData, 0, inflatedData.length);
-			zip.closeEntry();
-			if (log.isLoggable(Level.FINE)) log.fine(entry.getCompressedSize() + " (" + entry.getSize() + ") "
-					+ (entry.getCompressedSize() * 100 / entry.getSize()) + "%");
+		if (MSysConfig.getBooleanValue(MSysConfig.IMAGE_DB_STORAGE_SAVE_AS_ZIP, false)) {
+			ByteArrayOutputStream out = new ByteArrayOutputStream();
+			ZipOutputStream zip = new ZipOutputStream(out);
+			zip.setMethod(ZipOutputStream.DEFLATED);
+			zip.setLevel(Deflater.BEST_COMPRESSION);
+			zip.setComment("idempiere");
 			//
-			// zip.finish();
-			zip.close();
-			deflatedData = out.toByteArray();
-			if (log.isLoggable(Level.FINE)) log.fine("Length=" + inflatedData.length);
-		} catch (Exception e) {
-			log.log(Level.SEVERE, "saveLOBData", e);
-			deflatedData = null;
+			try {
+				ZipEntry entry = new ZipEntry(image.getName());
+				entry.setTime(System.currentTimeMillis());
+				entry.setMethod(ZipEntry.DEFLATED);
+				zip.putNextEntry(entry);
+				zip.write(inflatedData, 0, inflatedData.length);
+				zip.closeEntry();
+				if (log.isLoggable(Level.FINE)) log.fine(entry.getCompressedSize() + " (" + entry.getSize() + ") "
+						+ (entry.getCompressedSize() * 100 / entry.getSize()) + "%");
+				//
+				// zip.finish();
+				zip.close();
+				deflatedData = out.toByteArray();
+				if (log.isLoggable(Level.FINE)) log.fine("Length=" + inflatedData.length);
+			} catch (Exception e) {
+				log.log(Level.SEVERE, "saveLOBData", e);
+				deflatedData = null;
+			}
+		} else {
+			deflatedData = inflatedData;
 		}
 		image.setByteData(deflatedData);
 	}

org.adempiere.base/src/org/compiere/model/ImageFileStorageImpl.java

@@ -177,10 +177,9 @@ public class ImageFileStorageImpl implements IImageStore {
 				}
 			}
 			
-			String ext = getExtension(image);
 			// write to path
 			msgfile = new StringBuilder().append(imagePathRoot).append(File.separator)
-					.append(image.getImageStoragePath()).append(image.get_ID()).append(ext);
+					.append(image.getImageStoragePath()).append(image.get_ID());
 			final File destFile = new File(msgfile.toString());
 
 			out = new BufferedOutputStream(new FileOutputStream(destFile));
@@ -194,7 +193,7 @@ public class ImageFileStorageImpl implements IImageStore {
 			document.appendChild(root);
 			document.setXmlStandalone(true);
 			final Element entry = document.createElement("entry");
-			StringBuilder msgsat = new StringBuilder(IMAGE_FOLDER_PLACEHOLDER).append(image.getImageStoragePath()).append(image.get_ID()).append(ext);
+			StringBuilder msgsat = new StringBuilder(IMAGE_FOLDER_PLACEHOLDER).append(image.getImageStoragePath()).append(image.get_ID());
 			entry.setAttribute("file", msgsat.toString());
 			root.appendChild(entry);
 			final Source source = new DOMSource(document);
@@ -239,9 +238,8 @@ public class ImageFileStorageImpl implements IImageStore {
 		if ("".equals(imagePathRoot)) {
 			throw new IllegalArgumentException("no image path defined");
 		}
-		String ext = getExtension(image);
 		StringBuilder msgfile = new StringBuilder().append(imagePathRoot)
-				.append(image.getImageStoragePath()).append(image.get_ID()).append(ext);
+				.append(image.getImageStoragePath()).append(image.getAD_Image_ID());
 		
 		File file=new File(msgfile.toString());
 		if (file !=null && file.exists()) {
@@ -253,15 +251,6 @@ public class ImageFileStorageImpl implements IImageStore {
 		return true;
 	}
 
-	private String getExtension(MImage image) {
-		String name = image.getName();
-		String ext = "";
-		if (name.lastIndexOf(".") > 0) {
-			ext = name.substring(name.lastIndexOf("."));
-		}
-		return ext;
-	}
-
 	@Override
 	public boolean isPendingFlush() {
 		return buffer != null && buffer.length > 0;

org.adempiere.base/src/org/compiere/model/MArchive.java

@@ -39,7 +39,7 @@ public class MArchive extends X_AD_Archive {
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = 3217541537768473865L;
+	private static final long serialVersionUID = -9116541441191978777L;
 
 	/**
 	 * Get Archives
@@ -269,7 +269,8 @@ public class MArchive extends X_AD_Archive {
 		return true;
 	} // beforeSave
 	
-	protected boolean beforeDelete ()
+	@Override
+	protected boolean postDelete()
 	{
 		IArchiveStore prov = provider.getArchiveStore();
 		if (prov != null)

org.adempiere.base/src/org/compiere/model/MAttachment.java

@@ -57,7 +57,7 @@ public class MAttachment extends X_AD_Attachment
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = -8261865873158774665L;
+	private static final long serialVersionUID = 6596285414376249694L;
 
 	/**
 	 * 
@@ -502,28 +502,20 @@ public class MAttachment extends X_AD_Attachment
 		return saveLOBData();		//	save in BinaryData
 	}	//	beforeSave
 
-	/**
-	 * 	Executed before Delete operation.
-	 *	@return true if record can be deleted
-	 */
-	protected boolean beforeDelete ()
-	{
-		return deleteLOBData();
-	}
-	
 	/**
 	 * 	Delete Entry Data in Zip File format
 	 *	@return true if saved
 	 */
-	private boolean deleteLOBData()
+	@Override
+	protected boolean postDelete()
 	{
 		if (m_items == null)
 			loadLOBData();
 		IAttachmentStore prov = provider.getAttachmentStore();
 		if (prov != null)
 			return prov.delete(this,provider);
-		return false;
-	} 	//	beforeDelete
+		return true;
+	} 	//	postDelete
 	
 	/**************************************************************************
 	 * 	Test

org.adempiere.base/src/org/compiere/model/MBPartner.java

@@ -47,7 +47,7 @@ public class MBPartner extends X_C_BPartner
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = -255154524310324997L;
+	private static final long serialVersionUID = 5534148976588041343L;
 
 	/**
 	 * 	Get Empty Template Business Partner
@@ -1006,4 +1006,16 @@ public class MBPartner extends X_C_BPartner
 		return success;
 	}	//	afterDelete
 
+	@Override
+	protected boolean postDelete() {
+		if (getLogo_ID() > 0) {
+			MImage img = new MImage(getCtx(), getLogo_ID(), get_TrxName());
+			if (!img.delete(true)) {
+				log.warning("Associated image could not be deleted for bpartner - AD_Image_ID=" + getLogo_ID());
+				return false;
+			}
+		}
+		return true;
+	}
+
 }	//	MBPartner

org.adempiere.base/src/org/compiere/model/MImage.java

@@ -329,8 +329,8 @@ public class MImage extends X_AD_Image
 	}	//	beforeSave
 
 	public String getImageStoragePath() {
-		StringBuilder path = new StringBuilder().append(this.getAD_Client_ID()).append(File.separator).append(this.getAD_Org_ID())
-				.append(File.separator);
+		StringBuilder path = new StringBuilder("AD_Image").append(File.separator)
+				.append(this.getAD_Client_ID()).append(File.separator);
 		return path.toString();
 	}
 	
@@ -354,18 +354,15 @@ public class MImage extends X_AD_Image
 	public void setByteData(byte[] BinaryData){
 		super.setBinaryData(BinaryData);
 	}
-	
+
 	@Override
-	protected boolean afterDelete (boolean success) {
-		if (success) {
-			IImageStore prov = provider.getImageStore();
-			if (prov != null)
-				return prov.delete(this,provider);
-		}
-		return success;
-		
+	protected boolean postDelete() {
+		IImageStore prov = provider.getImageStore();
+		if (prov != null)
+			return prov.delete(this,provider);
+		return true;
 	}
-	
+
 	@Override
 	protected void saveNew_afterSetID()
 	{

org.adempiere.base/src/org/compiere/model/MPOSKey.java

@@ -28,11 +28,10 @@ import java.util.Properties;
  */
 public class MPOSKey extends X_C_POSKey
 {
-
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = -5810613982853803837L;
+	private static final long serialVersionUID = 2595668386249398840L;
 
 	/**
 	 * 	Standard Constructor
@@ -56,4 +55,16 @@ public class MPOSKey extends X_C_POSKey
 		super(ctx, rs, trxName);
 	}	//	MPOSKey
 
+	@Override
+	protected boolean postDelete() {
+		if (getAD_Image_ID() > 0) {
+			MImage img = new MImage(getCtx(), getAD_Image_ID(), get_TrxName());
+			if (!img.delete(true)) {
+				log.warning("Associated image could not be deleted for POS Key - AD_Image_ID=" + getAD_Image_ID());
+				return false;
+			}
+		}
+		return true;
+	}
+
 }	//	MPOSKey

org.adempiere.base/src/org/compiere/model/MSysConfig.java

@@ -39,10 +39,10 @@ import org.compiere.util.DisplayType;
  */
 public class MSysConfig extends X_AD_SysConfig
 {
-    /**
+	/**
 	 * 
 	 */
-	private static final long serialVersionUID = 6662424546913925975L;
+	private static final long serialVersionUID = -9208749663408576569L;
 
 	public static final String ADDRESS_VALIDATION = "ADDRESS_VALIDATION";
     public static final String ALERT_SEND_ATTACHMENT_AS_XLS = "ALERT_SEND_ATTACHMENT_AS_XLS";
@@ -99,6 +99,7 @@ public class MSysConfig extends X_AD_SysConfig
     public static final String HTML_REPORT_THEME = "HTML_REPORT_THEME";
     public static final String IBAN_VALIDATION = "IBAN_VALIDATION";
     public static final String IDENTIFIER_SEPARATOR = "IDENTIFIER_SEPARATOR";
+    public static final String IMAGE_DB_STORAGE_SAVE_AS_ZIP = "IMAGE_DB_STORAGE_SAVE_AS_ZIP";
     public static final String INFO_DEFAULTSELECTED = "INFO_DEFAULTSELECTED";
     public static final String INFO_DOUBLECLICKTOGGLESSELECTION = "INFO_DOUBLECLICKTOGGLESSELECTION";
     public static final String Invoice_ReverseUseNewNumber = "Invoice_ReverseUseNewNumber";

org.adempiere.base/src/org/compiere/model/MUser.java

@@ -33,7 +33,6 @@ import javax.mail.internet.AddressException;
 import javax.mail.internet.InternetAddress;
 
 import org.adempiere.exceptions.DBException;
-import org.codehaus.groovy.classgen.GeneratorContext;
 import org.compiere.util.CCache;
 import org.compiere.util.CLogger;
 import org.compiere.util.DB;
@@ -59,7 +58,7 @@ public class MUser extends X_AD_User
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = 7996468236476384128L;
+	private static final long serialVersionUID = 1366564982801896588L;
 
 	/**
 	 * Get active Users of BPartner
@@ -1105,4 +1104,17 @@ public class MUser extends X_AD_User
 		}
 		return super.afterSave(newRecord, success);
 	}
+
+	@Override
+	protected boolean postDelete() {
+		if (getAD_Image_ID() > 0) {
+			MImage img = new MImage(getCtx(), getAD_Image_ID(), get_TrxName());
+			if (!img.delete(true)) {
+				log.warning("Associated image could not be deleted for user - AD_Image_ID=" + getAD_Image_ID());
+				return false;
+			}
+		}
+		return true;
+	}
+
 }	//	MUser

org.adempiere.base/src/org/compiere/model/PO.java

@@ -110,7 +110,7 @@ public abstract class PO
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = -1743619574547406959L;
+	private static final long serialVersionUID = -1330388218446118451L;
 
 	public static final String LOCAL_TRX_PREFIX = "POSave";
 
@@ -3504,6 +3504,10 @@ public abstract class PO
 			//	Reset
 			if (success)
 			{
+				if (!postDelete()) {
+					log.warning("postDelete failed");
+				}
+
 				//osgi event handler
 				Event event = EventManager.newEvent(IEventTopics.PO_POST_DELETE, this);
 				EventManager.getInstance().postEvent(event);
@@ -3603,6 +3607,14 @@ public abstract class PO
 		return success;
 	} 	//	afterDelete
 
+	/**
+	 * 	Executed after the Delete operation is committed in the database.
+	 *	@return true if post delete is a success
+	 */
+	protected boolean postDelete()
+	{
+		return true;
+	}
 
 	/**
 	 * 	Insert (missing) Translation Records

org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/editor/WImageEditor.java

@@ -178,6 +178,10 @@ public class WImageEditor extends WEditor
 						//
 						ValueChangeEvent vce = new ValueChangeEvent(WImageEditor.this, gridField.getColumnName(), oldValue, newValue);
 						fireValueChange(vce);
+						if (oldValue == null && newValue != null && getGridField() != null && getGridField().getGridTab() != null) {
+							// save automatically when creating a new image
+							getGridField().getGridTab().dataSave(false);
+						}
 					}
 					
 				}