Feat 12010 #12403
Feat 12010 #12403
Conversation
…2010 # Conflicts: # src/backend/ci/core/common/common-pipeline-yaml/src/main/kotlin/com/tencent/devops/process/yaml/transfer/ModelTransfer.kt # src/backend/ci/core/common/common-pipeline-yaml/src/main/kotlin/com/tencent/devops/process/yaml/transfer/TemplateModelTransfer.kt # src/backend/ci/core/common/common-pipeline-yaml/src/main/kotlin/com/tencent/devops/process/yaml/v3/models/Variable.kt # src/backend/ci/core/common/common-pipeline/src/main/kotlin/com/tencent/devops/common/pipeline/pojo/BuildFormProperty.kt # src/backend/ci/core/process/api-process/src/main/kotlin/com/tencent/devops/process/constant/ProcessMessageCode.kt # src/backend/ci/core/process/biz-base/src/main/kotlin/com/tencent/devops/process/engine/service/PipelineRepositoryService.kt # src/backend/ci/core/process/biz-base/src/main/kotlin/com/tencent/devops/process/engine/utils/TemplateInstanceUtil.kt # src/backend/ci/core/process/biz-process/src/main/kotlin/com/tencent/devops/process/service/PipelineInfoFacadeService.kt # src/backend/ci/core/process/biz-process/src/main/kotlin/com/tencent/devops/process/service/template/v2/version/processor/PTemplateCompatibilityVersionPostProcessor.kt # support-files/i18n/process/message_en_US.properties # support-files/i18n/process/message_ja_JP.properties # support-files/i18n/process/message_zh_CN.properties
| modelInput.model.handlePublicVarInfo() | ||
| val publicVarGroupNames = modelInput.model.getPublicVarGroups() | ||
| if (!publicVarGroupNames.isNullOrEmpty()) { | ||
| variables!![TEMPLATE_KEY] = publicVarGroupNames.map { |
| fun exportToFile(yaml: String, fileName: String): Response { | ||
| // 流式下载 | ||
| val fileStream = StreamingOutput { output -> | ||
| val sb = StringBuilder() |
There was a problem hiding this comment.
就一个变量，没有必要用StringBuilder对象
| val fileStream = StreamingOutput { output -> | ||
| val sb = StringBuilder() | ||
| sb.append(yaml) | ||
| output.write(sb.toString().toByteArray()) |
There was a problem hiding this comment.
写入字节数组指定字符集为 UTF-8，保证字符正确
| val encodedFileName = URLEncoder.encode("$fileName.yml", "UTF-8") | ||
| return Response | ||
| .ok(fileStream, MediaType.APPLICATION_OCTET_STREAM_TYPE) | ||
| .header("content-disposition", "attachment; filename = $encodedFileName") |
|
|
||
| @JsonSetter("variables") | ||
| private fun setVariables(raw: Any?) { | ||
| when (raw) { |
| referType?.let { conditions.add(REFER_TYPE.eq(it.name)) } | ||
|
|
||
| // 查找每个REFER_ID对应的CREATE_TIME最大的记录 | ||
| val t2 = this.`as`("t2") |
| * 更新变量组引用信息 | ||
| */ | ||
| fun updateVarGroupReferInfo( | ||
| dslContext: DSLContext, |
There was a problem hiding this comment.
方法参数太多，超过5个就最好封装成bean对象
| deleteFlag: Boolean? = null | ||
| ): Int { | ||
| with(TTemplatePipeline.T_TEMPLATE_PIPELINE) { | ||
| val conditions = getQueryTemplatePipelineCondition(projectId, listOf(templateId), instanceType, deleteFlag) |
| } | ||
| } catch (e: Throwable) { | ||
| logger.warn("Failed to update reference counts for projectId: $projectId", e) | ||
| throw e |
There was a problem hiding this comment.
异常堆栈直接抛出去了，不进行任何异常包装处理吗？？
| version: Int, | ||
| latestFlag: Boolean = false | ||
| ) { | ||
| logger.info("updateSingleGroupReferCount for group: $groupName, version: $version, latestFlag: $latestFlag") |
There was a problem hiding this comment.
调用这个方法有些地方没有加model锁，有线程安全问题，参数应该加一个可为空的model锁，如果调用这个方法的地方外面加了就传空，否则这个方法还得加锁保证线程安全
|
|
||
| try { | ||
| // 首先批量保存新的引用信息 | ||
| publicVarGroupReferInfoDao.batchSave(dslContext, resourcePublicVarGroupReferPOS) |
There was a problem hiding this comment.
该dao方法没在事务内，相关的地方都check下
| .distinct() | ||
|
|
||
| // 统一更新所有受影响的变量组引用计数 | ||
| allAffectedGroups.forEach { (groupName, version) -> |
There was a problem hiding this comment.
该dao方法没在事务内，相关的地方都check下
| } | ||
|
|
||
| // 直接更新为实际引用数，而不是增量更新 | ||
| publicVarGroupDao.updateReferCount( |
There was a problem hiding this comment.
当 version = -1（动态版本）时，targetVersion 会被设为最新版本。那么：
dynamicReferCount 统计的是 version = -1 的引用
specificReferCount 统计的是 version = 最新版本号 的引用
但这两个统计可能有重叠，因为同一个引用可能既有 version = -1 的记录，又有 version = 具体版本 的记录。
| val context = DSL.using(configuration) | ||
|
|
||
| // 删除指定版本的变量组引用记录 | ||
| publicVarGroupReferInfoDao.deleteByReferId( |
There was a problem hiding this comment.
只删除变量组引用记录，未删除对应的变量引用记录，与 deletePublicVerGroupRefByReferId 行为不一致
| } | ||
| } catch (e: Throwable) { | ||
| logger.warn("Failed to process pipeline references", e) | ||
| emptyList() |
There was a problem hiding this comment.
异常时返回空列表，但 totalCount 可能不为0，导致分页数据不准确
| return Pair(totalCount, varGroupReferInfo) | ||
| } catch (e: Throwable) { | ||
| logger.warn("Failed to query var group refer info for group: $groupName", e) | ||
| return Pair(0, emptyList()) |
There was a problem hiding this comment.
异常时返回 Pair(0, emptyList())，可能掩盖真实错误，记录详细错误信息或向上抛出
| logger.warn("Failed to list resource var refer info for referId: $referId, groupName: $groupName", e) | ||
| throw ErrorCodeException( | ||
| errorCode = CommonMessageCode.ERROR_REST_EXCEPTION_COMMON_TIP, | ||
| params = arrayOf(e.message ?: "Unknown error") |
| queryReq: PublicVarGroupInfoQueryReqDTO, | ||
| groupName: String, | ||
| varName: String | ||
| ): Pair<Int, List<ResourcePublicVarGroupReferPO>> { |
There was a problem hiding this comment.
方法不要返回Pair<Int, List>这样的结果集，可读性不好
| } else { | ||
| emptyList() | ||
| } | ||
| if (releaseRecords.isNotEmpty() && segmentIds.isNullOrEmpty()) { |
There was a problem hiding this comment.
只检查了 segmentIds 是否为空，未检查数量是否匹配，可能导致索引越界。
| ) | ||
|
|
||
| // 取第一条记录的基本信息 | ||
| val firstRecord = versionRecords.first() |
There was a problem hiding this comment.
versionRecords.first() 在列表为空时会抛出 NoSuchElementException。建议使用firstOrNull方法，记录不存在则报错
| } | ||
| deletedVars.forEach { oldVar -> | ||
| val showInfo = mapOf( | ||
| "alias" to oldVar.alias, |
There was a problem hiding this comment.
不要重复写这么多字符串的变量，这个强调很多次了，可以使用PublicVarDO::alias.name，其它类似的地方都改了
| private val publicVarGroupReferInfoDao: PublicVarGroupReferInfoDao, | ||
| private val publicVarGroupReleaseRecordService: PublicVarGroupReleaseRecordService, | ||
| private val publicVarGroupReferInfoService: PublicVarGroupReferInfoService, | ||
| private val tokenService: ClientTokenService, |
| ) | ||
|
|
||
| if (version != 0) { | ||
| publicVarGroupReferInfoService.updateSingleGroupReferCount( |
| // 提交后再更新新版本的引用计数 | ||
| // 新版本使用 latestFlag = true，统计所有动态引用（version=-1）和固定版本引用 | ||
| if (newVersionReferCount > 0) { | ||
| publicVarGroupReferInfoService.updateSingleGroupReferCount( |
| ): Int { | ||
| var index = currentIndex | ||
| varPOs.forEach { po -> | ||
| val varName = (po as? PublicVarPO)?.varName ?: return@forEach |
There was a problem hiding this comment.
po as? PublicVarPO为什么要这么写，po不是已经确定是PublicVarPO
| } | ||
| val newVersion = version + 1 | ||
| // 计算新版本的初始引用计数（只统计动态版本引用） | ||
| val newVersionReferCount = if (version != 0) { |
There was a problem hiding this comment.
为什么新一个版本号，动态版本号的引用数怎么都算在了这个新版本号上面
|
|
||
| dslContext.transaction { configuration -> | ||
| val context = DSL.using(configuration) | ||
| publicVarGroupDao.deleteByGroupName(context, projectId, groupName) |
There was a problem hiding this comment.
删除前未检查变量组是否被引用（referCount > 0），可能误删正在使用的变量组
| } | ||
| } catch (t: Throwable) { | ||
| logger.warn("Failed to add variable group $groupName", t) | ||
| throw t |
There was a problem hiding this comment.
只有我们已经处理的异常（比如ErrorCodeException）才能抛出去，其它错误堆栈不经包装处理就抛出去了，展示给用户看的就是一堆错误堆栈消息，体验很差
| } | ||
|
|
||
| // 如果是新建变量组（首次创建），注册到权限中心 | ||
| if (isCreate) { |
There was a problem hiding this comment.
这段逻辑放在数据库操作之前更好，要不数据插入成功了，调这个接口失败了会产生脏数据
| private val pipelinePublicVarGroupReleaseRecordDao: PublicVarGroupReleaseRecordDao, | ||
| private val publicVarGroupReferInfoDao: PublicVarGroupReferInfoDao, | ||
| private val publicVarGroupReleaseRecordService: PublicVarGroupReleaseRecordService, | ||
| private val publicVarGroupReferInfoService: PublicVarGroupReferInfoService, |
| // 验证变量组名称格式 | ||
| if (parserVO.name.isBlank() || !parserVO.name.matches(GROUP_NAME_REGEX)) { | ||
| throw ErrorCodeException( | ||
| errorCode = ERROR_PUBLIC_VAR_GROUP_YAML_NAME_FORMAT |
|
|
||
| // 正则表达式常量 | ||
| private val GROUP_NAME_REGEX = Regex("^[a-zA-Z][a-zA-Z0-9_]{2,31}$") | ||
| private val VAR_NAME_REGEX = Regex("^[a-zA-Z_][a-zA-Z0-9_]*$") |
| fun deleteGroup(userId: String, projectId: String, groupName: String): Boolean { | ||
| val redisLock = RedisLock( | ||
| redisOperation = redisOperation, | ||
| lockKey = "${ProcessMessageCode.PUBLIC_VAR_GROUP_DELETE_LOCK_KEY}_${projectId}_$groupName", |
There was a problem hiding this comment.
删除也要判断引用计数，那么涉及引用计数改动的地方应该都使用同一把分布式锁才能保证线程安全
| return true | ||
| } catch (t: Throwable) { | ||
| logger.warn("Failed to delete variable group $groupName", t) | ||
| throw t |
There was a problem hiding this comment.
不能一刀切抛出异常，异常处理按照前面说的方法处理，相关地方都改了
| ) | ||
|
|
||
| // 获取最新版本的变量列表 | ||
| val latestVarPOs = publicVarService.getGroupPublicVar( |
There was a problem hiding this comment.
先查询组信息，再查询变量信息，中间可能有其他线程修改数据；这二个操作应该放在同一个事务里面
| groupName = groupName, | ||
| groupVersion = groupVersion, | ||
| buildFormProperty = buildFormProperty, | ||
| originalIndex = index++ |
There was a problem hiding this comment.
PublicVarGroupVariable 对象的 originalIndex 值没有保证连续唯一性，确认下会不会引起其它问题？
| * @param userId 用户ID | ||
| * @param projectId 项目ID | ||
| * @param model 流水线模型 | ||
| * @param resourceId 资源ID（如流水线ID） |
| */ | ||
| private fun queryLatestVersions( | ||
| projectId: String, | ||
| modelVarGroups: List<*> |
There was a problem hiding this comment.
为什么要写成List<>？这个类型不是确定的吗？不要使用号，相似的地方都改了
| } | ||
|
|
||
| // 计算需要更新引用计数的变量 | ||
| val varsNeedUpdateCount = calculateVarsNeedUpdateCount(context) |
There was a problem hiding this comment.
calculateAndExecuteDelete、referRecordsToAdd、varsNeedUpdateCount这几个方法相同的逻辑重复三次，每次都会调用 getReferencedVarsForGroup和calculateVarGroupDiff、访问数据库；优化下代码
| if (referRecordsToAdd.isNotEmpty()) { | ||
| dslContext.transaction { configuration -> | ||
| val transactionContext = DSL.using(configuration) | ||
| publicVarReferInfoDao.batchSave( |
There was a problem hiding this comment.
calculateAndExecuteDelete、batchSave、calculateVarsNeedUpdateCount、batchUpdateReferCounts不在一个事务里面
|
|
||
| // 确定实际版本号 | ||
| val actualVersion = version ?: publicVarGroupDao.getLatestVersionByGroupName( | ||
| dslContext = dslContext, |
There was a problem hiding this comment.
这逻辑有问题，动态版本引用（version=-1）在统计时被忽略，动态版本就统一算在version=-1上面，而不是当时对应的实际版本吧
|
|
||
| publicVarDao.batchSave(dslContext = transactionContext, publicVarGroupPOs = publicVarPOs) | ||
|
|
||
| publicVarGroupReleaseRecordService.batchAddPublicVarGroupReleaseRecord( |
There was a problem hiding this comment.
一个事务里的所有表操作逻辑要使用同一个dslContext才能保证逻辑正确
| ) | ||
| // 统计所有版本的变量引用进行赋值 | ||
| val varNames = publicVarPOs.map { it.varName } | ||
| publicVarReferInfoService.updateVarReferCount( |
There was a problem hiding this comment.
在变量组刚创建新版本时更新引用计数，但此时还没有引用；要确认model是不是确实引用了该变量
| referVersion = modelPublicVarHandleContext.referVersion | ||
| ) | ||
| // 从引用信息中获取源头项目ID，如果没有则使用当前项目ID | ||
| sourceProjectId = groupReferInfos.firstOrNull()?.sourceProjectId ?: projectId |
There was a problem hiding this comment.
这代码写得有问题，后续可能有跨项目使用变量组的场景，不同变量组可能来自不同项目
| .sortedDescending() // 降序排序，确保先删除索引大的 | ||
|
|
||
| indicesToRemove.forEach { index -> | ||
| params.removeAt(index) |
There was a problem hiding this comment.
按原始位置删除，但删除过程中列表长度变化，可能导致索引错乱
| // 批量生成ID | ||
| val segmentIds = client.get(ServiceAllocIdResource::class) | ||
| .batchGenerateSegmentId("T_RESOURCE_PUBLIC_VAR", publicVarDTO.publicVars.size).data | ||
| if (segmentIds.isNullOrEmpty()) { |
There was a problem hiding this comment.
虽然检查了 segmentIds.isNullOrEmpty()，但没有检查 segmentIds.size >= publicVarDTO.publicVars.size
| version = version | ||
| ).map { publicVarPO -> | ||
| JsonUtil.to(publicVarPO.buildFormProperty, BuildFormProperty::class.java).apply { | ||
| varGroupVersion = null // 清除版本信息 |
There was a problem hiding this comment.
为什么要清除版本信息？这可能导致后续逻辑无法区分不同版本
| } | ||
|
|
||
| fun getTemplateLatestVersion(templateId: String): Long? { | ||
| return templateDao.getLatestTemplate(dslContext, templateId).version |
There was a problem hiding this comment.
只是为了查一个版本号，无需把一行中所有记录都查出来，这行记录有大字段model，另外模板重构后，模板估计不存这张表了，这要确认清楚
| class ServicePipelineSettingResourceImpl @Autowired constructor( | ||
| private val pipelineSettingFacadeService: PipelineSettingFacadeService | ||
| private val pipelineSettingFacadeService: PipelineSettingFacadeService, | ||
| private val modelHandleService: ModelHandleService |
There was a problem hiding this comment.
这个类都没有用到，删掉依赖，相似的地方都改了
| updateDraftVersion(draftResource) | ||
| } | ||
| } | ||
| (pTemplateResourceWithoutVersion.model as? Model)?.let { |
| } | ||
|
|
||
| (pTemplateResourceWithoutVersion.model as? Model)?.let { | ||
| publicVarGroupReferInfoService.handleVarGroupReferBus( |
There was a problem hiding this comment.
一个版本在草稿阶段和发布阶段均调handleVarGroupReferBus这个方法，会不会造成引用处理处理有重复的问题？
…2010 # Conflicts: # support-files/i18n/process/message_en_US.properties # support-files/i18n/process/message_ja_JP.properties # support-files/i18n/process/message_zh_CN.properties
2 participants
No description provided.