Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
.requestMatchers("/error").permitAll()
.requestMatchers("/error/**").permitAll()
.requestMatchers("/actuator/**").permitAll()
.requestMatchers("/terms").permitAll()

Choose a reason for hiding this comment

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

medium

The /terms endpoint is now publicly accessible. While /terms itself might be public for viewing general terms, /terms/me and /terms/me (POST) are protected by PreAuthorize. Ensure that the public access to /terms is intentional and does not expose any sensitive information or allow unauthorized actions, especially if there are plans to add more sub-paths under /terms that should be protected.

.anyRequest().authenticated())

.addFilterBefore(usernamePasswordAuthenticationFilter(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package kr.swyp.backend.member.repository;

import java.util.List;
import java.util.Optional;
import java.util.UUID;
import kr.swyp.backend.member.domain.MemberTermsAgreement;
import org.springframework.data.jpa.repository.JpaRepository;

public interface MemberTermsAgreementRepository extends JpaRepository<MemberTermsAgreement, Long> {

List<MemberTermsAgreement> findAllByMemberId(UUID memberId);

Optional<MemberTermsAgreement> findByMemberIdAndTermsId(UUID memberId, Long termsId);

void deleteAllByMemberId(UUID memberId);

Choose a reason for hiding this comment

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

medium

The deleteAllByMemberId method is used to remove all terms agreements for a given member. While this is functional, consider using a @Modifying query with @Query annotation for bulk deletion, which can be more efficient than fetching all entities and then deleting them one by one, especially for a large number of agreements. This can improve performance during member withdrawal.

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import kr.swyp.backend.member.repository.MemberCheckRateRepository;
import kr.swyp.backend.member.repository.MemberRepository;
import kr.swyp.backend.member.repository.MemberSocialLoginInfoRepository;
import kr.swyp.backend.member.repository.MemberTermsAgreementRepository;
import kr.swyp.backend.member.repository.MemberWithdrawalLogRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
Expand All @@ -30,6 +31,7 @@ public class MemberServiceImpl implements MemberService {
private final MemberWithdrawalLogRepository memberWithdrawalLogRepository;
private final MemberCheckRateRepository memberCheckRateRepository;
private final FriendRepository friendRepository;
private final MemberTermsAgreementRepository memberTermsAgreementRepository;

@Transactional(readOnly = true)
public MemberInfoResponse getMemberInfo(UUID memberId) {
Expand Down Expand Up @@ -57,6 +59,9 @@ public void withdrawMember(UUID memberId, MemberWithdrawRequest request) {
memberCheckRateRepository.findByMember(member)
.ifPresent(memberCheckRateRepository::delete);

// 약관 동의 정보 삭제
memberTermsAgreementRepository.deleteAllByMemberId(memberId);

// 탈퇴 처리
member.updateWithdrawnAt();

Expand Down
52 changes: 52 additions & 0 deletions src/main/java/kr/swyp/backend/term/controller/TermController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package kr.swyp.backend.term.controller;

import jakarta.validation.Valid;
import java.util.List;
import java.util.UUID;
import kr.swyp.backend.member.dto.MemberDetails;
import kr.swyp.backend.term.dto.TermDto.TermResponse;
import kr.swyp.backend.term.dto.TermDto.TermsAgreementRequest;
import kr.swyp.backend.term.dto.TermDto.TermsAgreementResponse;
import kr.swyp.backend.term.service.TermService;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping("/terms")
@RequiredArgsConstructor
public class TermController {

private final TermService termService;

@GetMapping
public ResponseEntity<List<TermResponse>> getAllTerms() {
List<TermResponse> terms = termService.getAllTerms();
return ResponseEntity.ok(terms);
}

@GetMapping("/me")
@PreAuthorize("hasAnyAuthority('USER', 'ADMIN', 'SUPER_ADMIN')")
public ResponseEntity<TermsAgreementResponse> getMyTermsAgreements(
@AuthenticationPrincipal MemberDetails memberDetails) {
UUID memberId = memberDetails.getMemberId();
TermsAgreementResponse response = termService.getMemberTermsAgreements(memberId);
return ResponseEntity.ok(response);
}

@PostMapping("/me")
@PreAuthorize("hasAnyAuthority('USER', 'ADMIN', 'SUPER_ADMIN')")
public ResponseEntity<Void> saveMyTermsAgreements(
@AuthenticationPrincipal MemberDetails memberDetails,
@Valid @RequestBody TermsAgreementRequest request) {
UUID memberId = memberDetails.getMemberId();
termService.saveMemberTermsAgreements(memberId, request);
return ResponseEntity.ok().build();
}
}
100 changes: 100 additions & 0 deletions src/main/java/kr/swyp/backend/term/dto/TermDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package kr.swyp.backend.term.dto;

import jakarta.validation.constraints.NotEmpty;
import jakarta.validation.constraints.NotNull;
import java.time.LocalDateTime;
import java.util.List;
import kr.swyp.backend.member.domain.MemberTermsAgreement;
import kr.swyp.backend.term.domain.Term;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

public class TermDto {

@Getter
@Builder
@NoArgsConstructor
@AllArgsConstructor
public static class TermResponse {

private Long termId;
private String title;
private String version;
private Boolean isRequired;

public static TermResponse fromEntity(Term term) {
return TermResponse.builder()
.termId(term.getId())
.title(term.getTitle())
.version(term.getVersion())
.isRequired(term.getIsRequired())
.build();
}
}

@Getter
@Builder
@NoArgsConstructor
@AllArgsConstructor
public static class TermAgreementRequest {

@NotNull(message = "약관 ID는 필수입니다.")
private Long termId;

@NotNull(message = "동의 여부는 필수입니다.")
private Boolean isAgreed;
}

@Getter
@Builder
@NoArgsConstructor
@AllArgsConstructor
public static class TermsAgreementRequest {

@NotEmpty(message = "약관 동의 목록은 비어있을 수 없습니다.")
private List<TermAgreementRequest> agreements;
}

@Getter
@Builder
@NoArgsConstructor
@AllArgsConstructor
public static class TermAgreementResponse {

private Long termId;
private String title;
private String version;
private Boolean isRequired;
private Boolean isAgreed;
private LocalDateTime agreedAt;

public static TermAgreementResponse fromEntity(Term term,
MemberTermsAgreement agreement) {
return TermAgreementResponse.builder()
.termId(term.getId())
.title(term.getTitle())
.version(term.getVersion())
.isRequired(term.getIsRequired())
.isAgreed(agreement != null && agreement.getIsAgreed())
.agreedAt(agreement != null ? agreement.getCreatedAt() : null)
.build();
}
}

@Getter
@Builder
@NoArgsConstructor
@AllArgsConstructor
public static class TermsAgreementResponse {

private List<TermAgreementResponse> agreements;

public static TermsAgreementResponse of(List<TermAgreementResponse> agreements) {
return TermsAgreementResponse.builder()
.agreements(agreements)
.build();
}
}
}
10 changes: 10 additions & 0 deletions src/main/java/kr/swyp/backend/term/repository/TermRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package kr.swyp.backend.term.repository;

import java.util.Optional;
import kr.swyp.backend.term.domain.Term;
import org.springframework.data.jpa.repository.JpaRepository;

public interface TermRepository extends JpaRepository<Term, Long> {

Optional<Term> findByTitle(String title);
}
26 changes: 26 additions & 0 deletions src/main/java/kr/swyp/backend/term/service/TermService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package kr.swyp.backend.term.service;

import java.util.List;
import java.util.UUID;
import kr.swyp.backend.term.dto.TermDto.TermAgreementResponse;
import kr.swyp.backend.term.dto.TermDto.TermResponse;
import kr.swyp.backend.term.dto.TermDto.TermsAgreementRequest;
import kr.swyp.backend.term.dto.TermDto.TermsAgreementResponse;

public interface TermService {

/**
* 모든 약관 목록 조회.
*/
List<TermResponse> getAllTerms();

/**
* 특정 회원의 약관 동의 상태 조회.
*/
TermsAgreementResponse getMemberTermsAgreements(UUID memberId);

/**
* 회원의 약관 동의 저장/업데이트.
*/
void saveMemberTermsAgreements(UUID memberId, TermsAgreementRequest request);
}
100 changes: 100 additions & 0 deletions src/main/java/kr/swyp/backend/term/service/TermServiceImpl.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package kr.swyp.backend.term.service;

import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.UUID;
import java.util.stream.Collectors;
import kr.swyp.backend.member.domain.MemberTermsAgreement;
import kr.swyp.backend.member.repository.MemberTermsAgreementRepository;
import kr.swyp.backend.term.domain.Term;
import kr.swyp.backend.term.dto.TermDto.TermAgreementRequest;
import kr.swyp.backend.term.dto.TermDto.TermAgreementResponse;
import kr.swyp.backend.term.dto.TermDto.TermResponse;
import kr.swyp.backend.term.dto.TermDto.TermsAgreementRequest;
import kr.swyp.backend.term.dto.TermDto.TermsAgreementResponse;
import kr.swyp.backend.term.repository.TermRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class TermServiceImpl implements TermService {

private final TermRepository termRepository;
private final MemberTermsAgreementRepository memberTermsAgreementRepository;

@Override
@Transactional(readOnly = true)
public List<TermResponse> getAllTerms() {
return termRepository.findAll().stream()
.map(TermResponse::fromEntity)
.collect(Collectors.toList());
}

@Override
@Transactional(readOnly = true)
public TermsAgreementResponse getMemberTermsAgreements(UUID memberId) {
// 모든 약관 조회
List<Term> allTerms = termRepository.findAll();

// 회원의 약관 동의 정보 조회
List<MemberTermsAgreement> memberAgreements =
memberTermsAgreementRepository.findAllByMemberId(memberId);

// termId를 키로 하는 Map 생성
Map<Long, MemberTermsAgreement> agreementMap = memberAgreements.stream()
.collect(Collectors.toMap(
MemberTermsAgreement::getTermsId,
agreement -> agreement
));

// 모든 약관에 대해 동의 상태를 포함한 응답 생성
List<TermAgreementResponse> responses = allTerms.stream()
.map(term -> TermAgreementResponse.fromEntity(
term,
agreementMap.get(term.getId())
))
.collect(Collectors.toList());

return TermsAgreementResponse.of(responses);
}

@Override
@Transactional
public void saveMemberTermsAgreements(UUID memberId, TermsAgreementRequest request) {
for (TermAgreementRequest agreementRequest : request.getAgreements()) {
// 약관 존재 여부 확인
Term term = termRepository.findById(agreementRequest.getTermId())
.orElseThrow(() -> new NoSuchElementException(
"약관을 찾을 수 없습니다. ID: " + agreementRequest.getTermId()));

// 필수 약관인데 동의하지 않은 경우 예외 발생
if (term.getIsRequired() && !agreementRequest.getIsAgreed()) {
throw new IllegalArgumentException(
"필수 약관에 동의해야 합니다: " + term.getTitle());
}

// 기존 동의 정보 조회
MemberTermsAgreement existingAgreement =
memberTermsAgreementRepository.findByMemberIdAndTermsId(
memberId, agreementRequest.getTermId()
).orElse(null);

if (existingAgreement != null) {
// 기존 레코드가 있으면 삭제 후 새로 생성 (업데이트)
memberTermsAgreementRepository.delete(existingAgreement);
}
Comment on lines +85 to +88

Choose a reason for hiding this comment

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

medium

The current implementation deletes an existing MemberTermsAgreement and then saves a new one to update it. This approach can lead to unnecessary database operations (delete then insert) and potential issues with database triggers or auditing if not handled carefully. A more efficient approach would be to update the isAgreed status of the existingAgreement directly if it exists, and only create a new one if it doesn't.

            if (existingAgreement != null) {
                existingAgreement.setIsAgreed(agreementRequest.getIsAgreed());
                memberTermsAgreementRepository.save(existingAgreement);
            } else {


// 새로운 동의 정보 저장
MemberTermsAgreement newAgreement = MemberTermsAgreement.builder()
.memberId(memberId)
.termsId(agreementRequest.getTermId())
.isAgreed(agreementRequest.getIsAgreed())
.build();

memberTermsAgreementRepository.save(newAgreement);
Comment on lines +90 to +97

Choose a reason for hiding this comment

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

medium

Following up on the previous comment, if an existing agreement is found, it should be updated instead of deleted and re-created. The else block should contain the creation of a new agreement only if existingAgreement is null.

            } else {
                // 새로운 동의 정보 저장
                MemberTermsAgreement newAgreement = MemberTermsAgreement.builder()
                        .memberId(memberId)
                        .termsId(agreementRequest.getTermId())
                        .isAgreed(agreementRequest.getIsAgreed())
                        .build();
                memberTermsAgreementRepository.save(newAgreement);
            }

Comment on lines +86 to +97
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The delete-then-insert pattern for updates is inefficient and may cause unnecessary database operations. Consider implementing an update method on the entity or using a single upsert operation. If the BaseEntity class provides timestamp tracking, the current approach also loses the original creation timestamp.

Suggested change
// 기존 레코드가 있으면 삭제 후 새로 생성 (업데이트)
memberTermsAgreementRepository.delete(existingAgreement);
}
// 새로운 동의 정보 저장
MemberTermsAgreement newAgreement = MemberTermsAgreement.builder()
.memberId(memberId)
.termsId(agreementRequest.getTermId())
.isAgreed(agreementRequest.getIsAgreed())
.build();
memberTermsAgreementRepository.save(newAgreement);
// 기존 레코드가 있으면 업데이트
existingAgreement.setIsAgreed(agreementRequest.getIsAgreed());
memberTermsAgreementRepository.save(existingAgreement);
} else {
// 새로운 동의 정보 저장
MemberTermsAgreement newAgreement = MemberTermsAgreement.builder()
.memberId(memberId)
.termsId(agreementRequest.getTermId())
.isAgreed(agreementRequest.getIsAgreed())
.build();
memberTermsAgreementRepository.save(newAgreement);
}

Copilot uses AI. Check for mistakes.
}
}
Comment on lines +64 to +99

Choose a reason for hiding this comment

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

high

현재 약관 동의 저장 로직은 요청에 포함된 각 약관에 대해 데이터베이스 조회, 삭제, 저장을 반복하고 있습니다. 이 방식은 요청하는 약관의 수(N)에 비례하여 DB I/O가 증가하여(N번의 SELECT, N번의 DELETE, N번의 INSERT 가능성) 비효율적입니다.

또한, 이 로직은 요청에 포함되지 않은 기존 약관 동의 정보를 처리하지 않아, 사용자가 선택 약관 동의를 철회하는 시나리오(요청에서 해당 약관을 제외)에서 기존 동의가 삭제되지 않는 문제가 발생할 수 있습니다.

전체 약관 동의 상태를 한 번에 교체하는 방식으로 로직을 개선하는 것을 제안합니다. 즉, 특정 회원의 기존 약관 동의를 모두 삭제한 후, 요청받은 약관 동의 목록을 한 번에 저장하는 것입니다. 이렇게 하면 DB 접근 횟수를 크게 줄일 수 있고 로직이 명확해집니다.

    @Override
    @Transactional
    public void saveMemberTermsAgreements(UUID memberId, TermsAgreementRequest request) {
        List<TermAgreementRequest> agreementRequests = request.getAgreements();
        List<Long> termIds = agreementRequests.stream()
                .map(TermAgreementRequest::getTermId)
                .collect(Collectors.toList());

        // 1. 약관 유효성 검증
        Map<Long, Term> termsMap = termRepository.findAllById(termIds).stream()
                .collect(Collectors.toMap(Term::getId, t -> t));

        if (termsMap.size() != agreementRequests.size()) {
            // 요청된 약관 ID 중 존재하지 않는 것이 있음
            throw new NoSuchElementException("하나 이상의 약관을 찾을 수 없습니다.");
        }

        for (TermAgreementRequest agreementRequest : agreementRequests) {
            Term term = termsMap.get(agreementRequest.getTermId());
            if (term.getIsRequired() && !agreementRequest.getIsAgreed()) {
                throw new IllegalArgumentException("필수 약관에 동의해야 합니다: " + term.getTitle());
            }
        }

        // 2. 기존 동의 내역 일괄 삭제
        memberTermsAgreementRepository.deleteAllByMemberId(memberId);

        // 3. 새로운 동의 내역 일괄 저장
        List<MemberTermsAgreement> newAgreements = agreementRequests.stream()
                .map(agreementRequest -> MemberTermsAgreement.builder()
                        .memberId(memberId)
                        .termsId(agreementRequest.getTermId())
                        .isAgreed(agreementRequest.getIsAgreed())
                        .build())
                .collect(Collectors.toList());

        memberTermsAgreementRepository.saveAll(newAgreements);
    }

}
Loading