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.

Sign up for GitHub

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

fix: AB테스트 토큰 발급용 API 추가 #1104

Merged
merged 3 commits into from
Dec 3, 2024
+71 −12
Merged

fix: AB테스트 토큰 발급용 API 추가 #1104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f756bf
fix: NPE 발생 부분 임시 조치
songsunkook Nov 30, 2024
c63ed36
feat: accessHistoryId 토큰 발급 API 추가 및 일부 리팩토링
songsunkook Dec 1, 2024
e0190f3
Merge branch 'refs/heads/develop' into fix/abtest-token-api
songsunkook Dec 1, 2024
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
15 changes: 15 additions & 0 deletions src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import in.koreatech.koin.admin.abtest.dto.request.AbtestAssignRequest;
import in.koreatech.koin.admin.abtest.dto.request.AbtestCloseRequest;
import in.koreatech.koin.admin.abtest.dto.request.AbtestRequest;
import in.koreatech.koin.admin.abtest.dto.response.AbtestAccessHistoryResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestAssignResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestDevicesResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestResponse;
Expand Down Expand Up @@ -179,6 +180,20 @@ ResponseEntity<Void> assignAbtestVariableByAdmin(
@RequestBody @Valid AbtestAdminAssignRequest abtestAdminAssignRequest
);

@ApiResponses(
value = {
@ApiResponse(responseCode = "200"),
@ApiResponse(responseCode = "400", content = @Content(schema = @Schema(hidden = true))),
@ApiResponse(responseCode = "404", content = @Content(schema = @Schema(hidden = true)))
}
)
@Operation(summary = "(NORMAL) AB테스트 토큰(access_history_id) 발급")
@PostMapping("/assign/token")
ResponseEntity<AbtestAccessHistoryResponse> issueAccessHistoryId(
@UserAgent UserAgentInfo userAgentInfo,
@UserId Integer userId
);

@ApiResponses(
value = {
@ApiResponse(responseCode = "200"),
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import in.koreatech.koin.admin.abtest.dto.request.AbtestAssignRequest;
import in.koreatech.koin.admin.abtest.dto.request.AbtestCloseRequest;
import in.koreatech.koin.admin.abtest.dto.request.AbtestRequest;
import in.koreatech.koin.admin.abtest.dto.response.AbtestAccessHistoryResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestAssignResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestDevicesResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestResponse;
Expand Down Expand Up @@ -125,6 +126,15 @@ public ResponseEntity<Void> assignAbtestVariableByAdmin(
return ResponseEntity.ok().build();
}

@PostMapping("/assign/token")
public ResponseEntity<AbtestAccessHistoryResponse> issueAccessHistoryId(
@UserAgent UserAgentInfo userAgentInfo,
@UserId Integer userId
) {
AbtestAccessHistoryResponse response = abtestService.issueAccessHistoryId(userAgentInfo, userId);
return ResponseEntity.ok(response);
}

@PostMapping("/assign")
public ResponseEntity<AbtestAssignResponse> assignOrGetAbtestVariable(
@RequestHeader(value = "access_history_id", required = false) Integer accessHistoryId,
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/in/koreatech/koin/admin/abtest/dto/response/AbtestAccessHistoryResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package in.koreatech.koin.admin.abtest.dto.response;

import in.koreatech.koin.admin.abtest.model.AccessHistory;
import io.swagger.v3.oas.annotations.media.Schema;

public record AbtestAccessHistoryResponse(
@Schema(description = "기기 식별자")
Integer accessHistoryId
) {

public static AbtestAccessHistoryResponse from(AccessHistory accessHistory) {
return new AbtestAccessHistoryResponse(accessHistory.getId());
}
}
4 changes: 3 additions & 1 deletion src/main/java/in/koreatech/koin/admin/abtest/dto/response/AbtestDevicesResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.fasterxml.jackson.databind.PropertyNamingStrategies.SnakeCaseStrategy;
import com.fasterxml.jackson.databind.annotation.JsonNaming;

import in.koreatech.koin.admin.abtest.model.AccessHistory;
import in.koreatech.koin.admin.abtest.model.Device;
import io.swagger.v3.oas.annotations.media.Schema;

Expand Down Expand Up @@ -37,11 +38,12 @@ private record InnerDeviceResponse(
) {

public static InnerDeviceResponse from(Device device) {
AccessHistory accessHistory = device.getAccessHistory();
return new InnerDeviceResponse(
device.getId(),
device.getType(),
device.getModel(),
device.getAccessHistory().getLastAccessedAt().toLocalDate());
accessHistory != null ? accessHistory.getLastAccessedAt().toLocalDate() : null);
}
}
}
6 changes: 6 additions & 0 deletions src/main/java/in/koreatech/koin/admin/abtest/model/AccessHistory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ private AccessHistory(
this.lastAccessedAt = lastAccessedAt;
}

public static AccessHistory create() {
return AccessHistory.builder()
.lastAccessedAt(LocalDateTime.now())
.build();
}

public void connectDevice(Device device) {
this.device = device;
device.setAccessHistory(this);
Expand Down
31 changes: 21 additions & 10 deletions src/main/java/in/koreatech/koin/admin/abtest/service/AbtestService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import in.koreatech.koin.admin.abtest.dto.request.AbtestAssignRequest;
import in.koreatech.koin.admin.abtest.dto.request.AbtestCloseRequest;
import in.koreatech.koin.admin.abtest.dto.request.AbtestRequest;
import in.koreatech.koin.admin.abtest.dto.response.AbtestAccessHistoryResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestAssignResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestDevicesResponse;
import in.koreatech.koin.admin.abtest.dto.response.AbtestResponse;
Expand Down Expand Up @@ -139,8 +140,7 @@ public void closeAbtest(Integer abtestId, AbtestCloseRequest request) {

@Transactional
public AbtestAssignResponse assignOrGetVariable(Integer accessHistoryId, UserAgentInfo userAgentInfo,
Integer userId,
AbtestAssignRequest request) {
Integer userId, AbtestAssignRequest request) {
Abtest abtest = abtestRepository.getByTitle(request.title());
AccessHistory accessHistory = findOrCreateAccessHistory(accessHistoryId);
Optional<AbtestVariable> winnerResponse = returnWinnerIfClosed(abtest);
Expand Down Expand Up @@ -312,31 +312,42 @@ private static Optional<AbtestVariable> returnWinnerIfClosed(Abtest abtest) {

public AccessHistory findOrCreateAccessHistory(Integer id) {
if (id == null) {
return accessHistoryRepository.save(AccessHistory.builder().build());
return accessHistoryRepository.save(AccessHistory.create());
}
return accessHistoryRepository.getById(id);
}

@Transactional
public AbtestAccessHistoryResponse issueAccessHistoryId(UserAgentInfo userAgentInfo, Integer userId) {
AccessHistory accessHistory = accessHistoryRepository.save(AccessHistory.create());
if (userId != null) {
createDeviceIfNotExists(userId, userAgentInfo, accessHistory, null);
}
return AbtestAccessHistoryResponse.from(accessHistory);
}

private void createDeviceIfNotExists(Integer userId, UserAgentInfo userAgentInfo,
AccessHistory accessHistory, Abtest abtest) {
userRepository.getById(userId);
if (accessHistory.getDevice() == null) {
Device device = deviceRepository.save(
User user = userRepository.getById(userId);
Device device = accessHistory.getDevice();
if (device == null) {
device = deviceRepository.save(
Device.builder()
.user(userRepository.getById(userId))
.user(user)
.model(userAgentInfo.getModel())
.type(userAgentInfo.getType())
.build()
);
accessHistory.connectDevice(device);
}
Device device = accessHistory.getDevice();
if (device.getModel() == null || device.getType() == null) {
device.setModelInfo(userAgentInfo.getModel(), userAgentInfo.getType());
}
if (!Objects.equals(device.getUser().getId(), userId)) {
device.changeUser(userRepository.getById(userId));
removeBeforeUserCache(accessHistory, abtest);
device.changeUser(user);
if (abtest != null) {
removeBeforeUserCache(accessHistory, abtest);
}
}
}
}
3 changes: 2 additions & 1 deletion src/main/java/in/koreatech/koin/admin/history/aop/AdminActivityHistoryAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ private void excludeGetMapping() {
+ "!execution(* in.koreatech.koin.admin.user.controller.AdminUserController.refresh(..)) && "
+ "!execution(* in.koreatech.koin.admin.user.controller.AdminUserController.createAdmin(..)) && "
+ "!execution(* in.koreatech.koin.admin.user.controller.AdminUserController.adminPasswordChange(..)) && "
+ "!execution(* in.koreatech.koin.admin.abtest.controller.AbtestController.assignOrGetAbtestVariable(..))")
+ "!execution(* in.koreatech.koin.admin.abtest.controller.AbtestController.assignOrGetAbtestVariable(..)) &&"
+ "!execution(* in.koreatech.koin.admin.abtest.controller.AbtestController.issueAccessHistoryId(..))")
Copy link
Contributor

Choose a reason for hiding this comment

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

A

처리잘해주셨어요 !!
히스토리 기능은 별도의 어노테이션을 적용해서 개선하는 방향으로 가야겠다는 생각이 이번에 들었네용,,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어노테이션도 좋은 방향인 것 같아요 👍

로직은 이해를 했는데, 코드 정리의 필요성(?)을 느낀 것 같아요. 리펙토링 계획이 있으신지 궁금합니다 !

당시에는 나름 예쁘다고 생각했는데.. 지금봐도 나름 예쁘다고 생각하는데....... 메서드분리 잘하지않았나... 싶었는데 다들 문제가 있어보인다고 하니 확실히 리팩토링을 해봐야겠네요.. 😥
DB에서 여러번 조회하는 등 문제있는 로직이 분명 존재하는 걸 확인하기도 했으니 방학중으로 날잡고 손보겠습니다.

private void excludeSpecificMethods() {
}

Expand Down
Loading