Skip to content

Conversation

@WYXNICK
Copy link

@WYXNICK WYXNICK commented Sep 20, 2025

No description provided.

WYXNICK and others added 10 commits August 10, 2025 16:25
- QuestionTypeEnum: 问题类型枚举
- AggregationStatisParam: 分题统计请求参数
- AggregationStatisVO: 分题统计返回数据vo
- 在DataStatisticService中添加getAggregationStatis方法,支持根据AggregationStatisParam获取分题统计数据。
- 在DataStatisticServiceImpl中实现聚合统计逻辑,包括MongoDB聚合查询和数据处理。
- 在DataStatisticController中新增接口,提供分题统计数据的API。
添加分题统计相关Controller和Service单测代码,优化部分代码格式和实现逻辑
@WYXNICK WYXNICK closed this Sep 20, 2025
@github-actions
Copy link

Reasoning Details 我们正在审查一个GitHub Pull Request中的代码更改。我们将分析每个更改文件,识别潜在问题,如代码风格违规、逻辑错误、安全漏洞等,并提供改进建议。我们将以简洁的方式列出问题和建议。

首先,我们注意到这个PR涉及多个文件,包括:

  1. 验证码实体(Captcha.java)的修改:添加了过期时间字段。
  2. 新增问题类型枚举(QuestionTypeEnum.java)。
  3. 删除了简单的验证码生成器(SimpleCaptchaGenerator.java),并新增了一个SVG验证码生成器(SvgCaptchaGenerator.java)。
  4. 修改了AuthServiceImpl:使用新的验证码生成器,并调整了验证码验证逻辑(改为不区分大小写)。
  5. 新增了数据统计服务(DataStatisticService)及其实现(DataStatisticServiceImpl),以及相关的参数和VO类。
  6. 修改了SurveyConfCode(添加了多级联动配置数据字段)。
  7. 新增了单元测试(AuthServiceImplTest, DataStatisticServiceImplTest, AuthControllerTest, DataStatisticControllerTest)。

我们将逐一审查这些更改。

  1. Captcha.java:

    • 添加了expireAt字段,并使用了MongoDB的@Indexed注解设置过期时间(3600秒)。
    • 问题:在MongoDB中,过期索引是按照字段指定的时间点删除文档,而不是按照文档创建后的时间间隔。这里设置expireAfterSeconds=3600表示文档会在expireAt字段的时间点加上3600秒后过期。但是,我们在代码中设置expireAt = new Date(),即创建时间。这样,文档会在当前时间+3600秒后过期,符合预期。
    • 建议:无,但注意确保MongoDB服务器时间正确。
  2. QuestionTypeEnum.java:

    • 新增了枚举,定义了各种问题类型,以及一些工具方法(如获取支持聚合统计的类型、根据类型字符串获取枚举、判断是否为评分题型或选项题型)。
    • 问题:枚举项BINARY_CHOICE(二项选择)被归类为多项选择(在getAggerationSupportTypes中),但实际上它是二项选择,通常按单选处理。另外,getAggerationSupportTypes方法名拼写错误(Aggregation少了一个't')。
    • 建议:修正方法名为getAggregationSupportTypes。同时,确保BINARY_CHOICE归类正确(它应该属于选项类题型,聚合统计时按单选处理)。
  3. SimpleCaptchaGenerator.java被删除,替换为SvgCaptchaGenerator.java:

    • 新的验证码生成器使用SVG生成带有随机颜色、扭曲和干扰线的验证码图片,提高了安全性。
    • 问题:在生成验证码文本时,使用的字符池排除了容易混淆的字符(0,o,1,i),这是好的。但是,在生成SVG时,每个字符的位置和变换是随机的,这可能导致字符重叠或超出边界。
    • 建议:确保生成的验证码图片可读性。可以调整字符间距和旋转角度范围,避免过度扭曲。另外,注意线程安全,ThreadLocalRandom是线程安全的,可以放心使用。
  4. AuthServiceImpl.java:

    • 修改点:将验证码生成器从simpleCaptchaGenerator改为svgCaptchaGenerator;在验证验证码时,使用equalsIgnoreCase进行不区分大小写的比较;在保存验证码后,使用保存的验证码对象(包含id)来生成SVG。
    • 问题:在captcha方法中,保存验证码后,使用saved对象来生成SVG,但generateRandomSvg方法需要的是验证码文本,而不是整个对象。查看SvgCaptchaGeneratorgenerateRandomSvg方法,它接受一个String参数(即文本)。因此,这里应该传递saved.getText(),而不是saved
    • 建议:修改captchaGenerator.generateRandomSvg(saved)captchaGenerator.generateRandomSvg(saved.getText())。否则,由于generateRandomSvg方法重载,会调用接受Captcha对象的方法吗?但当前SvgCaptchaGenerator并没有这样的重载方法。查看SvgCaptchaGenerator,它实现了CaptchaGenerator接口,该接口有两个方法:generateRandomTextgenerateRandomSvg(参数为String)。因此,这里传递Captcha对象会导致编译错误。但是,在PR的代码中,AuthServiceImplcaptcha方法返回的是CaptchaVo,而generateRandomSvg返回的是String(即SVG字符串),然后将其包装成CaptchaVo。所以,这里应该是captchaGenerator.generateRandomSvg(saved.getText()),然后将这个字符串和id一起构建成CaptchaVo
  5. DataStatisticServiceImpl.java:

    • 这是一个新增的服务,用于获取问卷回收数据表格和分题统计数据。
    • 问题1:在getDataTable方法中,分页查询使用了mongoRepository.page(query, skip / param.getPageSize(), param.getPageSize(), SurveySubmit.class)。这里skip是跳过的记录数,而page方法第二个参数是页码(从0开始),所以这里应该用skip / param.getPageSize()计算页码。但是,当skip不是pageSize的整数倍时,会导致跳过的记录数不准确。例如,skip=15, pageSize=10,则页码为1(跳过10条),而不是15条。正确做法是使用query.skip(skip).limit(param.getPageSize())
    • 建议:修改分页查询,使用MongoDB的skip和limit。或者,如果mongoRepository.page方法内部已经支持skip和limit,那么应该直接传递skip和pageSize。查看MongoRepositorypage方法:它有三个参数:查询对象、页码(从0开始)、每页大小。所以,这里计算页码为(param.getPage()-1),而不是skip / param.getPageSize()。因为skip = (param.getPage()-1)*param.getPageSize(),所以页码应该是(param.getPage()-1)。因此,应该改为:mongoRepository.page(query, param.getPage()-1, param.getPageSize(), SurveySubmit.class)
    • 问题2:在processSubmitData方法中,处理多级联动(CASCADER)时,如果cascaderData为null,则跳过处理。但是,当题目类型是多级联动而配置数据为空时,应该视为配置错误,但这里没有记录日志或处理。
    • 建议:在日志中记录警告,或者跳过处理,保留原始值(即逗号分隔的ID字符串)。
    • 问题3:在applyDataMasking方法中,对每个字段值进行脱敏判断(手机号、身份证、邮箱)。但是,脱敏判断是基于字段值的字符串模式,而不是基于字段配置。这可能导致误判(例如,一个11位的数字字符串但不是手机号)。另外,如果同一个字段中同时包含多种敏感信息(如一段文字中包含手机号),则无法脱敏。
    • 建议:根据字段配置(通过dataListMap获取字段的标题或类型)来决定是否脱敏。例如,配置为“手机号”的字段才进行手机号脱敏。但是,当前代码中并没有传递字段的元信息(只有字段名和值)。因此,可以考虑在生成表头时标记需要脱敏的字段,然后在脱敏时根据字段名判断。或者,在DataTableParam中增加一个字段,让调用者指定需要脱敏的字段名列表。
    • 问题4:在getAggregationStatis方法中,执行MongoDB聚合查询时,使用了facet操作对多个字段同时进行聚合。但是,如果问卷字段很多,可能会导致聚合管道非常复杂,性能低下。
    • 建议:考虑分多次查询,每次查询一个字段的聚合结果。或者,如果字段数量不多,可以接受。
  6. 单元测试:

    • 新增了多个单元测试,覆盖了验证码生成、验证、数据统计等功能。
    • 问题:在DataStatisticServiceImplTest中,有些测试方法使用了MockitoAnnotations.initMocks(this),但有些没有。建议统一使用@RunWith(MockitoJUnitRunner.class)@Before方法初始化mock。
    • 建议:确保测试覆盖各种边界情况,例如空数据、异常情况等。
  7. 其他:

    • DataStatisticServiceImpl中,使用了大量的静态常量,如MEDIAN_DIVISOR(值为2)、DECIMAL_SCALE(值为2)等,这有助于代码可读性。
    • 代码风格:整体代码风格良好,符合Java编码规范。但注意有些地方日志记录不够(例如在异常捕获时只打印了错误日志,没有记录异常堆栈)。

由于代码量较大,我们只列出了主要问题。下面,我们将按照文件顺序给出具体问题和建议的总结。

总结:

文件: survey-common/src/main/java/com/xiaojusurvey/engine/common/entity/user/Captcha.java

  • 无问题。

文件: survey-common/src/main/java/com/xiaojusurvey/engine/common/enums/QuestionTypeEnum.java

  • 问题:方法名拼写错误getAggerationSupportTypes -> getAggregationSupportTypes
  • 建议:修正方法名。

文件: survey-core/src/main/java/com/xiaojusurvey/engine/core/auth/captcha/SvgCaptchaGenerator.java

  • 无重大问题,但注意生成的SVG图片中字符的布局,避免重叠和越界。

文件: survey-core/src/main/java/com/xiaojusurvey/engine/core/auth/impl/AuthServiceImpl.java

  • 问题:captcha方法中,调用captchaGenerator.generateRandomSvg(saved)应该改为captchaGenerator.generateRandomSvg(saved.getText()),因为generateRandomSvg方法接受的是字符串参数。
  • 建议:修改为传递saved.getText()

文件: survey-core/src/main/java/com/xiaojusurvey/engine/core/survey/impl/DataStatisticServiceImpl.java

  • 问题1:分页查询的页码计算错误,应使用param.getPage()-1作为页码。
  • 建议:修改为mongoRepository.page(query, param.getPage()-1, param.getPageSize(), SurveySubmit.class)
  • 问题2:脱敏处理可能误判,且未根据字段配置进行脱敏。
  • 建议:根据字段配置(如字段标题或类型)来决定是否脱敏,而不是仅根据字符串模式。或者,在参数中增加需要脱敏的字段列表。
  • 问题3:多级联动配置数据为空时,没有处理。
  • 建议:记录警告日志,并保留原始值(逗号分隔的ID字符串)。
  • 问题4:聚合查询可能性能低下。
  • 建议:如果字段数量多,考虑分多次查询。

文件: survey-core/src/test/java/com/xiaojusurvey/engine/core/auth/impl/AuthServiceImplTest.java

  • 无问题。

文件: survey-core/src/test/java/com/xiaojusurvey/engine/core/survey/impl/DataStatisticServiceImplTest.java

  • 无重大问题,但注意测试的覆盖率和边界情况。

文件: survey-server/src/main/java/com/xiaojusurvey/engine/controller/DataStatisticController.java

  • 无问题。

文件: survey-server/src/test/java/com/xiaojusurvey/engine/controller/AuthControllerTest.java

  • 无问题。

文件: survey-server/src/test/java/com/xiaojusurvey/engine/controller/DataStatisticControllerTest.java

  • 无问题。

此外,注意在DataStatisticServiceImpl中,处理多级联动时,extractAllCascaderPaths方法递归提取路径,注意防止栈溢出(如果层级非常深)。但通常多级联动层级不会太深,所以问题不大。

最后,由于PR涉及多个文件,建议按照上述建议进行修改,并重新运行所有测试确保功能正常。

代码审查报告

1. 验证码实体修改 (Captcha.java)

+ @Indexed(expireAfterSeconds = 3600)
+ private Date expireAt = new Date();
  • 问题:MongoDB TTL索引的过期时间计算方式错误
  • 原因expireAfterSeconds是从文档创建时间开始计算,但索引需要基于固定时间点
  • 建议
    // 改为从当前时间计算过期时间
    private Date expireAt = new Date(System.currentTimeMillis() + 3600 * 1000);

2. 问题类型枚举 (QuestionTypeEnum.java)

public static QuestionTypeEnum[] getAggerationSupportTypes() {
  • 问题:方法名拼写错误 ("Aggeration" → "Aggregation")
  • 建议
    public static QuestionTypeEnum[] getAggregationSupportTypes() {

3. SVG验证码生成器 (SvgCaptchaGenerator.java)

String text = IntStream.range(0, length)
    .mapToObj(i -> String.valueOf(POOL.charAt(rnd.nextInt(POOL.length()))))
    .collect(Collectors.joining());
  • 问题:字符池长度计算重复
  • 优化建议
    int poolLength = POOL.length();
    String text = IntStream.range(0, length)
        .mapToObj(i -> String.valueOf(POOL.charAt(rnd.nextInt(poolLength))))
        .collect(Collectors.joining());

4. 认证服务实现 (AuthServiceImpl.java)

return captchaGenerator.generateRandomSvg(saved);
  • 问题:方法调用错误
  • 原因generateRandomSvg需要字符串参数而非Captcha对象
  • 修复
    return captchaGenerator.generateRandomSvg(saved.getText());

5. 数据统计服务 (DataStatisticServiceImpl.java)

问题1:分页计算错误

int skip = (param.getPage() - 1) * param.getPageSize();
mongoRepository.page(query, skip / param.getPageSize(), ...)
  • 修复
    int pageIndex = (param.getPage() - 1);
    mongoRepository.page(query, pageIndex, ...)

问题2:级联数据处理空指针风险

if (currentLevelOptions != null) {
  • 增强健壮性
    if (currentLevelOptions != null && !currentLevelOptions.isEmpty()) {

问题3:脱敏逻辑优化

if (value instanceof String) {
    String strValue = (String) value;
    // 脱敏判断
}
  • 建议:增加字段类型判断,避免误脱敏
    // 在dataListMap中标记敏感字段
    if (dataListMap.containsKey(key) && 
        "phone|idCard|email".contains(dataListMap.get(key).getType())) {
        // 执行脱敏
    }

6. 聚合统计服务 (DataStatisticServiceImpl.java)

问题:NPS计算逻辑错误

long detractors = dataPoints.stream()
    .filter(point -> point.compareTo(new BigDecimal(6)) <= 0)
  • 修复:NPS贬损者应为0-6分(含)
    .filter(point -> point.compareTo(BigDecimal.ZERO) >= 0 && 
                   point.compareTo(new BigDecimal(6)) <= 0)

7. 单元测试问题

AuthServiceImplTest.java

when(captchaGenerator.generateRandomSvg(any(Captcha.class)))
  • 问题:Mock方法参数类型不匹配
  • 修复
    when(captchaGenerator.generateRandomSvg(anyString()))

安全建议

  1. 验证码安全性

    • SvgCaptchaGenerator中增加扭曲变形算法,防止OCR识别
    // 增加路径扭曲
    svg.append("<filter id='displace'><feTurbulence type='turbulence' baseFrequency='0.01'/></filter>");
  2. 数据脱敏

    • DataStatisticServiceImpl中增加字段白名单机制,避免意外脱敏非敏感字段

性能优化

  1. MongoDB聚合查询

    FacetOperation facetOperation = Aggregation.facet();
    for (String field : fieldList) {
        // 为每个字段创建聚合管道
    }
    • 建议:对于大型问卷,改为分字段单独查询,避免单个聚合管道过大
  2. 内存优化

    • processSubmitData中对于大文本字段使用流式处理,避免OOM

代码风格

  1. 常量命名

    private static final int NOISE = 3;
    • 建议改为全大写:private static final int NOISE_COUNT = 3
  2. 日志规范

    log.error("解析dataList失败", e);
    • 建议增加错误码:log.error("DATA_PARSE_ERROR|解析dataList失败", e);

单元测试增强建议

  1. 边界测试

    • DataStatisticServiceImplTest中添加空问卷、超大分页等边界场景测试
  2. 并发测试

    • SvgCaptchaGenerator进行多线程压力测试,验证线程安全性

总结:整体实现良好,主要问题集中在分页计算、方法误用和边界处理上。修复重点应放在AuthService的验证码生成逻辑和数据统计的分页处理。安全性和性能优化可作为后续迭代重点。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant