去哪儿网Code Review大赛
去哪儿网Code Review大赛
序号12
1、tagger 系统会协助运营人员圈定一批用户进行营销活动的投放，量级在百万到千万量级。
2、推送平台需要将这些用户信息从数据库中读取出来做一些预处理。UserService 就是来做这个读取并预处理工作的。
public class UserService {
@Autowired
private UserDAO userDao;
public void preproccessUser(String deviceTableName) {
int totalCount = 10000000;
int pageSize = 1000;
int pageCount = totalCount / pageSize;
for (int pageIndex = 0; pageIndex < pageCount; pageIndex++) {
List<Object> objects = userDao.selectByPage(deviceTableName, pageIndex * pageSize, pageSize);
this.preproccess(objects);
}
}
public void preproccess(List<Object> objectList) {
for (Object obj : objectList) {
}
}
}
@DAO
interface UserDAO {
@SQL("select * from :1 limit :2 , :3")
List<Object> selectByPage(String tableName, int from, int size);
}
|
官方采分点：
1（1分）对objectList 进行判空处理
2（1分）分页改主键游标分页
3（1分）28行 SQL注入风险
|
我的采分点：
- 在注释规范上：风格十分要统一，比如9行～11行是放在代码的后面。19行的注释放在代码的马上。个人更喜欢上面的风格）
- 单词拼写错误，process。命名比注释更加重要。逃）
- 分页性能问题，采用游标的方式，记录每次返回的max(id)，然后带着id去查询
- 空指针问题：objectList判空处理
- 数据量比较大，建议采用多线程处理，不然有可能出现活动结束了，程序都没推送完
- UserDAO采用参数传入到执行sql中，这时候就要小心SQL注入
序号16
集合迭代中修改的常见错误点
本实例为搜索用户筛选品牌信息场景，通过运营配置相关品牌映射关系实现召回相关的品牌商
package com.noah.starter.demo.web.code;
public class KeywordMapBrandTransferAnalyzer extends Analyzer {
private String configKey;
@Override
boolean doAccept(ProcContext context) {
Set<SearchCategory> categorySet = context.getParamPgCate();
if (CollectionUtils.isEmpty(categorySet)) {
return true;
}
Map<Integer, Set<Integer>> transferMapping = ConfigCenterValues.getBrandTransferMapping(configKey);
for (SearchCategory category : categorySet) {
Set<Integer> brands = brandMap.get(category.getBrandId());
if (category.getBrandId() != 0 && CollectionUtils.isNotEmpty(brands)) {
for (Integer brand : brands) {
if (brand != category.getBrandId()) {
SearchCategory mapCategory = new SearchCategory(category.getCateId(), brand, category.getModelId());
categorySet.add(mapCategory);
}
}
}
}
return true;
}
}
class SearchCategory {
Integer cateId;
Integer brandId;
Integer modelId;
public SearchCategory(Integer cateId, Integer brandId, Integer modelId) {
this.cateId = cateId;
this.brandId = brandId;
this.modelId = modelId;
}
public Integer getCateId() {
return cateId;
}
public void setCateId(Integer cateId) {
this.cateId = cateId;
}
public Integer getBrandId() {
return brandId;
}
public void setBrandId(Integer brandId) {
this.brandId = brandId;
}
public Integer getModelId() {
return modelId;
}
public void setModelId(Integer modelId) {
this.modelId = modelId;
}
}
|
官方采分点：
1、 categorySet存在有概率出现迭代时修改问题
2、 SearchCategory没有没有重写hashCode和equals无法判重
|
我的采分点：
- SearchCategory的属性没有被private修饰，不符合Java对象封装的思想
- 第二十行的if判断，两边都是Integer对象，应该使用
equals()方法来比较是否相等。而不是使用==，因为在-128～127范围的数字使用==和不在这个范围的数字是不等的
- SearchCategory 没有重写 hashcode 和 equals 方法
- categorySet存在有概率出现迭代时修改问题，在for循环当中新增/修改/移除元素
序号17
不可变集合
促销活动查询用户库存
public List<PlatformUserStock> queryAllPlatformUserStock(long userId) {
final List<PlatformUserStock> platformUserDayStocks = this.platformUserStockService
.queryUserStock(userId, StoreLimitTypeEnum.EVERY_DAY.getCode());
final List<PlatformUserStock> platformUserAllStocks = this.platformUserStockService
.queryUserStock(userId, StoreLimitTypeEnum.TOTAL_LIMIT.getCode());
platformUserDayStocks.addAll(platformUserAllStocks);
return platformUserDayStocks;
}
public List<PlatformUserStock> queryUserStock(Long userId, int type) {
if (userId == null) {
return Collections.emptyList();
}
return platformUserTotalStockMapper.queryUserStock(userId, type);
}
|
官方采分点：
1、 Collections.emptyList() 是不可变list，在后面的使用中，调用add方法是不支持的，会出现异常。
|
我的采分点：
- 为什么调用
queryUserStock方法要采用：注入自己的方式来调用，this.platformUserStockService，应该要删除
- queryAllPlatformUserStock的入参是基本类型：long，而queryUserStock是封装类型：Long，所以第11行的代码返回结果永远为true，入参类型应该要保持一致
- 注意点：第10行的代码永远不会返回空指针，因为mybatis会帮我们封装空集合
- Collections.emptyList() 是不可变list，在后面的使用中，调用add方法是不支持的，会出现异常。
序号21
redis原子操作
机票报价代理商政策批量导入时，会给代理商加锁，只允许一个任务对数据库进行写操作。
public static boolean lockImport(String key, String value, int expire) {
String redisKey = makeKey(key);
try {
if (redisClient.setnx(redisKey, value) >= SETNX_SUCCESS) {
redisClient.expire(redisKey, expire);
return true;
}
} catch (Exception e) {
QMonitor.recordOne("redis_add_error");
}
return false;
}
|
官方得分点：
1、第四行和第五行，加锁和设置过期时间应该要原子性操作
2、catch异常，直接吞了异常，没有打印异常堆栈信息
|
我的得分点：
同上～
序号22
Stream不可变性和TreeSet的排序
在机票首页有一个乘机人填写框，通过填写框可以添加本次搜索的乘机人。在乘机人填写框上 有个推荐逻辑，推荐我的联系人列表里的最近有过生单记录的乘机人 2 人，乘机人选取顺序是生单时间最近的 2 人。
代码上下文：本段代码是过滤掉乘机人证件类型不是身份证的乘机人。
private void certsFilter(OmPassengersResponse omPassengersResponse) {
List<OmPassengersResponse.Passenger> passengers = omPassengersResponse.getPassengers();
passengers = omPassengersResponse.getPassengers().stream()
.sorted((a, b) -> a.getOrderTime().compareTo(b.getOrderTime())).collect(Collectors.toList());
passengers = omPassengersResponse.getPassengers().stream()
.filter(passenger -> StringUtils.isNotBlank(passenger.getPhoneNum())).collect(Collectors.toList());
passengers = passengers.stream().filter(passenger -> CollectionUtils.isNotEmpty(passenger.getCerts()))
.collect(Collectors.collectingAndThen(Collectors.toCollection(() -> new TreeSet<>(Comparator.comparing(compare()))), ArrayList::new));
if (passengers.size() > configVal.getNum()) {
passengers = passengers.subList(0, configVal.getNum());
}
omPassengersResponse.setPassengers(passengers);
}
private Function<OmPassengersResponse.Passenger, String> compare() {
return p -> p.getName() + p.getCerts().get(0).getNumber();
}
|
官方得到分点：
1、本题目考查的是过滤、排序的操作，应该先操作过滤再排序。性能会更好。
2、使用TreeSet<>(Comparator.comparing(compare())))来排序，不符合题目的要求：最近有过生单记录的乘机人 2 人
|
序号23
编码规范
规则校验 充血模式开发，在 entity 中增加一些必要的逻辑代码，例如重写 get 方法等
代码上下文：规则校验简易代码，RuleParamEntity 为规则校验的入参，runQlExpress 为规则执行代码。
class RuleParamEntity implements Serializable {
private static final long serialVersionUID = -7816951117938588941L;
private String ruleNo;
private String userName;
private String qunarMobile;
public String getQunarMobile() {
if (StringUtils.isNotBlank(qunarMobile)) {
return qunarMobile;
}
if (StringUtils.isNotBlank(userName) && StringUtils.isBlank(qunarMobile)) {
UserDetail userDetail = UserClient.queryEncryptUserByName(userName);
qunarMobile = userDetail.getMobile();
}
return qunarMobile;
}
public boolean runQlExpress(ExpressRunner runner, RuleParamEntity request, TbfRuleConfEntity entity) {
try {
QMonitor.recordOne("RuleComponent_ruleChecking_" + entity.getKeyword());
DefaultContext<String, Object> context = new DefaultContext<String, Object>();
Object curInfo = ParseBeanUtils.invokeMethod(request, entity.getKeyword());
if (curInfo == null) {
log.info("runQlExpress get curInfo blank ! request : {} entity : {}", JSONObject.toJSONString(request), JSONObject.toJSONString(entity));
QMonitor.recordOne("RuleComponent_ruleChecking_" + entity.getKeyword() + "_NOT_VERIFIED");
entity.setVerificationResult(VerificationResultsEnum.NOT_VERIFIED.getCode());
return VerificationResultsEnum.NOT_VERIFIED;
}
Object res = runner.execute(entity.getQLExpress(ParseBeanUtils.fieldIsString(request, entity.getKeyword())), context, null, true, false);
if (!((Boolean) res).booleanValue()) {
entity.setVerificationResult(VerificationResultsEnum.DENIED.getCode());
QMonitor.recordOne("RuleComponent_ruleChecking_" + entity.getKeyword() + "_DENIED");
return VerificationResultsEnum.DENIED;
}
entity.setVerificationResult(VerificationResultsEnum.PASSED.getCode());
QMonitor.recordOne("RuleComponent_ruleChecking_" + entity.getKeyword() + "_PASSED");
return VerificationResultsEnum.PASSED;
} catch (Exception e) {
log.error("ruleChecking error request : {} entity : {}", JSONObject.toJSONString(request), JSONObject.toJSONString(entity), e);
QMonitor.recordOne("RuleComponent_runQlExpress_Exception");
}
entity.setVerificationResult(VerificationResultsEnum.DENIED.getCode());
return VerificationResultsEnum.DENIED;
}
}
|
官方得分：
1、49行JSONObject.toJSONString(entity)，使用fastJson库会调用entity的get方法，但是我们业务中使用的充血模型。重写getQunarMobile()方法，建议方法名修改为queryXXX()
2、12行代码userDetail，使用该对象的时候，需要判空
|
序号24
接口安全
更新当前用户的基本信息和地址信息，该操作必须要求用户是登录态。
@RestController
public class DemoController {
private final UserService userService;
public DemoController(UserService userService) {
this.userService = userService;
}
@PostMapping("/update")
public Object updateUserInfo(@Valid @RequestBody UserUpdateCommand command, HttpServletRequest request) {
String openId = getCurrentUser(request).getOpenid();
if (StringUtils.isEmpty(openId)) {
return ApiResult.forbidden("用户未登录，不允许进行操作");
}
List<Address> addresses = Lists.transform(command.getAddresses(), (Function<AddressDTO, Address>) input -> DTOMapper.INSTANCE.toDTO(input));
User user = userService.queryUser(command.getOpenId());
for (Address address : addresses) {
address.setOpenId(openId);
}
DTOMapper.INSTANCE.copyToEntity(command, user);
user.setAddresses(addresses);
userService.save(user);
return ApiResult.success();
}
}
|
官方得分点：
1、11行获取到的openId和传参数的command的openId有可能不同，需要加上校验逻辑
|
我的得分点：
同上～
序号14
redis 分布式锁实现
基础数据与携程比价后调用中转点庖丁接口，发现有中转报价缺失时会发 QMQ 消息，当缺失 原因为【政策包报价不包含当前中转点过滤】时，需要记录当前航线中转点，并按时间先后补 齐缺失中转点城市至最大数量
代码上下文:
1、data 为基础数据推送 QMQ 消息 bean,内有中转航组信息和缺失原因
2、中转航组信息机场码需转城市码
3、获取航线锁，使用 zset 存储缺失中转点至最大数量
4、QMQ 推送消息 QPS 为 2~3
{
String depAirport1 = data.getDep1();
String arrAirport1 = data.getArr1();
String depAirport2 = data.getDep2();
String arrAirport2 = data.getArr2();
String flyDate1 = data.getFlyDate1();
if (!Objects.equals(arrAirport1, depAirport2)) {
QMonitor.recordOne("smart_flight_price_miss_mid_different");
}
String missedMidAirport = StringUtils.isBlank(arrAirport1) ? depAirport2 : arrAirport1;
if (StringUtils.isBlank(depAirport1) || StringUtils.isBlank(arrAirport2) || StringUtils.isBlank(missedMidAirport) || StringUtils.isBlank(flyDate1)) {
logger.error("缺少必要信息，本次中转点不补充 depAirport1:{},arrAirport2:{},missedMidAirport:{},flyDate1:{}", depAirport1, arrAirport2, missedMidAirport, flyDate1);
QMonitor.recordOne("smart_flight_price_miss_param_miss");
return;
}
String lockKey = null;
boolean tryLock = false;
try {
String depCity1 = InfoCenter.getCityCodeFromAirportCode(depAirport1);
String missedMidCity = InfoCenter.getCityCodeFromAirportCode(missedMidAirport);
String arrCity2 = InfoCenter.getCityCodeFromAirportCode(arrAirport2);
lockKey = getLockKey(depCity1, arrCity2, flyDate1);
tryLock = RedisLockUtil.checkLock(transferSedis, lockKey, QConfigUtils.getConfig("union_price_miss_lock_expire_time", 3000));
if (!tryLock) {
logger.info("smart_flight_price_miss_lock_check is locked {},{},{}", depCity1, arrCity2, flyDate1);
QMonitor.recordOne("smart_flight_price_miss_lock_check_is_locked");
return;
}
String redisKey = ctripMissMiddleCityService.buildCacheKey(depCity1, arrCity2, flyDate1);
Long sedisSize = transferSedis.zcard(redisKey);
int selectMaxCount = QConfigUtils.getConfig(QconfigConstant.SELECT_MAX_COUNT, QconfigConstant.DEFAULT_SELECT_MAX_COUNT);
long overSize = sedisSize - selectMaxCount;
if (overSize >= 0) {
transferSedis.zremrangeByRank(redisKey, 0L, overSize);
}
transferSedis.zadd(redisKey, System.currentTimeMillis(), missedMidCity);
transferSedis.expire(redisKey, QConfigUtils.getConfig("union_price_miss_mid_city_expire_time", 60000));
QMonitor.recordOne("smart_flight_price_miss_put_zset");
} catch (Exception e) {
logger.error("获取缺失中转点错误", e);
QMonitor.recordOne("smart_flight_price_miss_city_add_error");
} finally {
if (tryLock && StringUtils.isNotEmpty(lockKey)) {
RedisLockUtil.unlock(transferSedis, lockKey);
}
}
}
|
官方得分点：
1、可能当前线程会释放其它线程加的锁
2、没有实现锁续命逻辑，可能有多个请求同时执行同步代码逻辑
3、zset存redis与设置zset集合key的过期时间两者没有原子执行
采分点：
1、 RedisLockUtil.unlock(transferSedis, lockKey);释放其他线程的锁，通常
一个客户端一个锁
2、实现续命锁逻辑对时间进行刷新
3、设置过期时间要原子操作（可用lua脚本调用多条执行命令）
|
我的得分点：
学习redission实现的分布式锁。
