Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply audit log functions to portal using audit-log module #5008

Merged
merged 8 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
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.Objects;
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,40 +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) {
String matchedValue = String.valueOf(arg);
api.appendDataInfluence("AnyMatched", entityName, 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
Contributor 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 @@ -90,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 @@ -78,6 +78,9 @@
OpType type = traceContext.tracer().getActiveSpan().getOpType();
ApolloAuditLogDataInfluence.Builder builder = ApolloAuditLogDataInfluence.builder().spanId(spanId)
.entityName(entityName).entityId(entityId).fieldName(fieldName);
if (type == null) {
return;

Check warning on line 82 in apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImpl.java

View check run for this annotation

Codecov / codecov/patch

apollo-audit/apollo-audit-impl/src/main/java/com/ctrip/framework/apollo/audit/component/ApolloAuditLogApiJpaImpl.java#L82

Added line #L82 was not covered by tests
}
switch (type) {
case CREATE:
case UPDATE:
Expand Down Expand Up @@ -106,7 +109,7 @@
f.setAccessible(true);
String val = String.valueOf(f.get(e));
String fieldName = f.getAnnotation(ApolloAuditLogDataInfluenceTableField.class).fieldName();
appendDataInfluence(tableId, tableName, fieldName, val);
appendDataInfluence(tableName, tableId, fieldName, val);
}
} catch (IllegalAccessException ex) {
throw new IllegalArgumentException("failed append data influence, "
Expand Down
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,159 @@
/*
* Copyright 2023 Apollo Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
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
Contributor 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 @@ -185,7 +185,7 @@ public void testAppendDataInfluences() {
api.appendDataInfluences(entities, MockDataInfluenceEntity.class);

Mockito.verify(api, Mockito.times(entityNum))
.appendDataInfluence(Mockito.anyString(), Mockito.eq("MockTableName"), Mockito.eq("MarkedAttribute"),
.appendDataInfluence(Mockito.eq("MockTableName"), Mockito.any(), Mockito.eq("MarkedAttribute"),
Mockito.any());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package com.ctrip.framework.apollo.biz.service;

import com.ctrip.framework.apollo.audit.annotation.ApolloAuditLog;
import com.ctrip.framework.apollo.audit.annotation.OpType;
import com.ctrip.framework.apollo.biz.entity.Audit;
import com.ctrip.framework.apollo.biz.entity.Cluster;
import com.ctrip.framework.apollo.biz.entity.Namespace;
Expand Down Expand Up @@ -102,6 +104,7 @@ public List<AppNamespace> findByAppIdAndNamespaces(String appId, Set<String> nam
}

@Transactional
@ApolloAuditLog(type = OpType.CREATE, name = "AppNamespace.createDefault")
public void createDefaultAppNamespace(String appId, String createBy) {
if (!isAppNamespaceNameUnique(appId, ConfigConsts.NAMESPACE_APPLICATION)) {
throw new ServiceException("appnamespace not unique");
Expand Down
Loading
Loading