From e7fcc74cd8a966224435485064e5116095bef4a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=86=A1=EC=84=A0=EA=B6=8C?= Date: Fri, 20 Sep 2024 19:47:53 +0900 Subject: [PATCH] =?UTF-8?q?feat:=20ab=ED=85=8C=EC=8A=A4=ED=8A=B8=20?= =?UTF-8?q?=ED=8E=B8=EC=9E=85=20api=20=EB=8B=A8=EC=9D=BC=ED=99=94=20(#907)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: abtest 편입 관련 api 단일화 * test: 테스트 수정 --- .../admin/abtest/controller/AbtestApi.java | 20 ++--------- .../abtest/controller/AbtestController.java | 15 ++------- .../AbtestAssignedUserException.java | 20 ----------- ...oryAbtestVariableCustomRepositoryImpl.java | 33 ------------------- .../admin/abtest/service/AbtestService.java | 33 +++++++++---------- .../koin/acceptance/AbtestApiTest.java | 14 +++++--- 6 files changed, 29 insertions(+), 106 deletions(-) delete mode 100644 src/main/java/in/koreatech/koin/admin/abtest/exception/AbtestAssignedUserException.java delete mode 100644 src/main/java/in/koreatech/koin/admin/abtest/repository/AccessHistoryAbtestVariableCustomRepositoryImpl.java diff --git a/src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestApi.java b/src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestApi.java index 945df4b1a..c5ae6ee8b 100644 --- a/src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestApi.java +++ b/src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestApi.java @@ -186,25 +186,9 @@ ResponseEntity assignAbtestVariableByAdmin( @ApiResponse(responseCode = "404", content = @Content(schema = @Schema(hidden = true))) } ) - @Operation(summary = "(NORMAL) 자신의 실험군 조회") - @GetMapping("/me") - ResponseEntity getMyAbtestVariable( - @RequestHeader("accessHistoryId") Integer accessHistoryId, - @UserAgent UserAgentInfo userAgentInfo, - @UserId Integer userId, - @RequestParam(name = "title") String title - ); - - @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) 실험군 최초 편입") + @Operation(summary = "(NORMAL) 실험군 편입 정보 확인") @PostMapping("/assign") - ResponseEntity assignAbtestVariable( + ResponseEntity assignOrGetAbtestVariable( @RequestHeader("accessHistoryId") Integer accessHistoryId, @UserAgent UserAgentInfo userAgentInfo, @UserId Integer userId, diff --git a/src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestController.java b/src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestController.java index cedaff715..c4f4c2584 100644 --- a/src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestController.java +++ b/src/main/java/in/koreatech/koin/admin/abtest/controller/AbtestController.java @@ -125,25 +125,14 @@ public ResponseEntity assignAbtestVariableByAdmin( return ResponseEntity.ok().build(); } - @GetMapping("/me") - public ResponseEntity getMyAbtestVariable( - @RequestHeader("access_history_id") Integer accessHistoryId, - @UserAgent UserAgentInfo userAgentInfo, - @UserId Integer userId, - @RequestParam(name = "title") String title - ) { - String response = abtestService.getMyVariable(accessHistoryId, userAgentInfo, userId, title); - return ResponseEntity.ok(response); - } - @PostMapping("/assign") - public ResponseEntity assignAbtestVariable( + public ResponseEntity assignOrGetAbtestVariable( @RequestHeader(value = "access_history_id", required = false) Integer accessHistoryId, @UserAgent UserAgentInfo userAgentInfo, @UserId Integer userId, @RequestBody @Valid AbtestAssignRequest abtestAssignRequest ) { - AbtestAssignResponse response = abtestService.assignVariable(accessHistoryId, userAgentInfo, userId, abtestAssignRequest); + AbtestAssignResponse response = abtestService.assignOrGetVariable(accessHistoryId, userAgentInfo, userId, abtestAssignRequest); return ResponseEntity.ok(response); } } diff --git a/src/main/java/in/koreatech/koin/admin/abtest/exception/AbtestAssignedUserException.java b/src/main/java/in/koreatech/koin/admin/abtest/exception/AbtestAssignedUserException.java deleted file mode 100644 index 39ffa08c7..000000000 --- a/src/main/java/in/koreatech/koin/admin/abtest/exception/AbtestAssignedUserException.java +++ /dev/null @@ -1,20 +0,0 @@ -package in.koreatech.koin.admin.abtest.exception; - -import in.koreatech.koin.global.exception.KoinIllegalArgumentException; - -public class AbtestAssignedUserException extends KoinIllegalArgumentException { - - private static final String DEFAULT_MESSAGE = "이미 편입된 사용자입니다."; - - public AbtestAssignedUserException(String message) { - super(message); - } - - public AbtestAssignedUserException(String message, String detail) { - super(message, detail); - } - - public static AbtestAssignedUserException withDetail(String detail) { - return new AbtestAssignedUserException(DEFAULT_MESSAGE, detail); - } -} diff --git a/src/main/java/in/koreatech/koin/admin/abtest/repository/AccessHistoryAbtestVariableCustomRepositoryImpl.java b/src/main/java/in/koreatech/koin/admin/abtest/repository/AccessHistoryAbtestVariableCustomRepositoryImpl.java deleted file mode 100644 index f01e0297a..000000000 --- a/src/main/java/in/koreatech/koin/admin/abtest/repository/AccessHistoryAbtestVariableCustomRepositoryImpl.java +++ /dev/null @@ -1,33 +0,0 @@ -package in.koreatech.koin.admin.abtest.repository; - -import java.util.List; - -import org.springframework.stereotype.Repository; - -import com.querydsl.jpa.impl.JPAQueryFactory; - -import in.koreatech.koin.admin.abtest.model.QAccessHistoryAbtestVariable; -import lombok.RequiredArgsConstructor; - -@Repository -@RequiredArgsConstructor -public class AccessHistoryAbtestVariableCustomRepositoryImpl - implements AccessHistoryAbtestVariableCustomRepository { - - private final JPAQueryFactory queryFactory; - - public List findIdsToMove(Integer fromVariableId, int limit) { - return queryFactory.select(QAccessHistoryAbtestVariable.accessHistoryAbtestVariable.id) - .from(QAccessHistoryAbtestVariable.accessHistoryAbtestVariable) - .where(QAccessHistoryAbtestVariable.accessHistoryAbtestVariable.variable.id.eq(fromVariableId)) - .limit(limit) - .fetch(); - } - - public void updateVariableIds(List idsToUpdate, Integer toVariableId) { - queryFactory.update(QAccessHistoryAbtestVariable.accessHistoryAbtestVariable) - .set(QAccessHistoryAbtestVariable.accessHistoryAbtestVariable.variable.id, toVariableId) - .where(QAccessHistoryAbtestVariable.accessHistoryAbtestVariable.id.in(idsToUpdate)) - .execute(); - } -} diff --git a/src/main/java/in/koreatech/koin/admin/abtest/service/AbtestService.java b/src/main/java/in/koreatech/koin/admin/abtest/service/AbtestService.java index 00e4ac4a8..3b729c0f9 100644 --- a/src/main/java/in/koreatech/koin/admin/abtest/service/AbtestService.java +++ b/src/main/java/in/koreatech/koin/admin/abtest/service/AbtestService.java @@ -21,7 +21,6 @@ import in.koreatech.koin.admin.abtest.dto.response.AbtestUsersResponse; import in.koreatech.koin.admin.abtest.dto.response.AbtestsResponse; import in.koreatech.koin.admin.abtest.exception.AbtestAlreadyExistException; -import in.koreatech.koin.admin.abtest.exception.AbtestAssignedUserException; import in.koreatech.koin.admin.abtest.exception.AbtestDuplicatedVariableException; import in.koreatech.koin.admin.abtest.exception.AbtestNotAssignedUserException; import in.koreatech.koin.admin.abtest.exception.AbtestNotInProgressException; @@ -38,7 +37,6 @@ import in.koreatech.koin.admin.abtest.repository.AbtestVariableAssignTemplateRepository; import in.koreatech.koin.admin.abtest.repository.AbtestVariableCountRepository; import in.koreatech.koin.admin.abtest.repository.AbtestVariableRepository; -import in.koreatech.koin.admin.abtest.repository.AccessHistoryAbtestVariableCustomRepository; import in.koreatech.koin.admin.abtest.repository.AccessHistoryRepository; import in.koreatech.koin.admin.abtest.repository.DeviceRepository; import in.koreatech.koin.domain.user.model.User; @@ -63,7 +61,6 @@ public class AbtestService { private final UserRepository userRepository; private final AbtestVariableAssignRepository abtestVariableAssignRepository; private final AbtestVariableAssignTemplateRepository abtestVariableAssignTemplateRepository; - private final AccessHistoryAbtestVariableCustomRepository accessHistoryAbtestVariableCustomRepository; @Transactional public AbtestResponse createAbtest(AbtestRequest request) { @@ -99,7 +96,6 @@ public AbtestResponse putAbtest(Integer abtestId, AbtestRequest request) { return AbtestResponse.from(abtest); } - private void deleteVariableAssignCache(Abtest abtest) { abtest.getAbtestVariables().forEach(abtestVariable -> abtestVariableAssignTemplateRepository.deleteAllByVariableId(abtestVariable.getId())); @@ -144,7 +140,8 @@ public void closeAbtest(Integer abtestId, AbtestCloseRequest request) { } @Transactional - public AbtestAssignResponse assignVariable(Integer accessHistoryId, UserAgentInfo userAgentInfo, Integer userId, + public AbtestAssignResponse assignOrGetVariable(Integer accessHistoryId, UserAgentInfo userAgentInfo, + Integer userId, AbtestAssignRequest request) { Abtest abtest = abtestRepository.getByTitle(request.title()); AccessHistory accessHistory = findOrCreateAccessHistory(accessHistoryId); @@ -152,7 +149,10 @@ public AbtestAssignResponse assignVariable(Integer accessHistoryId, UserAgentInf if (winnerResponse.isPresent()) { return AbtestAssignResponse.of(winnerResponse.get(), accessHistory); } - validateAssignedUser(abtest, accessHistory.getId(), userId); + if (isAssignedUser(abtest, accessHistory.getId(), userId)) { + return AbtestAssignResponse.of( + getMyVariable(accessHistory.getId(), userAgentInfo, userId, request.title()), accessHistory); + } List cacheCount = loadCacheCount(abtest); AbtestVariable variable = abtest.findAssignVariable(cacheCount); if (userId != null) { @@ -193,14 +193,14 @@ private void variableAssignCacheSave(AbtestVariable variable, Integer accessHist abtestVariableAssignRepository.save(AbtestVariableAssign.of(variable.getId(), accessHistoryId)); } - @Transactional - public String getMyVariable(Integer accessHistoryId, UserAgentInfo userAgentInfo, Integer userId, String title) { + private AbtestVariable getMyVariable(Integer accessHistoryId, UserAgentInfo userAgentInfo, Integer userId, + String title) { Abtest abtest = abtestRepository.getByTitle(title); AccessHistory accessHistory = accessHistoryRepository.getById(accessHistoryId); syncCacheCountToDB(abtest); Optional winner = returnWinnerIfClosed(abtest); if (winner.isPresent()) { - return winner.get().getName(); + return winner.get(); } if (userId != null) { createDeviceIfNotExists(userId, userAgentInfo, accessHistory, abtest); @@ -215,23 +215,20 @@ public String getMyVariable(Integer accessHistoryId, UserAgentInfo userAgentInfo .orElseThrow(() -> AbtestNotAssignedUserException.withDetail("abtestId: " + abtest.getId() + ", " + "accessHistoryId: " + accessHistory.getId())); abtestVariableAssignRepository.save(AbtestVariableAssign.of(dbVariable.getId(), accessHistory.getId())); - return dbVariable.getName(); + return dbVariable; } accessHistory.updateLastAccessedAt(clock); - return cacheVariable.get().getName(); + return cacheVariable.get(); } - private void validateAssignedUser(Abtest abtest, Integer accessHistoryId, Integer userId) { + private boolean isAssignedUser(Abtest abtest, Integer accessHistoryId, Integer userId) { AccessHistory accessHistory = accessHistoryRepository.getById(accessHistoryId); if (userId != null && accessHistory.getDevice() != null && !Objects.equals(accessHistory.getDevice().getUser().getId(), userId)) { - return; - } - if (abtest.getAbtestVariables().stream() - .anyMatch(abtestVariable -> accessHistory.hasVariable(abtestVariable.getId()))) { - throw AbtestAssignedUserException.withDetail( - "abtestId: " + abtest.getId() + ", accessHistoryId: " + accessHistoryId); + return false; } + return abtest.getAbtestVariables().stream() + .anyMatch(abtestVariable -> accessHistory.hasVariable(abtestVariable.getId())); } @Transactional diff --git a/src/test/java/in/koreatech/koin/acceptance/AbtestApiTest.java b/src/test/java/in/koreatech/koin/acceptance/AbtestApiTest.java index 1f59994f4..842246195 100644 --- a/src/test/java/in/koreatech/koin/acceptance/AbtestApiTest.java +++ b/src/test/java/in/koreatech/koin/acceptance/AbtestApiTest.java @@ -476,13 +476,17 @@ void setUp() { "title": "dining_ui_test" } """)) - ); + ); MvcResult mvcResult = mockMvc.perform( - get("/abtest/me") + post("/abtest/assign") .contentType(MediaType.APPLICATION_JSON) .header("access_history_id", device.getAccessHistory().getId()) - .param("title", abtest.getTitle()) + .content(String.format(""" + { + "title": "dining_ui_test" + } + """)) ) .andExpect(status().isOk()) .andReturn(); @@ -498,7 +502,9 @@ void setUp() { .map(AccessHistoryAbtestVariable::getVariable) .filter(var -> var.getAbtest().getTitle().equals(abtest.getTitle())) .findAny(); - softly.assertThat(responseBody).isEqualTo(variable.get().getName()); + softly.assertThat(responseBody).isEqualTo( + String.format("{\"variable_name\":\"A\",\"access_history_id\":1}", + variable.get().getName(), result.getAccessHistory().getId())); } ); });