Michael Kublin has uploaded a new change for review. Change subject: engine: Super audit log improvement ......................................................................
engine: Super audit log improvement So what is current (before patch) behaviour inorder to make some audit log message: 1. Pass class that extends AuditLogableBase 2. Pass a message with keys that should be replaces based on properties from passed class 3. Go over all properties in passed class, collect them and create a HashMap with key as property name and value as property value 4. Replace keys in message using a created HashMap. Now, lets take a look on 3, how we retrieve a properties - we invoke all getXXX methods in our class. How many getXXX methods has simple AuditLogableBase class? How big will be our HashMap? How many queries/calculations will run during invoking some of the getXXX messages, because our class not simple bean? The only answer is a lot, only in simple run I saw a HashMap with 150 values (sic!). I fix this, if I have a message I know all keys that I need, I don't need to go throught all getXXX methods. Intresting fact: optimized solution required less code. Another intresting fact - adding of additional getter to AuditLogableBase, before a fix, was like adding a small brake to the system. Change-Id: Id68a098be36fcfc47c23c57fce3abea50e6273e3 Signed-off-by: Michael Kublin <[email protected]> --- M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirectorTest.java 2 files changed, 23 insertions(+), 23 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/25/13425/1 diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java index 1b2eb34..a23fbdb 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java @@ -3,10 +3,10 @@ import java.util.EnumMap; import java.util.HashMap; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.MissingResourceException; import java.util.ResourceBundle; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -16,7 +16,6 @@ import org.ovirt.engine.core.common.businessentities.AuditLog; import org.ovirt.engine.core.compat.ApplicationException; import org.ovirt.engine.core.compat.Guid; -import org.ovirt.engine.core.compat.backendcompat.PropertyInfo; import org.ovirt.engine.core.compat.backendcompat.TypeCompat; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.log.Log; @@ -751,7 +750,7 @@ } } - static void checkMessages() { + private static void checkMessages() { AuditLogType[] values = AuditLogType.values(); if (values.length != messages.size()) { for (AuditLogType value : values) { @@ -762,7 +761,7 @@ } } - static void checkSeverities() { + private static void checkSeverities() { AuditLogType[] values = AuditLogType.values(); if (values.length != severities.size()) { for (AuditLogType value : values) { @@ -933,7 +932,7 @@ static String resolveMessage(String message, AuditLogableBase logable) { String returnValue = message; if (logable != null) { - Map<String, String> map = getAvailableValues(logable); + Map<String, String> map = getAvailableValues(message, logable); returnValue = resolveMessage(message, map); } return returnValue; @@ -974,22 +973,27 @@ return buffer.toString(); } - static Map<String, String> getAvailableValues(AuditLogableBase logable) { - Map<String, String> returnValue = new HashMap<String, String>(logable.getCustomValues()); - Class<?> type = AuditLogableBase.class; - for (PropertyInfo propertyInfo : TypeCompat.GetProperties(type)) { - Object value = propertyInfo.getValue(logable, null); - String stringValue = value != null ? value.toString() : null; - if (!returnValue.containsKey(propertyInfo.getName().toLowerCase())) { - returnValue.put(propertyInfo.getName().toLowerCase(), stringValue); - } else { - log.errorFormat("Try to add duplicate audit log values with the same name. Type: {0}. Value: {1}", - logable.getAuditLogTypeValue(), propertyInfo.getName().toLowerCase()); - } + private static Set<String> resolvePlaceHolders(String message) { + Set<String> result = new HashSet<String>(); + Matcher matcher = pattern.matcher(message); + + String token; + while (matcher.find()) { + token = matcher.group(); + + // remove leading ${ and trailing } + token = token.substring(2, token.length() - 1); + result.add(token.toLowerCase()); } - List<String> attributes = AuditLogHelper.getCustomLogFields(logable.getClass(), true); + return result; + } + + private static Map<String, String> getAvailableValues(String message, AuditLogableBase logable) { + Map<String, String> returnValue = new HashMap<String, String>(logable.getCustomValues()); + Set<String> attributes = resolvePlaceHolders(message); + attributes = AuditLogHelper.merge(attributes, AuditLogHelper.getCustomLogFields(logable.getClass(), true)); if (attributes != null && attributes.size() > 0) { - TypeCompat.getPropertyValues(logable, new HashSet<String>(attributes), returnValue); + TypeCompat.getPropertyValues(logable, attributes, returnValue); } return returnValue; } diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirectorTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirectorTest.java index 18c8933..05769f8 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirectorTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirectorTest.java @@ -42,10 +42,6 @@ // PowerMockito.when(AuditLogDirector.getDbFacadeInstance()).thenReturn(dbFacade); // } // - @Test - public void testPropertyLoading() { - AuditLogDirector.checkSeverities(); - } // // /** // * The test assures that audit loggable objects with timeout, which were created without an explicit log type, with -- To view, visit http://gerrit.ovirt.org/13425 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id68a098be36fcfc47c23c57fce3abea50e6273e3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Michael Kublin <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
