svn commit: r1056973 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java

Author: jleroux
Date: Sun Jan  9 16:36:40 2011
New Revision: 1056973

URL: http://svn.apache.org/viewvc?rev=1056973&view=rev
Log:
A patch from Jeroen van der Wal "When audit log is enabled it creates log transactions even when there are no changes" (https://issues.apache.org/jira/browse/OFBIZ-4076) - OFBIZ-4076

The createEntittyAuditLogSingle function creates a record in the AuditLog entity even when there's no change. This patch only logs new, changed and deleted values. Also, all changes in the same transaction will get the same change date.

After applying this patch the overhead of the AuditEntityLog is recuded from 32% to 25%.

There's even more room for performance increase: a findOne search for the previous entity is done for every field that's enabled.

Modified:
    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java?rev=1056973&r1=1056972&r2=1056973&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericDelegator.java Sun Jan  9 16:36:40 2011
@@ -21,6 +21,7 @@ package org.ofbiz.entity;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.URL;
+import java.sql.Timestamp;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
@@ -2644,29 +2645,21 @@ public class GenericDelegator implements
     }
 
     protected void createEntityAuditLogAll(GenericValue value, boolean isUpdate, boolean isRemove) throws GenericEntityException {
+        Timestamp nowTimestamp = UtilDateTime.nowTimestamp();
         for (ModelField mf: value.getModelEntity().getFieldsUnmodifiable()) {
             if (mf.getEnableAuditLog()) {
-                createEntityAuditLogSingle(value, mf, isUpdate, isRemove);
+                createEntityAuditLogSingle(value, mf, isUpdate, isRemove, nowTimestamp);
             }
         }
     }
 
-    protected void createEntityAuditLogSingle(GenericValue value, ModelField mf, boolean isUpdate, boolean isRemove) throws GenericEntityException {
+    protected void createEntityAuditLogSingle(GenericValue value, ModelField mf, boolean isUpdate, boolean isRemove, Timestamp nowTimestamp) throws GenericEntityException {
         if (value == null || mf == null || !mf.getEnableAuditLog() || this.testRollbackInProgress) {
             return;
         }
 
-        GenericValue entityAuditLog = this.makeValue("EntityAuditLog");
-        entityAuditLog.set("auditHistorySeqId", this.getNextSeqId("EntityAuditLog"));
-        entityAuditLog.set("changedEntityName", value.getEntityName());
-        entityAuditLog.set("changedFieldName", mf.getName());
-
-        String pkCombinedValueText = value.getPkShortValueString();
-        if (pkCombinedValueText.length() > 250) {
-            // uh-oh, the string is too long!
-            pkCombinedValueText = pkCombinedValueText.substring(0, 250);
-        }
-        entityAuditLog.set("pkCombinedValueText", pkCombinedValueText);
+        String newValueText = null;
+        String oldValueText = null;
 
         GenericValue oldGv = null;
         if (isUpdate) {
@@ -2677,11 +2670,10 @@ public class GenericDelegator implements
         }
         if (oldGv == null) {
             if (isUpdate || isRemove) {
-                entityAuditLog.set("oldValueText", "[ERROR] Old value not found even though it was an update or remove");
+                oldValueText = "[ERROR] Old value not found even though it was an update or remove";
             }
         } else {
             // lookup old value
-            String oldValueText = null;
             Object oldValue = oldGv.get(mf.getName());
             if (oldValue != null) {
                 oldValueText = oldValue.toString();
@@ -2689,11 +2681,9 @@ public class GenericDelegator implements
                     oldValueText = oldValueText.substring(0, 250);
                 }
             }
-            entityAuditLog.set("oldValueText", oldValueText);
         }
 
         if (!isRemove) {
-            String newValueText = null;
             Object newValue = value.get(mf.getName());
             if (newValue != null) {
                 newValueText = newValue.toString();
@@ -2701,14 +2691,28 @@ public class GenericDelegator implements
                     newValueText = newValueText.substring(0, 250);
                 }
             }
+        }
+        
+        if (!(newValueText==null ? "" : newValueText).equals((oldValueText==null ? "" : oldValueText))) {
+            // only save changed values
+            GenericValue entityAuditLog = this.makeValue("EntityAuditLog");
+            entityAuditLog.set("auditHistorySeqId", this.getNextSeqId("EntityAuditLog"));
+            entityAuditLog.set("changedEntityName", value.getEntityName());
+            entityAuditLog.set("changedFieldName", mf.getName());
+            
+            String pkCombinedValueText = value.getPkShortValueString();
+            if (pkCombinedValueText.length() > 250) {
+                // uh-oh, the string is too long!
+                pkCombinedValueText = pkCombinedValueText.substring(0, 250);
+            }
+            entityAuditLog.set("pkCombinedValueText", pkCombinedValueText);
             entityAuditLog.set("newValueText", newValueText);
+            entityAuditLog.set("oldValueText", oldValueText);
+            entityAuditLog.set("changedDate", nowTimestamp);
+            entityAuditLog.set("changedByInfo", getCurrentUserIdentifier());
+            entityAuditLog.set("changedSessionInfo", getCurrentSessionIdentifier());
+            this.create(entityAuditLog);      
         }
-
-        entityAuditLog.set("changedDate", UtilDateTime.nowTimestamp());
-        entityAuditLog.set("changedByInfo", getCurrentUserIdentifier());
-        entityAuditLog.set("changedSessionInfo", getCurrentSessionIdentifier());
-
-        this.create(entityAuditLog);
     }
 
     /* (non-Javadoc)