Merge pull request #138 from ptaylor/1.x_maintenance

Allow whitelist of properties to be used instead of a ignore list
grails-plugins · Apr 12, 2017 · 1b66138 · 1b66138
2 parents 28b257f + 9b9c2b0
commit 1b66138
Show file tree

Hide file tree

Showing 4 changed files with 167 additions and 8 deletions.
diff --git a/audit-test/test/integration/test/AuditDeleteSpec.groovy b/audit-test/test/integration/test/AuditDeleteSpec.groovy
@@ -174,6 +174,42 @@ class AuditDeleteSpec extends IntegrationSpec {
 
     }
 
+    void "Test auditableProperties"() {
+        given:
+        Author.auditable = [auditableProperties: ['famous', 'age', 'dateCreated']]
+        def author = Author.findByName("Aaron")
+
+        when:
+        author.delete(flush: true, failOnError: true)
+
+        then: "only properties in auditableProperties are logged"
+        def events = MyAuditLogEvent.findAllByClassName('test.Author')
+
+        events.size() == 3
+        ['famous', 'age', 'dateCreated'].each { name ->
+            assert events.find {it.propertyName == name}, "${name} was not logged"
+        }
+    }
+
+    void "Test auditableProperties overrides ignore list"() {
+        given:
+        Author.auditable = [
+                auditableProperties: ['famous', 'age', 'dateCreated'],
+                ignore: ['famous', 'age']
+        ]
+        def author = Author.findByName("Aaron")
+
+        when:
+        author.delete(flush: true, failOnError: true)
+
+        then: "only properties in auditableProperties are logged"
+        def events = MyAuditLogEvent.findAllByClassName('test.Author')
+
+        events.size() == 3
+        ['famous', 'age', 'dateCreated'].each { name ->
+            assert events.find {it.propertyName == name}, "${name} was not logged"
+        }
+    }
 
 }
 
diff --git a/audit-test/test/integration/test/AuditInsertSpec.groovy b/audit-test/test/integration/test/AuditInsertSpec.groovy
@@ -289,5 +289,42 @@ class AuditInsertSpec extends IntegrationSpec {
         }
     }
 
+    void "Test auditableProperties"() {
+        given:
+        Author.auditable = [auditableProperties: ['name', 'age', 'dateCreated']]
+        def author = new Author(name: "Aaron", age: 50, famous: true, ssn: '123-981-0001')
+
+        when:
+        author.save(flush: true, failOnError: true)
+
+        then: "only properties in auditableProperties are logged"
+        def events = MyAuditLogEvent.findAllByClassName('test.Author')
+
+        events.size() == 3
+        ['name', 'age', 'dateCreated'].each { name ->
+            assert events.find {it.propertyName == name}, "${name} was not logged"
+        }
+    }
+
+    void "Test auditableProperties overrides ignore list"() {
+        given:
+        Author.auditable = [
+                auditableProperties: ['name', 'age', 'dateCreated'],
+                ignore: ['name', 'age']
+        ]
+        def author = new Author(name: "Aaron", age: 50, famous: true, ssn: '123-981-0001')
+
+        when:
+        author.save(flush: true, failOnError: true)
+
+        then: "only properties in auditableProperties are logged"
+        def events = MyAuditLogEvent.findAllByClassName('test.Author')
+
+        events.size() == 3
+        ['name', 'age', 'dateCreated'].each { name ->
+            assert events.find {it.propertyName == name}, "${name} was not logged"
+        }
+    }
+
 
 }
diff --git a/audit-test/test/integration/test/AuditUpdateSpec.groovy b/audit-test/test/integration/test/AuditUpdateSpec.groovy
@@ -217,6 +217,52 @@ class AuditUpdateSpec extends IntegrationSpec {
 
     }
 
+    void "Test auditableProperties"() {
+        given:
+        Author.auditable = [auditableProperties: ['name', 'famous', 'lastUpdated']]
+        def author = Author.findByName("Aaron")
+
+        when:
+        author.age = 50
+        author.famous = false
+        author.name = 'Bob'
+        author.save(flush: true, failOnError: true)
+
+        then: "only properties in auditableProperties are logged"
+        def events = MyAuditLogEvent.findAllByClassName('test.Author')
+
+        events.size() == 3
+
+        ['name', 'famous', 'lastUpdated'].each { name ->
+            assert events.find {it.propertyName == name}, "${name} was not logged"
+        }
+    }
+
+    void "Test auditableProperties orverrides ignore list"() {
+        given:
+        Author.auditable = [
+                auditableProperties: ['name', 'famous', 'lastUpdated'],
+                ignore: ['name', 'famous']
+        ]
+        def author = Author.findByName("Aaron")
+
+        when:
+        author.age = 50
+        author.famous = false
+        author.name = 'Bob'
+        author.save(flush: true, failOnError: true)
+
+        then: "only properties in auditableProperties are logged"
+        def events = MyAuditLogEvent.findAllByClassName('test.Author')
+
+        events.size() == 3
+
+        ['name', 'famous', 'lastUpdated'].each { name ->
+            assert events.find {it.propertyName == name}, "${name} was not logged"
+        }
+    }
+
+
     void "Test handler is called"() {
         given:
         def author = Author.findByName("Aaron")

diff --git a/...lugin/src/groovy/org/codehaus/groovy/grails/plugins/orm/auditable/AuditLogListener.groovy b/...lugin/src/groovy/org/codehaus/groovy/grails/plugins/orm/auditable/AuditLogListener.groovy
@@ -237,6 +237,32 @@ class AuditLogListener extends AbstractPersistenceEventListener {
     return ignore
   }
 
+
+  /**
+   * Get the list of auditable properties.  This is used to override
+   * the default behaviour of auditing all properties except those in the
+   * ignore list.
+   *
+   *   static auditable = [auditableProperties: ['dateCreated','lastUpdated','myField']]
+   *
+   *
+   */
+  List auditableProperties(domain) {
+
+    Map auditableMap = getAuditableMap(domain)
+    if (auditableMap?.containsKey('auditableProperties')) {
+      log.debug "Found auditableProperty list on this entity ${domain.class.name}"
+      def list = auditableMap['auditableProperties']
+      if (list instanceof List) {
+        return list
+      }
+    }
+
+    null
+  }
+
+
+
   /**
    * The default properties to mask list is:  ['password']
    * if you want to provide your own mask list, specify in the DomainClass:
@@ -460,14 +486,11 @@ class AuditLogListener extends AbstractPersistenceEventListener {
    * to provide a list of fields for onChange to ignore.
    */
   protected boolean significantChange(domain, Map oldMap, Map newMap) {
+    def auditableProperties = auditableProperties(domain)
     def ignore = ignoreList(domain)
-    ignore?.each { String key ->
-      oldMap.remove(key)
-      newMap.remove(key)
-    }
     boolean changed = false
     oldMap.each { String k, Object v ->
-      if (v != newMap[k]) {
+      if (isPropertyAuditable(k, auditableProperties, ignore) && v != newMap[k]) {
         changed = true
       }
     }
@@ -493,6 +516,7 @@ class AuditLogListener extends AbstractPersistenceEventListener {
     }
   }
 
+
   /**
    * Leans heavily on the "toString()" of a property
    * ... this feels crufty... should be tighter...
@@ -508,12 +532,13 @@ class AuditLogListener extends AbstractPersistenceEventListener {
     newMap?.remove('version')
     oldMap?.remove('version')
 
+    List auditableProperties = auditableProperties(domain)
     List ignoreList = ignoreList(domain)
 
     if (newMap && oldMap) {
       log.trace "There are new and old values to log"
       newMap.each { String key, val ->
-        if (key in ignoreList) {
+        if (!isPropertyAuditable(key, auditableProperties, ignoreList)) {
           return
         }
         if (val != oldMap[key]) {
@@ -535,7 +560,7 @@ class AuditLogListener extends AbstractPersistenceEventListener {
     if (newMap && verbose && !AuditLogListenerThreadLocal.auditLogNonVerbose) {
       log.trace "there are new values and logging is verbose ... "
       newMap.each { String key, val ->
-        if (key in ignoreList) {
+        if (!isPropertyAuditable(key, auditableProperties, ignoreList)) {
           return
         }
         def audit = auditClass.newInstance(
@@ -555,7 +580,7 @@ class AuditLogListener extends AbstractPersistenceEventListener {
     if (oldMap && verbose && !AuditLogListenerThreadLocal.auditLogNonVerbose) {
       log.trace "there is only an old map of values available and logging is set to verbose... "
       oldMap.each { String key, val ->
-        if (key in ignoreList) {
+        if (!isPropertyAuditable(key, auditableProperties, ignoreList)) {
           return
         }
         def audit = auditClass.newInstance(
@@ -761,6 +786,21 @@ class AuditLogListener extends AbstractPersistenceEventListener {
     }
   }
 
+  /**
+   * Returns a boolean indicating if the given property should be audited or ignored.
+   * If there is an auditableProperties list the property is auditable if it is in that
+   * list.  Otherwise the property is auditable if is is not in the ignoreList.
+   */
+  boolean isPropertyAuditable(def fieldName, List auditableProperties, List ignoreList) {
+    if (auditableProperties) {
+      return fieldName in auditableProperties
+    } else if (ignoreList) {
+      return !(fieldName in ignoreList)
+    }
+
+    true
+  }
+
   protected ConfigObject getAuditConfig() { AuditLoggingUtils.auditConfig }