Skip to content

Commit 2c78274

Browse files
dengfeigedengfeige
authored
refactor(member): enhance readability & add unit test (#8)
* refactor(member): enhance readability & add unit test
1 parent 036e99c commit 2c78274

13 files changed

Lines changed: 256 additions & 115 deletions

File tree

‎src/main/java/com/featureprobe/api/base/constants/MessageKey.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ public class MessageKey {
77

88
public static final String NOT_FOUND = "resource.error.not_found";
99

10+
public static final String FORBIDDEN = "resource.error.forbidden";
11+
1012
public static final String INVALID_REQUEST = "validate.invalid_request";
1113

14+
public static final String INVALID_OLD_PASSWORD = "validate.invalid_old_password";
15+
1216
public static final String USING = "resource.error.using";
1317
}

‎src/main/java/com/featureprobe/api/base/constants/ResponseCode.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ public enum ResponseCode {
44

55
CONFLICT("conflict", MessageKey.CONFLICT),
66
NOT_FOUND("not_found", MessageKey.NOT_FOUND),
7-
INVALID_REQUEST("invalid_request", MessageKey.INVALID_REQUEST);
7+
INVALID_REQUEST("invalid_request", MessageKey.INVALID_REQUEST),
8+
FORBIDDEN("forbidden", MessageKey.FORBIDDEN);
89

910
private String code;
1011

‎src/main/java/com/featureprobe/api/base/exception/ForbiddenException.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
package com.featureprobe.api.base.exception;
2+
3+
public class ForbiddenException extends RuntimeException{
4+
}

‎src/main/java/com/featureprobe/api/base/exception/WebExceptionAspect.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,16 @@ public void resourceConflictHandler(HttpServletResponse response, ResourceConfli
4242
response.getWriter().write(toErrorResponse(ResponseCode.CONFLICT));
4343
}
4444

45-
@ExceptionHandler(value = PasswordErrorException.class)
46-
public void passwordErrorHandler(HttpServletResponse response, PasswordErrorException e)
45+
@ExceptionHandler(value = ForbiddenException.class)
46+
public void forbiddenHandler(HttpServletResponse response, ForbiddenException e)
4747
throws IOException {
48-
response.setStatus(HttpStatus.BAD_REQUEST.value());
48+
response.setStatus(HttpStatus.FORBIDDEN.value());
4949
response.setCharacterEncoding(StandardCharsets.UTF_8.name());
50-
response.getWriter().write(toErrorResponse(ResponseCode.INVALID_REQUEST));
50+
response.getWriter().write(toErrorResponse(ResponseCode.FORBIDDEN));
5151
}
5252

5353
@ExceptionHandler(value = IllegalArgumentException.class)
54-
public void invalidHandler(HttpServletResponse response, IllegalArgumentException e)
54+
public void invalidArgumentHandler(HttpServletResponse response, IllegalArgumentException e)
5555
throws IOException {
5656
response.setStatus(HttpStatus.BAD_REQUEST.value());
5757
response.setCharacterEncoding(StandardCharsets.UTF_8.name());

‎src/main/java/com/featureprobe/api/controller/MemberController.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public List<MemberResponse> create(@Validated @RequestBody MemberCreateRequest c
5454
@GetMapping
5555
@Operation(summary = "List Member", description = "Get a list of all member")
5656
public Page<MemberResponse> list(MemberSearchRequest searchRequest) {
57-
return memberService.list(searchRequest);
57+
return memberService.query(searchRequest);
5858
}
5959

6060
@PatchMapping
@@ -78,6 +78,6 @@ public MemberResponse delete(@Validated @RequestBody MemberDeleteRequest deleteR
7878
@GetMapping("/query")
7979
@Operation(summary = "Get member", description = "Get a single member by account.")
8080
public MemberResponse query(String account) {
81-
return memberService.query(account);
81+
return memberService.queryByAccount(account);
8282
}
8383
}

‎src/main/java/com/featureprobe/api/controller/SegmentController.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,17 @@
1010
import com.featureprobe.api.dto.PaginationRequest;
1111
import com.featureprobe.api.dto.SegmentCreateRequest;
1212
import com.featureprobe.api.dto.SegmentResponse;
13+
import com.featureprobe.api.dto.SegmentSearchRequest;
1314
import com.featureprobe.api.dto.SegmentUpdateRequest;
1415
import com.featureprobe.api.dto.ToggleSegmentResponse;
1516
import com.featureprobe.api.service.SegmentService;
16-
import com.featureprobe.api.base.doc.GetApiResponse;
17-
import com.featureprobe.api.dto.SegmentResponse;
18-
import com.featureprobe.api.dto.SegmentSearchRequest;
19-
import com.featureprobe.api.service.SegmentService;
2017
import com.featureprobe.api.validate.ResourceExistsValidate;
2118
import io.swagger.v3.oas.annotations.Operation;
2219
import lombok.AllArgsConstructor;
2320
import lombok.extern.slf4j.Slf4j;
2421
import org.springframework.data.domain.Page;
25-
import org.springframework.web.bind.annotation.DeleteMapping;
26-
import org.springframework.web.bind.annotation.GetMapping;
27-
import org.springframework.web.bind.annotation.PathVariable;
2822
import org.springframework.validation.annotation.Validated;
23+
import org.springframework.web.bind.annotation.DeleteMapping;
2924
import org.springframework.web.bind.annotation.GetMapping;
3025
import org.springframework.web.bind.annotation.PatchMapping;
3126
import org.springframework.web.bind.annotation.PathVariable;
@@ -35,8 +30,6 @@
3530
import org.springframework.web.bind.annotation.RequestParam;
3631
import org.springframework.web.bind.annotation.RestController;
3732

38-
import java.util.List;
39-
4033

4134
@Slf4j
4235
@AllArgsConstructor

‎src/main/java/com/featureprobe/api/service/MemberService.java‎

Lines changed: 88 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package com.featureprobe.api.service;
22

33
import com.featureprobe.api.auth.UserPasswordAuthenticationToken;
4+
import com.featureprobe.api.base.constants.MessageKey;
45
import com.featureprobe.api.base.enums.ResourceType;
56
import com.featureprobe.api.base.enums.RoleEnum;
6-
import com.featureprobe.api.base.exception.PasswordErrorException;
7+
import com.featureprobe.api.base.exception.ForbiddenException;
78
import com.featureprobe.api.base.exception.ResourceConflictException;
89
import com.featureprobe.api.base.exception.ResourceNotFoundException;
910
import com.featureprobe.api.dto.MemberCreateRequest;
@@ -14,50 +15,60 @@
1415
import com.featureprobe.api.entity.Member;
1516
import com.featureprobe.api.mapper.MemberMapper;
1617
import com.featureprobe.api.repository.MemberRepository;
17-
import lombok.AllArgsConstructor;
18+
import com.featureprobe.api.util.PageRequestUtil;
1819
import lombok.extern.slf4j.Slf4j;
1920
import org.apache.commons.lang3.StringUtils;
2021
import org.springframework.data.domain.Page;
21-
import org.springframework.data.domain.PageRequest;
2222
import org.springframework.data.domain.Pageable;
2323
import org.springframework.data.domain.Sort;
2424
import org.springframework.data.jpa.domain.Specification;
2525
import org.springframework.security.core.context.SecurityContextHolder;
2626
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
27+
import org.springframework.security.crypto.password.PasswordEncoder;
2728
import org.springframework.stereotype.Service;
2829
import org.springframework.transaction.annotation.Transactional;
2930

30-
import javax.persistence.criteria.CriteriaBuilder;
31-
import javax.persistence.criteria.CriteriaQuery;
32-
import javax.persistence.criteria.Predicate;
33-
import javax.persistence.criteria.Root;
34-
import java.util.ArrayList;
3531
import java.util.Date;
3632
import java.util.List;
33+
import java.util.Optional;
3734
import java.util.stream.Collectors;
3835

3936

4037
@Slf4j
41-
@AllArgsConstructor
4238
@Service
4339
public class MemberService {
4440

4541
private MemberRepository memberRepository;
4642

43+
private PasswordEncoder passwordEncoder;
44+
45+
public MemberService(MemberRepository memberRepository) {
46+
this.memberRepository = memberRepository;
47+
this.passwordEncoder = new BCryptPasswordEncoder();
48+
}
49+
4750
@Transactional(rollbackFor = Exception.class)
4851
public List<MemberResponse> create(MemberCreateRequest createRequest) {
49-
List<Member> members = new ArrayList<>();
50-
createRequest.getAccounts().stream().forEach(account -> {
51-
if (memberRepository.findByAccountIncludeDeleted(account).isPresent()) {
52-
throw new ResourceConflictException(ResourceType.MEMBER);
53-
}
54-
members.add(newMember(account, createRequest.getPassword()));
55-
});
56-
List<Member> saveMembers = memberRepository.saveAll(members);
57-
return saveMembers.stream().map(item ->
52+
List<Member> savedMembers = memberRepository.saveAll(newNumbers(createRequest));
53+
54+
return savedMembers.stream().map(item ->
5855
MemberMapper.INSTANCE.entityToResponse(item)).collect(Collectors.toList());
5956
}
6057

58+
private List<Member> newNumbers(MemberCreateRequest createRequest) {
59+
return createRequest.getAccounts()
60+
.stream()
61+
.filter(this::notExistsAccount)
62+
.map(account -> newMember(account, createRequest.getPassword())).collect(Collectors.toList());
63+
}
64+
65+
private boolean notExistsAccount(String account) {
66+
if (memberRepository.findByAccountIncludeDeleted(account).isPresent()) {
67+
throw new ResourceConflictException(ResourceType.MEMBER);
68+
}
69+
return true;
70+
}
71+
6172
private Member newMember(String account, String password) {
6273
Member member = new Member();
6374
member.setAccount(account);
@@ -68,73 +79,88 @@ private Member newMember(String account, String password) {
6879

6980
@Transactional(rollbackFor = Exception.class)
7081
public MemberResponse update(MemberUpdateRequest updateRequest) {
71-
UserPasswordAuthenticationToken authentication =
72-
(UserPasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
73-
Member member = memberRepository.findByAccount(updateRequest.getAccount())
74-
.orElseThrow(() -> new ResourceNotFoundException(ResourceType.MEMBER, updateRequest.getAccount()));
82+
verifyAdminPrivileges();
83+
84+
Member member = findMemberByAccount(updateRequest.getAccount());
7585
MemberMapper.INSTANCE.mapEntity(updateRequest, member);
76-
if (authentication.isAdmin()) {
77-
return MemberMapper.INSTANCE.entityToResponse(memberRepository.save(member));
78-
}
79-
return null;
86+
return MemberMapper.INSTANCE.entityToResponse(memberRepository.save(member));
8087
}
8188

8289
@Transactional(rollbackFor = Exception.class)
8390
public MemberResponse modifyPassword(MemberModifyPasswordRequest modifyPasswordRequest) {
84-
UserPasswordAuthenticationToken authentication =
85-
(UserPasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
86-
Member member = memberRepository.findByAccount(authentication.getAccount())
87-
.orElseThrow(() -> new ResourceNotFoundException(ResourceType.MEMBER, authentication.getAccount()));
88-
if (new BCryptPasswordEncoder().matches(modifyPasswordRequest.getOldPassword(), member.getPassword())) {
89-
member.setPassword(new BCryptPasswordEncoder().encode(modifyPasswordRequest.getNewPassword()));
90-
return MemberMapper.INSTANCE.entityToResponse(memberRepository.save(member));
91-
} else {
92-
throw new PasswordErrorException();
91+
Member member = findLoggedInMember();
92+
verifyPassword(modifyPasswordRequest.getOldPassword(), member.getPassword());
93+
94+
member.setPassword(passwordEncoder.encode(modifyPasswordRequest.getNewPassword()));
95+
return MemberMapper.INSTANCE.entityToResponse(memberRepository.save(member));
96+
}
97+
98+
private void verifyPassword(String oldPassword, String newPassword) {
99+
if (!passwordEncoder.matches(oldPassword, newPassword)) {
100+
throw new IllegalArgumentException(MessageKey.INVALID_OLD_PASSWORD);
93101
}
94102
}
95103

96104
@Transactional(rollbackFor = Exception.class)
97105
public void updateVisitedTime(String account) {
98-
Member member = memberRepository.findByAccount(account)
99-
.orElseThrow(() -> new ResourceNotFoundException(ResourceType.MEMBER, account));
106+
Member member = findMemberByAccount(account);
107+
100108
member.setVisitedTime(new Date());
101109
memberRepository.save(member);
102110
}
103111

104112
@Transactional(rollbackFor = Exception.class)
105113
public MemberResponse delete(String account) {
114+
verifyAdminPrivileges();
115+
116+
Member member = findMemberByAccount(account);
117+
member.setDeleted(true);
118+
return MemberMapper.INSTANCE.entityToResponse(memberRepository.save(member));
119+
}
120+
121+
private void verifyAdminPrivileges() {
106122
UserPasswordAuthenticationToken authentication =
107123
(UserPasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
108-
Member member = memberRepository.findByAccount(account).orElseThrow(() ->
109-
new ResourceNotFoundException(ResourceType.MEMBER, account));
110-
member.setDeleted(true);
111-
if (authentication.isAdmin()) {
112-
return MemberMapper.INSTANCE.entityToResponse(memberRepository.save(member));
124+
125+
if (authentication == null || !authentication.isAdmin()) {
126+
throw new ForbiddenException();
113127
}
114-
return null;
115-
}
116-
117-
public Page<MemberResponse> list(MemberSearchRequest searchRequest) {
118-
Pageable pageable = PageRequest.of(searchRequest.getPageIndex(), searchRequest.getPageSize(),
119-
Sort.Direction.DESC, "createdTime");
120-
Specification<Member> spec = new Specification<Member>() {
121-
@Override
122-
public Predicate toPredicate(Root<Member> root, CriteriaQuery<?> query, CriteriaBuilder cb) {
123-
if (StringUtils.isNotBlank(searchRequest.getKeyword())) {
124-
Predicate p0 = cb.like(root.get("account"), "%" + searchRequest.getKeyword() + "%");
125-
query.where(p0);
126-
}
127-
return query.getRestriction();
128-
}
129-
};
130-
Page<Member> members = memberRepository.findAll(spec, pageable);
128+
}
129+
130+
public Page<MemberResponse> query(MemberSearchRequest searchRequest) {
131+
Pageable pageable = PageRequestUtil.toPageable(searchRequest, Sort.Direction.DESC, "createdTime");
132+
Page<Member> members = memberRepository.findAll(accountLike(searchRequest.getKeyword()), pageable);
133+
131134
return members.map(item -> MemberMapper.INSTANCE.entityToResponse(item));
132135
}
133136

134-
public MemberResponse query(String account) {
135-
Member member = memberRepository.findByAccountIncludeDeleted(account)
136-
.orElseThrow(() -> new ResourceNotFoundException(ResourceType.MEMBER, account));
137+
private Specification<Member> accountLike(String account) {
138+
if (StringUtils.isBlank(account)) {
139+
return null;
140+
}
141+
return (root, query, criteriaBuilder) -> criteriaBuilder.like(root.get("account"),
142+
"%" + account + "%");
143+
}
144+
145+
public MemberResponse queryByAccount(String account) {
146+
Member member = findMemberByAccount(account, true);
137147
return MemberMapper.INSTANCE.entityToResponse(member);
138148
}
139149

150+
private Member findLoggedInMember() {
151+
UserPasswordAuthenticationToken authentication =
152+
(UserPasswordAuthenticationToken) SecurityContextHolder.getContext().getAuthentication();
153+
154+
return findMemberByAccount(authentication.getAccount());
155+
}
156+
157+
private Member findMemberByAccount(String account) {
158+
return findMemberByAccount(account, false);
159+
}
160+
161+
private Member findMemberByAccount(String account, boolean includeDeleted) {
162+
Optional<Member> member = includeDeleted ? memberRepository.findByAccountIncludeDeleted(account) :
163+
memberRepository.findByAccount(account);
164+
return member.orElseThrow(() -> new ResourceNotFoundException(ResourceType.MEMBER, account));
165+
}
140166
}

‎src/main/java/com/featureprobe/api/util/PageRequestUtil.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package com.featureprobe.api.util;
2+
3+
import com.featureprobe.api.dto.PaginationRequest;
4+
import org.springframework.data.domain.PageRequest;
5+
import org.springframework.data.domain.Pageable;
6+
import org.springframework.data.domain.Sort;
7+
8+
public class PageRequestUtil {
9+
10+
public static Pageable toPageable(PaginationRequest pageRequest, Sort.Direction direction, String sortBy) {
11+
return PageRequest.of(pageRequest.getPageIndex(), pageRequest.getPageSize(),
12+
direction, sortBy);
13+
}
14+
}

‎src/main/resources/i18n/messages_en_US.properties‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,7 @@ resource.member=Member
55

66
resource.error.conflict=This resource already exists, please modify
77
resource.error.not_found={0} {1} not found
8-
validate.invalid_request=request invalid
8+
resource.error.forbidden=Forbidden
9+
validate.invalid_request=Request invalid
910
resource.error.using=This resource is being used
11+
validate.invalid_old_password=Invalid old password

‎src/main/resources/i18n/messages_zh_CN.properties‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,8 @@ resource.member=成员
55

66
resource.error.conflict=资源已经存在
77
resource.error.not_found={0} {1} 不存在
8+
resource.error.using=资源正在被使用
9+
resource.error.forbidden=操作不合法
10+
811
validate.invalid_request=请求无效
9-
resource.error.using=资源正在被使用
12+
validate.invalid_old_password=旧密码错误

0 commit comments

Comments
 (0)