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

feat: 쿼리 카운터 적용 #445

Merged
merged 5 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
40 changes: 40 additions & 0 deletions backend/src/main/java/zipgo/aspect/ConnectionProxyHandler.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package zipgo.aspect;

import org.springframework.web.context.request.RequestContextHolder;

import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;

public class ConnectionProxyHandler implements InvocationHandler {

private static final String QUERY_PREPARE_STATEMENT = "prepareStatement";

private final Object connection;
private final QueryCounter queryCounter;

public ConnectionProxyHandler(Object connection, QueryCounter queryCounter) {
this.connection = connection;
this.queryCounter = queryCounter;
}

@Override
public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
countQuery(method);
Copy link
Collaborator

Choose a reason for hiding this comment

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

리뷰하며 공부하기
메서드를 실행하기 전에 쿼리 카운트를 한답.

return method.invoke(connection, args);
Comment on lines +21 to +23
Copy link
Collaborator

Choose a reason for hiding this comment

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

한 가지.. 제가 잘 모르는 상태에서 봐서 아쉬웠던 점이 있는데요..!

countQuery 메서드로 분리되어서, invoke 메서드만 봤을 때는
이 핸들러가 무조건 쿼리를 센다는 것처럼 인식돼서 전체 구조를 보며 이해할 때 좀 헷갈리는 요인이 되었던 것 같습니당.
그런 면에서 메서드 분리를 하지 않아도 괜찮지 않았을까? 라는 생각이 들었어요 ☀️

이상 InvocationHandler 처음 본 인간의 하소연이었습니다.
반영은 무민 맘대로 해주시면 될 것 같아요. 구현해준 무민 최고~!

public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
    if (isPrepareStatement(method) && isRequest()) { // 이럴때만
        queryCounter.increaseCount(); // 쿼리를 세는 녀석입니당 ~
    }
    return method.invoke(connection, args);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

method.invoke가 원래 메서드가 발동하는 곳이고 countQuery는 부가적으로 넣은 것이라 후에 부가적으로 몇개 들어가게된다면 분리해주는 게 좋을 것 같아서 분리시켜놨습니다! 지금은 사실 딱히 분리 안 시켜줘도 상관은 없긴한데 고치면 approve가 다 풀릴거 같아서 이대로 가겠습니당

}

private void countQuery(Method method) {
if (isPrepareStatement(method) && isRequest()) {
queryCounter.increaseCount();
}
}

private boolean isPrepareStatement(Method method) {
return method.getName().equals(QUERY_PREPARE_STATEMENT);
}

private boolean isRequest() {
return RequestContextHolder.getRequestAttributes() != null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

리뷰하며 공부하기

따라서 RequestContextHolder.getRequestAttributes()가 null을 반환하는 경우는 웹 요청이나 웹 관련 작업이 아닌 상황에서 발생합니다. 예를 들어, 백그라운드 작업, 스케줄러, 혹은 일반적인 Java 응용 프로그램 실행 중에는 RequestAttributes가 존재하지 않기 때문에 null을 반환합니다.

오오! 그래서 isRequest()라는 이름이 붙었구나.

}

}
18 changes: 18 additions & 0 deletions backend/src/main/java/zipgo/aspect/QueryCounter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package zipgo.aspect;

import lombok.Getter;
import org.springframework.stereotype.Component;
import org.springframework.web.context.annotation.RequestScope;

@Getter
@Component
@RequestScope
public class QueryCounter {

private int count;

public void increaseCount() {
count++;
}

}
29 changes: 29 additions & 0 deletions backend/src/main/java/zipgo/aspect/QueryCounterAop.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package zipgo.aspect;

import lombok.RequiredArgsConstructor;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.springframework.stereotype.Component;

import java.lang.reflect.Proxy;

@Aspect
@Component
@RequiredArgsConstructor
public class QueryCounterAop {

private final QueryCounter queryCounter;

@Around("execution(* javax.sql.DataSource.getConnection(..))")
public Object getConnection(ProceedingJoinPoint proceedingJoinPoint) throws Throwable {
Object connection = proceedingJoinPoint.proceed();

return Proxy.newProxyInstance(
Copy link
Collaborator

Choose a reason for hiding this comment

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

리뷰하며 공부하기
Connection에 적용된것은 프록시.
AOP는 DataSource.getConnection에만 적용한 것이다!

connection.getClass().getClassLoader(),
connection.getClass().getInterfaces(),
new ConnectionProxyHandler(connection, queryCounter)
);
}

}
4 changes: 4 additions & 0 deletions backend/src/main/java/zipgo/common/config/WebConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import zipgo.auth.presentation.JwtMandatoryArgumentResolver;
import zipgo.auth.presentation.OptionalJwtArgumentResolver;
import zipgo.auth.support.JwtProvider;
import zipgo.common.interceptor.LoggingInterceptor;

import static org.springframework.http.HttpHeaders.LOCATION;

Expand All @@ -25,6 +26,7 @@ public class WebConfig implements WebMvcConfigurer {
private static final String FRONTEND_LOCALHOST = "http://localhost:3000";

private final AuthInterceptor authInterceptor;
private final LoggingInterceptor loggingInterceptor;
private final JwtProvider jwtProvider;

@Override
Expand All @@ -39,6 +41,8 @@ public void addCorsMappings(CorsRegistry registry) {
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(authInterceptor)
.addPathPatterns("/auth");

registry.addInterceptor(loggingInterceptor);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package zipgo.common.interceptor;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.HandlerInterceptor;
import zipgo.aspect.QueryCounter;

@Slf4j
@Component
@RequiredArgsConstructor
public class LoggingInterceptor implements HandlerInterceptor {

private static final String QUERY_COUNT_LOG = "METHOD: {}, URL: {}, STATUS_CODE: {}, QUERY_COUNT: {}";
private static final String QUERY_COUNT_WARN_LOG = "쿼리가 {}번 이상 실행되었습니다!!!";
private static final int WARN_QUERY_COUNT= 8;
Copy link
Collaborator

Choose a reason for hiding this comment

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

WARN_QUERY_COUNT가 8인 이유가 궁금합니다요! 🤓

Copy link
Member Author

Choose a reason for hiding this comment

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

딱히 큰 이유는 없고, 5번과 10번중 고민하다 사이 값으로 했습니다!


private final QueryCounter queryCounter;

@Override
public void afterCompletion(HttpServletRequest request, HttpServletResponse response,
Object handler, Exception ex) {
int queryCount = queryCounter.getCount();
log.info(QUERY_COUNT_LOG, request.getMethod(), request.getRequestURI(), response.getStatus(), queryCount);
if (queryCount >= WARN_QUERY_COUNT) {
log.warn(QUERY_COUNT_WARN_LOG, WARN_QUERY_COUNT);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,26 @@

import com.epages.restdocs.apispec.MockMvcRestDocumentationWrapper;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.nio.charset.StandardCharsets;
import java.util.List;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.restdocs.AutoConfigureRestDocs;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType;
import org.springframework.mock.web.MockMultipartFile;
import org.springframework.restdocs.mockmvc.RestDocumentationResultHandler;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.MockMvc;
import zipgo.admin.application.AdminQueryService;
import zipgo.admin.application.AdminService;
import zipgo.admin.dto.BrandCreateRequest;
import zipgo.admin.dto.PetFoodCreateRequest;
import zipgo.auth.presentation.AuthInterceptor;
import zipgo.auth.presentation.JwtMandatoryArgumentResolver;
import zipgo.auth.support.JwtProvider;
import zipgo.common.acceptance.MockMvcTest;
import zipgo.image.application.ImageService;
import zipgo.petfood.domain.fixture.PetFoodFixture;

import java.nio.charset.StandardCharsets;
import java.util.List;
Copy link
Collaborator

Choose a reason for hiding this comment

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

민무 idea 설정 바뀐듯

Copy link
Member Author

Choose a reason for hiding this comment

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

import 패키지 이렇게 분리되는게 맞지 않나


import static com.epages.restdocs.apispec.RestAssuredRestDocumentationWrapper.resourceDetails;
import static org.mockito.Mockito.when;
import static org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders.multipart;
Expand All @@ -37,19 +31,12 @@
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import static zipgo.brand.domain.fixture.BrandFixture.무민_브랜드_생성_요청;

@AutoConfigureRestDocs
@ExtendWith(SpringExtension.class)
@SuppressWarnings("NonAsciiCharacters")
@WebMvcTest(controllers = AdminController.class)
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class AdminControllerMockTest {
class AdminControllerMockTest extends MockMvcTest {

@Autowired
private ObjectMapper objectMapper;

@Autowired
private MockMvc mockMvc;

@MockBean
private ImageService imageService;

Expand All @@ -59,12 +46,6 @@ class AdminControllerMockTest {
@MockBean
private AdminQueryService adminQueryService;

@MockBean
private JwtProvider jwtProvider;

@MockBean
private AuthInterceptor authInterceptor;

@MockBean
private JwtMandatoryArgumentResolver argumentResolver;

Expand Down Expand Up @@ -102,7 +83,9 @@ class AdminControllerMockTest {
partWithName("brandCreateRequest").description("브랜드 생성 요청").attributes(
key("contentType").value("application/json")
)));
}
}{
}


@Nested
class 식품_생성 {
Expand Down Expand Up @@ -145,4 +128,4 @@ class 식품_생성 {

}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,20 @@
import com.epages.restdocs.apispec.MockMvcRestDocumentationWrapper;
import com.epages.restdocs.apispec.ResourceSnippetDetails;
import com.epages.restdocs.apispec.Schema;
import java.util.List;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.restdocs.AutoConfigureRestDocs;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.restdocs.mockmvc.RestDocumentationResultHandler;
import org.springframework.restdocs.payload.JsonFieldType;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.MockMvc;
import zipgo.auth.application.AuthService;
import zipgo.auth.exception.OAuthTokenNotBringException;
import zipgo.auth.support.JwtProvider;
import zipgo.common.acceptance.MockMvcTest;
import zipgo.member.application.MemberQueryService;
import zipgo.pet.application.PetQueryService;
import zipgo.pet.domain.fixture.PetFixture;

import java.util.List;

import static com.epages.restdocs.apispec.RestAssuredRestDocumentationWrapper.resourceDetails;
import static java.util.Collections.EMPTY_LIST;
import static org.mockito.Mockito.when;
Expand All @@ -34,30 +28,20 @@
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import static zipgo.member.domain.fixture.MemberFixture.식별자_있는_멤버;

@AutoConfigureRestDocs
@ExtendWith(SpringExtension.class)
@SuppressWarnings("NonAsciiCharacters")
@WebMvcTest(controllers = AuthController.class)
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class AuthControllerTest {
class AuthControllerTest extends MockMvcTest {

private static final Schema 응답_형식 = Schema.schema("TokenResponse");
private static final ResourceSnippetDetails 문서_정보 = resourceDetails().summary("로그인")
.description("로그인 합니다.")
.responseSchema(응답_형식);

@Autowired
private MockMvc mockMvc;

@MockBean
private MemberQueryService memberQueryService;

@MockBean
private PetQueryService petQueryService;

@MockBean
private JwtProvider jwtProvider;

@MockBean
private AuthService authService;

Expand Down
31 changes: 31 additions & 0 deletions backend/src/test/java/zipgo/common/acceptance/MockMvcTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package zipgo.common.acceptance;

import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.restdocs.AutoConfigureRestDocs;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.MockMvc;
import zipgo.aspect.QueryCounter;
import zipgo.auth.support.JwtProvider;

@WebMvcTest
@AutoConfigureRestDocs
@ExtendWith(SpringExtension.class)
@SuppressWarnings("NonAsciiCharacters")
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
public class MockMvcTest {

@Autowired
protected MockMvc mockMvc;

@MockBean
protected JwtProvider jwtProvider;

@MockBean
protected QueryCounter queryCounter;

}
Original file line number Diff line number Diff line change
@@ -1,23 +1,17 @@
package zipgo.image.presentaion;

import com.epages.restdocs.apispec.MockMvcRestDocumentationWrapper;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.restdocs.AutoConfigureRestDocs;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType;
import org.springframework.mock.web.MockMultipartFile;
import org.springframework.restdocs.mockmvc.RestDocumentationResultHandler;
import org.springframework.restdocs.payload.JsonFieldType;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.MockMvc;
import zipgo.auth.presentation.AuthInterceptor;
import zipgo.auth.presentation.JwtMandatoryArgumentResolver;
import zipgo.auth.support.JwtProvider;
import zipgo.common.acceptance.MockMvcTest;
import zipgo.image.ImageDirectoryUrl;
import zipgo.image.application.ImageService;

Expand All @@ -32,25 +26,15 @@
import static org.springframework.restdocs.request.RequestDocumentation.requestParts;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

@AutoConfigureRestDocs
@ExtendWith(SpringExtension.class)
@SuppressWarnings("NonAsciiCharacters")
@WebMvcTest(controllers = ImageController.class)
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class ImageControllerTest {
class ImageControllerTest extends MockMvcTest {

@Autowired
private MockMvc mockMvc;

@MockBean
private ImageService imageService;

@MockBean
private JwtProvider jwtProvider;

@MockBean
private AuthInterceptor authInterceptor;

@MockBean
private JwtMandatoryArgumentResolver argumentResolver;

Expand Down