From e2b76ee0e5f4d9f5edc57c5f22d9dc736daf1fd0 Mon Sep 17 00:00:00 2001 From: YunaiV Date: Sun, 21 Nov 2021 12:05:34 +0800 Subject: [PATCH] =?UTF-8?q?code=20review=20=E4=BF=AE=E6=94=B9=E5=AF=86?= =?UTF-8?q?=E7=A0=81=E7=AD=89=E7=9A=84=E5=8D=95=E5=85=83=E6=B5=8B=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- yudao-admin-server/pom.xml | 2 -- yudao-user-server/pom.xml | 1 + .../controller/user/SysUserProfileController.java | 1 + .../user/vo/MbrUserUpdateMobileReqVO.java | 2 +- .../service/user/impl/MbrUserServiceImpl.java | 2 ++ .../system/controller/auth/SysAuthController.java | 1 + .../auth/vo/MbrAuthResetPasswordReqVO.java | 2 +- .../service/auth/impl/SysAuthServiceImpl.java | 3 +++ .../service/sms/impl/SysSmsCodeServiceImpl.java | 4 ++++ .../controller/SysUserProfileControllerTest.java | 5 +++-- .../member/service/MbrUserServiceImplTest.java | 8 ++++---- .../modules/system/service/SysAuthServiceTest.java | 14 ++++---------- 12 files changed, 25 insertions(+), 20 deletions(-) diff --git a/yudao-admin-server/pom.xml b/yudao-admin-server/pom.xml index 5cdf85712..4314e0fed 100644 --- a/yudao-admin-server/pom.xml +++ b/yudao-admin-server/pom.xml @@ -117,8 +117,6 @@ yudao-spring-boot-starter-excel - - org.apache.velocity velocity-engine-core diff --git a/yudao-user-server/pom.xml b/yudao-user-server/pom.xml index b4c2cf8a5..03dbfd699 100644 --- a/yudao-user-server/pom.xml +++ b/yudao-user-server/pom.xml @@ -91,6 +91,7 @@ test + junit diff --git a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/controller/user/SysUserProfileController.java b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/controller/user/SysUserProfileController.java index 39c3d88f3..b2afbc78b 100644 --- a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/controller/user/SysUserProfileController.java +++ b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/controller/user/SysUserProfileController.java @@ -70,6 +70,7 @@ public class SysUserProfileController { @PreAuthenticated public CommonResult updateMobile(@RequestBody @Valid MbrUserUpdateMobileReqVO reqVO) { // 校验验证码 + // TODO @宋天：统一到 userService.updateMobile 方法里 smsCodeService.useSmsCode(reqVO.getMobile(),SysSmsSceneEnum.CHANGE_MOBILE_BY_SMS.getScene(), reqVO.getCode(),getClientIP()); userService.updateMobile(getLoginUserId(), reqVO); diff --git a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/controller/user/vo/MbrUserUpdateMobileReqVO.java b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/controller/user/vo/MbrUserUpdateMobileReqVO.java index 0e5499bc7..df1980b89 100644 --- a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/controller/user/vo/MbrUserUpdateMobileReqVO.java +++ b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/controller/user/vo/MbrUserUpdateMobileReqVO.java @@ -25,9 +25,9 @@ public class MbrUserUpdateMobileReqVO { @Pattern(regexp = "^[0-9]+$", message = "手机验证码必须都是数字") private String code; - @ApiModelProperty(value = "手机号",required = true,example = "15823654487") @NotBlank(message = "手机号不能为空") + // TODO @宋天：手机校验，直接使用 @Mobile 哈 @Length(min = 8, max = 11, message = "手机号码长度为 8-11 位") @Pattern(regexp = "^1[3-9]\\d{9}$", message = "手机号格式错误") private String mobile; diff --git a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/service/user/impl/MbrUserServiceImpl.java b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/service/user/impl/MbrUserServiceImpl.java index 76b4501eb..f45ad257b 100644 --- a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/service/user/impl/MbrUserServiceImpl.java +++ b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/member/service/user/impl/MbrUserServiceImpl.java @@ -126,8 +126,10 @@ public class MbrUserServiceImpl implements MbrUserService { // 检测用户是否存在 MbrUserDO userDO = checkUserExists(userId); // 检测手机与验证码是否匹配 + // TODO @宋天：修改手机的时候。应该要校验，老手机 + 老手机 code；新手机 + 新手机 code sysAuthService.checkIfMobileMatchCodeAndDeleteCode(userDO.getMobile(),reqVO.getCode()); // 更新用户手机 + // TODO @宋天：更新的时候，单独创建对象。直接全量更新，会可能导致属性覆盖。可以看看打印出来的 SQL 哈 userDO.setMobile(reqVO.getMobile()); userMapper.updateById(userDO); } diff --git a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/controller/auth/SysAuthController.java b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/controller/auth/SysAuthController.java index eb6e142ee..4dfdb7a8c 100644 --- a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/controller/auth/SysAuthController.java +++ b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/controller/auth/SysAuthController.java @@ -98,6 +98,7 @@ public class SysAuthController { @ApiOperation(value = "校验验证码是否正确") @PreAuthenticated public CommonResult checkSmsCode(@RequestBody @Valid SysAuthSmsLoginReqVO reqVO) { + // TODO @宋天：check 的时候，不应该使用 useSmsCode 哈，这样验证码就直接被使用了。另外，check 开头的方法，更多是校验的逻辑，不会有 update 数据的动作。这点，在方法命名上，也是要注意的 smsCodeService.useSmsCode(reqVO.getMobile(),SysSmsSceneEnum.CHECK_CODE_BY_SMS.getScene(),reqVO.getCode(),getClientIP()); return success(true); } diff --git a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/controller/auth/vo/MbrAuthResetPasswordReqVO.java b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/controller/auth/vo/MbrAuthResetPasswordReqVO.java index 92fd8445f..3b3556fdb 100644 --- a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/controller/auth/vo/MbrAuthResetPasswordReqVO.java +++ b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/controller/auth/vo/MbrAuthResetPasswordReqVO.java @@ -8,7 +8,6 @@ import lombok.Data; import lombok.NoArgsConstructor; import org.hibernate.validator.constraints.Length; -import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotEmpty; import javax.validation.constraints.Pattern; @@ -29,4 +28,5 @@ public class MbrAuthResetPasswordReqVO { @Length(min = 4, max = 6, message = "手机验证码长度为 4-6 位") @Pattern(regexp = "^[0-9]+$", message = "手机验证码必须都是数字") private String code; + } diff --git a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/service/auth/impl/SysAuthServiceImpl.java b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/service/auth/impl/SysAuthServiceImpl.java index eb05f7086..f4193b35b 100644 --- a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/service/auth/impl/SysAuthServiceImpl.java +++ b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/service/auth/impl/SysAuthServiceImpl.java @@ -285,6 +285,7 @@ public class SysAuthServiceImpl implements SysAuthService { MbrUserDO userDO = checkOldPassword(userId, reqVO.getOldPassword()); // 更新用户密码 + // TODO @宋天：不要更新整个对象哈 userDO.setPassword(passwordEncoder.encode(reqVO.getPassword())); userMapper.updateById(userDO); } @@ -300,6 +301,8 @@ public class SysAuthServiceImpl implements SysAuthService { // TODO @芋艿 这一步没必要检验验证码与手机是否匹配，因为是根据验证码去redis中查找手机号，然后根据手机号查询用户 // 也就是说 即便黑客以其他方式将验证码发送到自己手机上，最终还是会根据手机号查询用户然后进行重置密码的操作，不存在安全问题 + // TODO @宋天：这块微信在讨论下哈~~~ + // 校验验证码 smsCodeService.useSmsCode(userDO.getMobile(), SysSmsSceneEnum.FORGET_MOBILE_BY_SMS.getScene(), reqVO.getCode(),getClientIP()); diff --git a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/service/sms/impl/SysSmsCodeServiceImpl.java b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/service/sms/impl/SysSmsCodeServiceImpl.java index d9a3d68f1..a5796c6fc 100644 --- a/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/service/sms/impl/SysSmsCodeServiceImpl.java +++ b/yudao-user-server/src/main/java/cn/iocoder/yudao/userserver/modules/system/service/sms/impl/SysSmsCodeServiceImpl.java @@ -58,10 +58,14 @@ public class SysSmsCodeServiceImpl implements SysSmsCodeService { // 创建验证码 String code = this.createSmsCode(mobile, scene, createIp); // 发送验证码 + // TODO @宋天：这里可以拓展下 SysSmsSceneEnum，支持设置对应的短信模板编号（不同场景的短信文案是不同的)、是否要校验手机号已经注册。这样 Controller 就可以收口成一个接口了。相当于说，不同场景，不同策略 smsCoreService.sendSingleSmsToMember(mobile, null, SysSmsTemplateCodeConstants.USER_SMS_LOGIN, MapUtil.of("code", code)); // 存储手机号与验证码到redis，用于标记 + // TODO @宋天：SysSmsCodeDO 表应该足够，无需增加额外的 redis 存储哇 + // TODO @宋天：Redis 相关的操作，不要散落到业务层，而是写一个它对应的 RedisDAO。这样，实现业务与技术的解耦 + // TODO @宋天：直接使用 code 作为 key，会存在 2 个问题：1）code 可能会冲突，多个手机号之间；2）缺少前缀。例如说 sms_code_${code} stringRedisTemplate.opsForValue().set(code,mobile,CODE_TIME, TimeUnit.MINUTES); } diff --git a/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/member/controller/SysUserProfileControllerTest.java b/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/member/controller/SysUserProfileControllerTest.java index d8b8e7f71..3f32cc82c 100644 --- a/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/member/controller/SysUserProfileControllerTest.java +++ b/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/member/controller/SysUserProfileControllerTest.java @@ -22,6 +22,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. * * @author 宋天 */ +// TODO @宋天：controller 的单测可以不写哈，因为收益太低了。未来我们做 qa 自动化测试 public class SysUserProfileControllerTest { private MockMvc mockMvc; @@ -35,7 +36,7 @@ public class SysUserProfileControllerTest { @Mock private SysSmsCodeService smsCodeService; - @Before + @Before // TODO @宋天：使用 junit5 哈 public void setup() { // 初始化 MockitoAnnotations.openMocks(this); @@ -52,7 +53,7 @@ public class SysUserProfileControllerTest { .content("{\"mobile\":\"15819844280\",\"code\":\"123456\"}}")) .andExpect(status().isOk()) .andDo(MockMvcResultHandlers.print()); - +// TODO @宋天：方法的结尾，不用空行哈 } diff --git a/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/member/service/MbrUserServiceImplTest.java b/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/member/service/MbrUserServiceImplTest.java index 0ed1e77e6..05571ba2e 100644 --- a/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/member/service/MbrUserServiceImplTest.java +++ b/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/member/service/MbrUserServiceImplTest.java @@ -6,7 +6,6 @@ import cn.iocoder.yudao.framework.common.enums.CommonStatusEnum; import cn.iocoder.yudao.framework.common.util.collection.ArrayUtils; import cn.iocoder.yudao.framework.redis.config.YudaoRedisAutoConfiguration; import cn.iocoder.yudao.userserver.BaseDbAndRedisUnitTest; -import cn.iocoder.yudao.userserver.BaseDbUnitTest; import cn.iocoder.yudao.userserver.modules.member.controller.user.vo.MbrUserInfoRespVO; import cn.iocoder.yudao.userserver.modules.member.controller.user.vo.MbrUserUpdateMobileReqVO; import cn.iocoder.yudao.userserver.modules.member.dal.mysql.user.MbrUserMapper; @@ -15,7 +14,6 @@ import cn.iocoder.yudao.userserver.modules.system.controller.auth.vo.SysAuthSend import cn.iocoder.yudao.userserver.modules.system.enums.sms.SysSmsSceneEnum; import cn.iocoder.yudao.userserver.modules.system.service.auth.impl.SysAuthServiceImpl; import cn.iocoder.yudao.userserver.modules.system.service.sms.SysSmsCodeService; -import cn.iocoder.yudao.userserver.modules.system.service.sms.impl.SysSmsCodeServiceImpl; import org.junit.jupiter.api.Test; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.Import; @@ -23,14 +21,16 @@ import org.springframework.data.redis.core.StringRedisTemplate; import org.springframework.security.crypto.password.PasswordEncoder; import javax.annotation.Resource; -import java.io.*; +import java.io.ByteArrayInputStream; import java.util.function.Consumer; import static cn.hutool.core.util.RandomUtil.*; -import static org.junit.jupiter.api.Assertions.assertEquals; import static cn.iocoder.yudao.framework.test.core.util.RandomUtils.randomPojo; import static cn.iocoder.yudao.framework.test.core.util.RandomUtils.randomString; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.*; + +// TODO @芋艿：单测的 review，等逻辑都达成一致后 /** * {@link MbrUserServiceImpl} 的单元测试类 * diff --git a/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/system/service/SysAuthServiceTest.java b/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/system/service/SysAuthServiceTest.java index 65fe3031c..c96425bda 100644 --- a/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/system/service/SysAuthServiceTest.java +++ b/yudao-user-server/src/test/java/cn/iocoder/yudao/userserver/modules/system/service/SysAuthServiceTest.java @@ -1,8 +1,6 @@ package cn.iocoder.yudao.userserver.modules.system.service; -import cn.hutool.core.util.ArrayUtil; import cn.iocoder.yudao.coreservice.modules.member.dal.dataobject.user.MbrUserDO; -import cn.iocoder.yudao.coreservice.modules.system.dal.dataobject.user.SysUserDO; import cn.iocoder.yudao.coreservice.modules.system.service.auth.SysUserSessionCoreService; import cn.iocoder.yudao.coreservice.modules.system.service.logger.SysLoginLogCoreService; import cn.iocoder.yudao.coreservice.modules.system.service.social.SysSocialService; @@ -10,11 +8,8 @@ import cn.iocoder.yudao.framework.common.enums.CommonStatusEnum; import cn.iocoder.yudao.framework.common.util.collection.ArrayUtils; import cn.iocoder.yudao.framework.redis.config.YudaoRedisAutoConfiguration; import cn.iocoder.yudao.userserver.BaseDbAndRedisUnitTest; -import cn.iocoder.yudao.userserver.BaseDbUnitTest; -import cn.iocoder.yudao.userserver.config.RedisTestConfiguration; import cn.iocoder.yudao.userserver.modules.member.dal.mysql.user.MbrUserMapper; import cn.iocoder.yudao.userserver.modules.member.service.user.MbrUserService; -import cn.iocoder.yudao.userserver.modules.member.service.user.impl.MbrUserServiceImpl; import cn.iocoder.yudao.userserver.modules.system.controller.auth.vo.MbrAuthResetPasswordReqVO; import cn.iocoder.yudao.userserver.modules.system.controller.auth.vo.MbrAuthUpdatePasswordReqVO; import cn.iocoder.yudao.userserver.modules.system.service.auth.SysAuthService; @@ -28,17 +23,17 @@ import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.crypto.password.PasswordEncoder; import javax.annotation.Resource; - import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import static cn.hutool.core.util.RandomUtil.randomEle; import static cn.hutool.core.util.RandomUtil.randomNumbers; -import static cn.iocoder.yudao.framework.test.core.util.RandomUtils.*; +import static cn.iocoder.yudao.framework.test.core.util.RandomUtils.randomPojo; +import static cn.iocoder.yudao.framework.test.core.util.RandomUtils.randomString; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.Mockito.*; - +import static org.mockito.Mockito.when; +// TODO @芋艿：单测的 review，等逻辑都达成一致后 /** * {@link SysAuthService} 的单元测试类 * @@ -68,7 +63,6 @@ public class SysAuthServiceTest extends BaseDbAndRedisUnitTest { @Resource private SysAuthServiceImpl authService; - @Test public void testUpdatePassword_success(){ // 准备参数