Skip to content

Commit 2618bf2

Browse files
KofTwentyTwoKofTwentyTwo
committed
fix(audits): address code review feedback for pluggable audit handlers
- Fix LOG exception parameter order (exception before logPairs) - Use BooleanUtils.isTrue() for nullable Boolean checks - Replace private ThrowingRunnable with UnsafeVoidVoidMethod - Add QInstanceValidator checks for audit handler metadata - Add Javadoc to fluent setters in ProcessedAuditHandlerInput - Remove unused insertOutput variable and redundant tearDown
1 parent 715b3a5 commit 2618bf2

File tree

5 files changed

+60
-43
lines changed

5 files changed

+60
-43
lines changed

‎qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/actions/audits/AuditHandlerExecutor.java‎

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.kingsrook.qqq.backend.core.model.metadata.audits.AuditHandlerType;
3939
import com.kingsrook.qqq.backend.core.model.metadata.audits.QAuditHandlerMetaData;
4040
import com.kingsrook.qqq.backend.core.utils.PrefixedDefaultThreadFactory;
41+
import com.kingsrook.qqq.backend.core.utils.lambdas.UnsafeVoidVoidMethod;
4142
import static com.kingsrook.qqq.backend.core.logging.LogUtils.logPair;
4243

4344

@@ -146,7 +147,7 @@ private void executeProcessedHandler(QAuditHandlerMetaData handlerMetaData, Proc
146147
/*******************************************************************************
147148
** Execute a handler synchronously, respecting failure policy.
148149
*******************************************************************************/
149-
private void executeSync(QAuditHandlerMetaData handlerMetaData, ThrowingRunnable runnable) throws QException
150+
private void executeSync(QAuditHandlerMetaData handlerMetaData, UnsafeVoidVoidMethod<Exception> runnable) throws QException
150151
{
151152
try
152153
{
@@ -165,7 +166,7 @@ private void executeSync(QAuditHandlerMetaData handlerMetaData, ThrowingRunnable
165166
** Note: FAIL_OPERATION policy cannot work with async handlers since the
166167
** original operation has already completed.
167168
*******************************************************************************/
168-
private void executeAsync(QAuditHandlerMetaData handlerMetaData, ThrowingRunnable runnable)
169+
private void executeAsync(QAuditHandlerMetaData handlerMetaData, UnsafeVoidVoidMethod<Exception> runnable)
169170
{
170171
CapturedContext capturedContext = QContext.capture();
171172

@@ -178,9 +179,8 @@ private void executeAsync(QAuditHandlerMetaData handlerMetaData, ThrowingRunnabl
178179
}
179180
catch(Exception e)
180181
{
181-
LOG.warn("Async audit handler failed",
182-
logPair("handlerName", handlerMetaData.getName()),
183-
e);
182+
LOG.warn("Async audit handler failed", e,
183+
logPair("handlerName", handlerMetaData.getName()));
184184
}
185185
finally
186186
{
@@ -206,39 +206,22 @@ private void handleFailure(QAuditHandlerMetaData handlerMetaData, Exception e) t
206206
{
207207
case LOG_AND_CONTINUE ->
208208
{
209-
LOG.warn("Audit handler failed, continuing",
210-
logPair("handlerName", handlerMetaData.getName()),
211-
e);
209+
LOG.warn("Audit handler failed, continuing", e,
210+
logPair("handlerName", handlerMetaData.getName()));
212211
}
213212
case FAIL_OPERATION ->
214213
{
215-
LOG.error("Audit handler failed, failing operation",
216-
logPair("handlerName", handlerMetaData.getName()),
217-
e);
214+
LOG.error("Audit handler failed, failing operation", e,
215+
logPair("handlerName", handlerMetaData.getName()));
218216
throw new QException("Audit handler [" + handlerMetaData.getName() + "] failed", e);
219217
}
220218
default ->
221219
{
222-
LOG.warn("Unknown failure policy, defaulting to LOG_AND_CONTINUE",
220+
LOG.warn("Unknown failure policy, defaulting to LOG_AND_CONTINUE", e,
223221
logPair("handlerName", handlerMetaData.getName()),
224-
logPair("failurePolicy", failurePolicy),
225-
e);
222+
logPair("failurePolicy", failurePolicy));
226223
}
227224
}
228225
}
229226

230-
231-
232-
/*******************************************************************************
233-
** Functional interface for runnables that can throw exceptions.
234-
*******************************************************************************/
235-
@FunctionalInterface
236-
private interface ThrowingRunnable
237-
{
238-
/***************************************************************************
239-
** Run the operation, potentially throwing an exception.
240-
***************************************************************************/
241-
void run() throws Exception;
242-
}
243-
244227
}

‎qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/instances/QInstanceValidator.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
import java.util.function.Supplier;
4242
import java.util.stream.Collectors;
4343
import java.util.stream.Stream;
44+
import com.kingsrook.qqq.backend.core.actions.audits.DMLAuditHandlerInterface;
45+
import com.kingsrook.qqq.backend.core.actions.audits.ProcessedAuditHandlerInterface;
4446
import com.kingsrook.qqq.backend.core.actions.automation.RecordAutomationHandlerInterface;
4547
import com.kingsrook.qqq.backend.core.actions.customizers.TableCustomizerInterface;
4648
import com.kingsrook.qqq.backend.core.actions.customizers.TableCustomizers;
@@ -64,6 +66,8 @@
6466
import com.kingsrook.qqq.backend.core.model.metadata.QSupplementalInstanceMetaData;
6567
import com.kingsrook.qqq.backend.core.model.metadata.authentication.AuthScope;
6668
import com.kingsrook.qqq.backend.core.model.metadata.authentication.QAuthenticationMetaData;
69+
import com.kingsrook.qqq.backend.core.model.metadata.audits.AuditHandlerType;
70+
import com.kingsrook.qqq.backend.core.model.metadata.audits.QAuditHandlerMetaData;
6771
import com.kingsrook.qqq.backend.core.model.metadata.automation.QAutomationProviderMetaData;
6872
import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeReference;
6973
import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeReferenceLambda;
@@ -206,6 +210,7 @@ public void validate(QInstance qInstance) throws QInstanceValidationException
206210
validateAuthentication(qInstance);
207211
validateScopedAuthentication(qInstance);
208212
validateAutomationProviders(qInstance);
213+
validateAuditHandlers(qInstance);
209214
validateTables(qInstance, joinGraph);
210215
validateProcesses(qInstance);
211216
validateReports(qInstance);
@@ -683,6 +688,39 @@ private void validateAutomationProviders(QInstance qInstance)
683688

684689

685690

691+
/*******************************************************************************
692+
** Validate audit handlers.
693+
*******************************************************************************/
694+
private void validateAuditHandlers(QInstance qInstance)
695+
{
696+
if(qInstance.getAuditHandlers() != null)
697+
{
698+
qInstance.getAuditHandlers().forEach((name, handler) ->
699+
{
700+
String prefix = "AuditHandler " + name + ": ";
701+
assertCondition(Objects.equals(name, handler.getName()), prefix + "Inconsistent naming: " + name + "/" + handler.getName());
702+
assertCondition(handler.getHandlerType() != null, prefix + "Missing handlerType");
703+
assertCondition(handler.getHandlerCode() != null, prefix + "Missing handlerCode");
704+
705+
if(handler.getHandlerCode() != null)
706+
{
707+
if(handler.getHandlerType() == AuditHandlerType.DML)
708+
{
709+
validateSimpleCodeReference(prefix + "handlerCode ", handler.getHandlerCode(), DMLAuditHandlerInterface.class);
710+
}
711+
else if(handler.getHandlerType() == AuditHandlerType.PROCESSED)
712+
{
713+
validateSimpleCodeReference(prefix + "handlerCode ", handler.getHandlerCode(), ProcessedAuditHandlerInterface.class);
714+
}
715+
}
716+
717+
runPlugins(QAuditHandlerMetaData.class, handler, qInstance);
718+
});
719+
}
720+
}
721+
722+
723+
686724
/*******************************************************************************
687725
**
688726
*******************************************************************************/

‎qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/actions/audits/ProcessedAuditHandlerInput.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ public void setAuditSingleInputs(List<AuditSingleInput> auditSingleInputs)
6363

6464
/*******************************************************************************
6565
** Fluent setter for auditSingleInputs
66+
**
67+
** @param auditSingleInputs list of individual audit records to be processed
6668
*******************************************************************************/
6769
public ProcessedAuditHandlerInput withAuditSingleInputs(List<AuditSingleInput> auditSingleInputs)
6870
{
@@ -94,6 +96,8 @@ public void setTimestamp(Instant timestamp)
9496

9597
/*******************************************************************************
9698
** Fluent setter for timestamp
99+
**
100+
** @param timestamp when the audit event occurred
97101
*******************************************************************************/
98102
public ProcessedAuditHandlerInput withTimestamp(Instant timestamp)
99103
{
@@ -125,6 +129,8 @@ public void setSession(QSession session)
125129

126130
/*******************************************************************************
127131
** Fluent setter for session
132+
**
133+
** @param session the user session associated with the audit
128134
*******************************************************************************/
129135
public ProcessedAuditHandlerInput withSession(QSession session)
130136
{
@@ -156,6 +162,8 @@ public void setSourceType(String sourceType)
156162

157163
/*******************************************************************************
158164
** Fluent setter for sourceType
165+
**
166+
** @param sourceType identifier for the type of source that triggered the audit
159167
*******************************************************************************/
160168
public ProcessedAuditHandlerInput withSourceType(String sourceType)
161169
{

‎qqq-backend-core/src/main/java/com/kingsrook/qqq/backend/core/model/metadata/QInstance.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
import com.kingsrook.qqq.backend.core.model.metadata.tables.QTableMetaData;
7474
import com.kingsrook.qqq.backend.core.model.session.QSystemUserSession;
7575
import com.kingsrook.qqq.backend.core.scheduler.schedulable.SchedulableType;
76+
import org.apache.commons.lang3.BooleanUtils;
7677
import com.kingsrook.qqq.backend.core.utils.CollectionUtils;
7778
import com.kingsrook.qqq.backend.core.utils.ListingHash;
7879
import com.kingsrook.qqq.backend.core.utils.StringUtils;
@@ -1973,7 +1974,7 @@ public QInstance withAuditHandler(QAuditHandlerMetaData handler)
19731974
public List<QAuditHandlerMetaData> getAuditHandlersForTable(String tableName, AuditHandlerType handlerType)
19741975
{
19751976
return auditHandlers.values().stream()
1976-
.filter(h -> Boolean.TRUE.equals(h.getEnabled()) || h.getEnabled() == null)
1977+
.filter(h -> BooleanUtils.isTrue(h.getEnabled()))
19771978
.filter(h -> handlerType.equals(h.getHandlerType()))
19781979
.filter(h -> CollectionUtils.nullSafeIsEmpty(h.getTableNames())
19791980
|| h.getTableNames().contains(tableName))

‎qqq-backend-core/src/test/java/com/kingsrook/qqq/backend/core/actions/audits/AuditHandlerIntegrationTest.java‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,7 @@
4646
import com.kingsrook.qqq.backend.core.model.metadata.audits.QAuditHandlerMetaData;
4747
import com.kingsrook.qqq.backend.core.model.metadata.audits.QAuditRules;
4848
import com.kingsrook.qqq.backend.core.model.metadata.code.QCodeReference;
49-
import com.kingsrook.qqq.backend.core.modules.backend.implementations.memory.MemoryRecordStore;
5049
import com.kingsrook.qqq.backend.core.utils.TestUtils;
51-
import org.junit.jupiter.api.AfterEach;
5250
import org.junit.jupiter.api.BeforeEach;
5351
import org.junit.jupiter.api.Test;
5452
import static org.assertj.core.api.Assertions.assertThat;
@@ -88,17 +86,6 @@ void setUp() throws QException
8886

8987

9088

91-
/*******************************************************************************
92-
** Clean up memory store after each test
93-
*******************************************************************************/
94-
@AfterEach
95-
void tearDown()
96-
{
97-
MemoryRecordStore.getInstance().reset();
98-
}
99-
100-
101-
10289
/*******************************************************************************
10390
** Test that DML handler is called when performing an INSERT operation.
10491
*******************************************************************************/
@@ -119,7 +106,7 @@ void testDMLHandlerCalledOnInsert() throws QException
119106
new QRecord().withValue("firstName", "John").withValue("lastName", "Doe")
120107
));
121108

122-
InsertOutput insertOutput = new InsertAction().execute(insertInput);
109+
new InsertAction().execute(insertInput);
123110

124111
assertEquals(1, capturedDMLInputs.size());
125112
DMLAuditHandlerInput captured = capturedDMLInputs.get(0);

0 commit comments

Comments
 (0)