Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@
import com.ctrip.framework.apollo.audit.annotation.ApolloAuditLogDataInfluenceTable;
import com.ctrip.framework.apollo.audit.annotation.ApolloAuditLogDataInfluenceTableField;
import com.ctrip.framework.apollo.audit.api.ApolloAuditLogApi;
import com.ctrip.framework.apollo.audit.constants.ApolloAuditConstants;
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Pointcut;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletRequestAttributes;

@Aspect
public class ApolloAuditSpanAspect {
Expand All @@ -48,49 +48,45 @@ public void setAuditSpan(ApolloAuditLog auditLog) {
@Around(value = "setAuditSpan(auditLog)")
public Object around(ProceedingJoinPoint pjp, ApolloAuditLog auditLog) throws Throwable {
String opName = auditLog.name();
if (opName.equals("") && RequestContextHolder.getRequestAttributes() != null) {
ServletRequestAttributes servletRequestAttributes = (ServletRequestAttributes) RequestContextHolder.getRequestAttributes();
opName = servletRequestAttributes.getRequest().getRequestURI();
}

try (AutoCloseable scope = api.appendAuditLog(auditLog.type(), opName,
auditLog.description())) {
Object[] args = pjp.getArgs();
Method method = findMethod(pjp.getTarget().getClass(), pjp.getSignature().getName());
for (int i = 0; i < args.length; i++) {
Object arg = args[i];
Annotation[] annotations = method.getParameterAnnotations()[i];
if (Arrays.stream(annotations).anyMatch(anno -> anno instanceof ApolloAuditLogDataInfluence)) {
String entityName = null;
String fieldName = null;
for(int j = 0; j < annotations.length; j++) {
if(annotations[j] instanceof ApolloAuditLogDataInfluenceTable) {
entityName = ((ApolloAuditLogDataInfluenceTable) annotations[j]).tableName();
}
if(annotations[j] instanceof ApolloAuditLogDataInfluenceTableField) {
fieldName = ((ApolloAuditLogDataInfluenceTableField) annotations[j]).fieldName();
}
}
if (entityName != null && fieldName != null) {
// if arg is a collection
if (arg instanceof Collection) {
for (Object o : ((Collection<?>) arg).toArray()) {
String matchedValue = String.valueOf(o);
api.appendDataInfluence(entityName, "AnyMatched", fieldName, matchedValue);
}
}
else {
String matchedValue = String.valueOf(arg);
api.appendDataInfluence(entityName, "AnyMatched", fieldName, matchedValue);
}
}
auditDataInfluenceArg(pjp);
return pjp.proceed();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only record the audit log after method executed success,

so auditDataInfluenceArg should after pjp.proceed()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds better 🫡

}
}

void auditDataInfluenceArg(ProceedingJoinPoint pjp) {
Method method = findMethod(pjp);
Object[] args = pjp.getArgs();
for (int i = 0; i < args.length; i++) {
Object arg = args[i];
Annotation[] annotations = method.getParameterAnnotations()[i];

boolean needAudit = false;
String entityName = null;
String fieldName = null;

for (Annotation annotation : annotations) {
if (annotation instanceof ApolloAuditLogDataInfluence) {
needAudit = true;
}
if (annotation instanceof ApolloAuditLogDataInfluenceTable) {
entityName = ((ApolloAuditLogDataInfluenceTable) annotation).tableName();
}
if (annotation instanceof ApolloAuditLogDataInfluenceTableField) {
fieldName = ((ApolloAuditLogDataInfluenceTableField) annotation).fieldName();
}
}
return pjp.proceed();

if (needAudit) {
parseArgAndAppend(entityName, fieldName, arg);
}
}
}

Method findMethod(Class<?> clazz, String methodName) {
Method findMethod(ProceedingJoinPoint pjp) {
Class<?> clazz = pjp.getTarget().getClass();
String methodName = pjp.getSignature().getName();
for (Method method : clazz.getDeclaredMethods()) {
if (method.getName().equals(methodName)) {
return method;
Expand All @@ -99,4 +95,23 @@ Method findMethod(Class<?> clazz, String methodName) {
return null;
}

void parseArgAndAppend(String entityName, String fieldName, Object arg) {
if (entityName == null || fieldName == null) {
return;
}

Collection<Object> parsedList = new ArrayList<>(Collections.emptyList());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest that iterate arg directly without create a new ArrayList

if (arg instanceof Collection) {
/* if arg is a collection */
parsedList.addAll((Collection<?>) arg);
} else {
parsedList.add(arg);
}

for (Object o : parsedList) {
String matchedValue = String.valueOf(o);
api.appendDataInfluence(entityName, ApolloAuditConstants.ANY_MATCHED_ID, fieldName,
matchedValue);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@ public interface ApolloAuditConstants {
String FOLLOWS_FROM_ID = "Apollo-Audit-FollowsFromId";

String TRACER = "Apollo-Audit-Tracer";

String ANY_MATCHED_ID = "AnyMatched";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package com.ctrip.framework.apollo.audit.aop;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.ctrip.framework.apollo.audit.annotation.ApolloAuditLog;
import com.ctrip.framework.apollo.audit.annotation.ApolloAuditLogDataInfluence;
import com.ctrip.framework.apollo.audit.annotation.ApolloAuditLogDataInfluenceTable;
import com.ctrip.framework.apollo.audit.annotation.ApolloAuditLogDataInfluenceTableField;
import com.ctrip.framework.apollo.audit.annotation.OpType;
import com.ctrip.framework.apollo.audit.api.ApolloAuditLogApi;
import com.ctrip.framework.apollo.audit.constants.ApolloAuditConstants;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.Signature;
import org.junit.jupiter.api.Test;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.boot.test.mock.mockito.SpyBean;
import org.springframework.test.context.ContextConfiguration;

@SpringBootTest
@ContextConfiguration(classes = ApolloAuditSpanAspect.class)
public class ApolloAuditSpanAspectTest {

@SpyBean
ApolloAuditSpanAspect aspect;

@MockBean
ApolloAuditLogApi api;

@Test
public void testAround() throws Throwable {
final OpType opType = OpType.CREATE;
final String opName = "App.create";
final String description = "no description";

ProceedingJoinPoint mockPJP = mock(ProceedingJoinPoint.class);
ApolloAuditLog mockAnnotation = mock(ApolloAuditLog.class);
AutoCloseable mockScope = mock(AutoCloseable.class);
{
when(mockAnnotation.type()).thenReturn(opType);
when(mockAnnotation.name()).thenReturn(opName);
when(mockAnnotation.description()).thenReturn(description);
when(api.appendAuditLog(eq(opType), eq(opName), eq(description)))
.thenReturn(mockScope);
doNothing().when(aspect).auditDataInfluenceArg(mockPJP);
}

aspect.around(mockPJP, mockAnnotation);
verify(api, times(1))
.appendAuditLog(eq(opType), eq(opName), eq(description));
verify(mockScope, times(1))
.close();
verify(aspect, times(1))
.auditDataInfluenceArg(eq(mockPJP));
}

@Test
public void testAuditDataInfluenceArg() throws NoSuchMethodException {
ProceedingJoinPoint mockPJP = mock(ProceedingJoinPoint.class);
Object[] args = new Object[]{new Object(), new Object()};
Method method = MockAuditClass.class.getMethod("mockAuditMethod", Object.class, Object.class);
{
doReturn(method).when(aspect).findMethod(any());
when(mockPJP.getArgs()).thenReturn(args);
}
aspect.auditDataInfluenceArg(mockPJP);
verify(aspect, times(1))
.parseArgAndAppend(eq("App"), eq("Name"), eq(args[0]));
}

@Test
public void testFindMethod() throws NoSuchMethodException {
ProceedingJoinPoint mockPJP = mock(ProceedingJoinPoint.class);
MockAuditClass mockAuditClass = new MockAuditClass();
Signature signature = mock(Signature.class);
Method method = MockAuditClass.class.getMethod("mockAuditMethod", Object.class, Object.class);
{
when(mockPJP.getTarget()).thenReturn(mockAuditClass);
when(mockPJP.getSignature()).thenReturn(signature);
when(signature.getName()).thenReturn("mockAuditMethod");
}
Method methodFounded = aspect.findMethod(mockPJP);

assertEquals(method, methodFounded);
}

@Test
public void testParseArgAndAppendCaseNullName() {
Object somewhat = new Object();
aspect.parseArgAndAppend(null, null, somewhat);
verify(api, times(0))
.appendDataInfluence(any(), any(), any(), any());
}

@Test
public void testParseArgAndAppendCaseCollectionTypeArg() {
final String entityName = "App";
final String fieldName = "Name";
List<Object> list = Arrays.asList(new Object(), new Object(), new Object());

{
doNothing().when(api).appendDataInfluence(any(), any(), any(), any());
}
aspect.parseArgAndAppend(entityName, fieldName, list);
verify(api, times(list.size())).appendDataInfluence(eq(entityName),
eq(ApolloAuditConstants.ANY_MATCHED_ID), eq(fieldName), any());
}

@Test
public void testParseArgAndAppendCaseNormalTypeArg() {
final String entityName = "App";
final String fieldName = "Name";
Object arg = new Object();

{
doNothing().when(api).appendDataInfluence(any(), any(), any(), any());
}
aspect.parseArgAndAppend(entityName, fieldName, arg);
verify(api, times(1)).appendDataInfluence(eq(entityName),
eq(ApolloAuditConstants.ANY_MATCHED_ID), eq(fieldName), any());
}

public class MockAuditClass {

public void mockAuditMethod(
@ApolloAuditLogDataInfluence
@ApolloAuditLogDataInfluenceTable(tableName = "App")
@ApolloAuditLogDataInfluenceTableField(fieldName = "Name") Object val1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test case when arg is a collection?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test case when arg is a collection?

There is a method to parsing arg, and a test method too, see testParseArgAndAppendCaseCollectionTypeArg at ApolloAuditSpanAspectTest

Object val2) {
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
*/
package com.ctrip.framework.apollo.portal.spi.defaultimpl;

import com.ctrip.framework.apollo.audit.annotation.ApolloAuditLog;
import com.ctrip.framework.apollo.audit.annotation.OpType;
import com.ctrip.framework.apollo.common.entity.App;
import com.ctrip.framework.apollo.common.entity.BaseEntity;
import com.ctrip.framework.apollo.core.ConfigConsts;
Expand Down