feat(openapi): introduce non-invasive OpenApiModelConverters for generated model types#5470
Conversation
WalkthroughIntroduces a new final utility class, OpenApiModelConverters, providing static bidirectional converters between portal domain objects and OpenAPI-generated DTOs. It covers single-object and collection mappings for items, apps, namespaces, releases, rules, clusters, organizations, instances, env-cluster info, and item change/diff structures, using Gson and BeanUtils. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dosubot
Bot
added
size:XS
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apollo-portal/pom.xml (1)
30-168: Do not fetch the OpenAPI spec from a mutable remote URL at build timePointing
<apollo.openapi.spec.url>tohttps://raw.githubusercontent.com/...makes every Maven build depend on external network access and an uncontrolled file that can change (or disappear) at any moment. That’s a supply-chain and reproducibility problem, and it will break offline or air‑gapped builds.Vendor the OpenAPI document inside this repository (or publish it to an internal, versioned artifact) and reference the local path/property instead. That way the generated sources stay deterministic and the build remains self-contained.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apollo-portal/pom.xml(4 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java(1 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java(5 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (1)
SpringSecurityUserInfoHolder(33-121)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/UserInfo.java (1)
UserInfo(19-79)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (8)
apollo-common/src/main/java/com/ctrip/framework/apollo/common/utils/BeanUtils.java (1)
BeanUtils(37-255)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/ItemBO.java (1)
ItemBO(21-76)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/NamespaceBO.java (1)
NamespaceBO(23-103)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/ReleaseBO.java (1)
ReleaseBO(24-46)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/model/NamespaceSyncModel.java (1)
NamespaceSyncModel(26-70)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/model/NamespaceTextModel.java (1)
NamespaceTextModel(24-104)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/EnvClusterInfo.java (1)
EnvClusterInfo(24-48)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/NamespaceIdentifier.java (1)
NamespaceIdentifier(23-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (8)
🔇 Additional comments (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java (1)
112-114: ConfirmConsumerServicepresence for auth/ldap contexts.With the new constructor dependency, the
springSecurityUserInfoHolderbean will now fail to instantiate if noConsumerServicebean is registered when theauthorldapprofile is active. Please double-check that those application contexts (including relevant tests) always exposeConsumerService; otherwise we’ll hit a startupNoSuchBeanDefinitionException.Also applies to: 216-218
| import org.springframework.util.CollectionUtils; | ||
|
|
||
| import java.lang.reflect.Type; | ||
| import java.util.*; |
There was a problem hiding this comment.
Avoid using the * symbol when importing packages
There was a problem hiding this comment.
Already fixed that issue, thanks for reviewing!
| return namespaceBOs.stream().map(OpenApiModelConverters::fromNamespaceBO).collect(Collectors.toCollection(LinkedList::new)); | ||
| } | ||
|
|
||
| public static OpenNamespaceLockDTO fromNamespaceLockDTO(String namespaceName, NamespaceLockDTO namespaceLock) { |
There was a problem hiding this comment.
The namespaces LockDTO and Opennamespaces LockDTO structures are inconsistent, which may result in compatibility issues
There was a problem hiding this comment.
Thanks for reviewing but this is a method originally defined in OpenApiBeanUtils and has been verified. All such methods are now annotated with "originally defined in OpenApiBeanUtils"
341746c to
bacf40a
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (2)
249-256: Critical: Fix NPE when gray release rule items are null.
openGrayReleaseRuleDTO.getRuleItems()returnsnullwhen clients omit rule items (valid for "clear all rules" scenarios). Line 249 directly wraps this innew HashSet<>(), causing aNullPointerExceptionthat will fail all such requests with HTTP 500.Apply this fix to handle null/empty rule items:
- Set<OpenGrayReleaseRuleItemDTO> openGrayReleaseRuleItemDTOSet = new HashSet<>(openGrayReleaseRuleDTO.getRuleItems()); - openGrayReleaseRuleItemDTOSet.forEach(openGrayReleaseRuleItemDTO -> { + Set<OpenGrayReleaseRuleItemDTO> openGrayReleaseRuleItemDTOSet = openGrayReleaseRuleDTO.getRuleItems(); + if (!CollectionUtils.isEmpty(openGrayReleaseRuleItemDTOSet)) { + openGrayReleaseRuleItemDTOSet.forEach(openGrayReleaseRuleItemDTO -> { - String clientAppId = openGrayReleaseRuleItemDTO.getClientAppId(); - Set<String> clientIpList = new HashSet<>(openGrayReleaseRuleItemDTO.getClientIpList()); - Set<String> clientLabelList = new HashSet<>(openGrayReleaseRuleItemDTO.getClientLabelList()); - GrayReleaseRuleItemDTO ruleItem = new GrayReleaseRuleItemDTO(clientAppId, clientIpList, clientLabelList); - grayReleaseRuleDTO.addRuleItem(ruleItem); - }); + String clientAppId = openGrayReleaseRuleItemDTO.getClientAppId(); + Set<String> clientIpList = new HashSet<>(openGrayReleaseRuleItemDTO.getClientIpList()); + Set<String> clientLabelList = new HashSet<>(openGrayReleaseRuleItemDTO.getClientLabelList()); + GrayReleaseRuleItemDTO ruleItem = new GrayReleaseRuleItemDTO(clientAppId, clientIpList, clientLabelList); + grayReleaseRuleDTO.addRuleItem(ruleItem); + }); + }
38-38: Replace wildcard import with explicit imports.Wildcard imports reduce code clarity and can cause unexpected compilation issues when new types are added to imported packages.
Based on coding guidelines
Replace line 38 with explicit imports:
-import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Set;
🧹 Nitpick comments (2)
apollo-portal/pom.xml (1)
163-163: Reconsider skipping OpenAPI spec validation.Setting
skipValidateSpectotruebypasses validation of the OpenAPI specification, which could allow malformed specs to generate broken code that only fails at runtime.Unless there's a specific compatibility issue with the validator, remove this line to enable validation:
-<skipValidateSpec>true</skipValidateSpec>If validation must be skipped due to known spec issues, please document the reason in a comment.
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (1)
45-50: Consider making utility class constructor private with clear message.The private constructor prevents instantiation but could benefit from an explicit assertion to fail fast if instantiated via reflection.
-private OpenApiModelConverters() {} +private OpenApiModelConverters() { + throw new AssertionError("Utility class should not be instantiated"); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apollo-portal/pom.xml(4 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/filter/ConsumerAuthenticationFilter.java(4 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/service/ConsumerService.java(2 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/ConsumerAuthUtil.java(4 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java(1 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java(5 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java(1 hunks)pom.xml(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (1)
SpringSecurityUserInfoHolder(37-125)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/UserInfo.java (1)
UserInfo(19-79)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (8)
apollo-common/src/main/java/com/ctrip/framework/apollo/common/utils/BeanUtils.java (1)
BeanUtils(37-255)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/ItemBO.java (1)
ItemBO(21-76)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/NamespaceBO.java (1)
NamespaceBO(23-103)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/ReleaseBO.java (1)
ReleaseBO(24-46)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/model/NamespaceSyncModel.java (1)
NamespaceSyncModel(26-70)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/model/NamespaceTextModel.java (1)
NamespaceTextModel(24-104)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/EnvClusterInfo.java (1)
EnvClusterInfo(24-48)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/NamespaceIdentifier.java (1)
NamespaceIdentifier(23-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (8)
🔇 Additional comments (3)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (3)
160-170: Document namespace lock structure mapping decision.As noted in past reviews,
NamespaceLockDTOandOpenNamespaceLockDTOhave inconsistent structures. The conversion at line 160-169 manually constructs the OpenAPI model rather than usingBeanUtils.transform, suggesting intentional structural differences.Please clarify:
- Are these structural differences documented in the OpenAPI spec design?
- Should this mapping logic be unit tested to prevent regression?
Consider adding a comment explaining why manual mapping is necessary:
// Manual mapping required: OpenNamespaceLockDTO uses isLocked boolean + namespaceName, // while NamespaceLockDTO uses presence/absence pattern with dataChangeCreatedBy public static OpenNamespaceLockDTO fromNamespaceLockDTO(String namespaceName, NamespaceLockDTO namespaceLock) {
47-48: Thread-safe Gson instance is appropriate for utility class.The static
GSONinstance is immutable and thread-safe by default, making it suitable for concurrent use across all conversion methods.
53-76: Item conversion methods handle null inputs correctly.The
Preconditions.checkArgumentguards on single-item conversions andCollectionUtils.isEmptychecks on batch operations provide appropriate null safety. The use of streams for batch conversions is clean and efficient.
| <common-lang3.version>3.18.0</common-lang3.version> | ||
| <gson.version>2.10.1</gson.version> | ||
| <guava.version>32.0.0-jre</guava.version> | ||
| <jackson-databind-nullable.version>0.2.7</jackson-databind-nullable.version> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify dependency versions are current and secure.
The added OpenAPI-related dependencies should be checked for:
- Whether these are the latest stable versions
- Any known security vulnerabilities
Please run the following script to check the latest versions and security advisories:
Also applies to: 83-84, 208-222
🏁 Script executed:
#!/bin/bash
# Check latest versions from Maven Central and security advisories
echo "=== Checking jackson-databind-nullable ==="
curl -s "https://search.maven.org/solrsearch/select?q=g:org.openapitools+AND+a:jackson-databind-nullable&rows=1&wt=json" | jq -r '.response.docs[0].latestVersion'
echo "=== Checking swagger-annotations ==="
curl -s "https://search.maven.org/solrsearch/select?q=g:io.swagger.core.v3+AND+a:swagger-annotations&rows=1&wt=json" | jq -r '.response.docs[0].latestVersion'
echo "=== Checking swagger-models ==="
curl -s "https://search.maven.org/solrsearch/select?q=g:io.swagger.core.v3+AND+a:swagger-models&rows=1&wt=json" | jq -r '.response.docs[0].latestVersion'
echo "=== Checking for security advisories ==="
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: MAVEN, package: "org.openapitools:jackson-databind-nullable") {
nodes {
advisory { summary severity publishedAt }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}'
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: MAVEN, package: "io.swagger.core.v3:swagger-annotations") {
nodes {
advisory { summary severity publishedAt }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}'
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: MAVEN, package: "io.swagger.core.v3:swagger-models") {
nodes {
advisory { summary severity publishedAt }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}'Length of output: 371
Invalid dependency versions detected — update to actual latest.
<jackson-databind-nullable.version>is 0.2.7 but Maven Central’s latest is 0.2.6.<swagger-annotations.version>and<swagger-models.version>are 2.2.37 but Maven Central’s latest are 2.2.31.
Adjust to published versions or configure the repository that hosts these newer releases.
🤖 Prompt for AI Agents
In pom.xml around line 74, the declared dependency property
<jackson-databind-nullable.version> is set to 0.2.7 and swagger properties
<swagger-annotations.version> and <swagger-models.version> are set to 2.2.37
even though Maven Central shows latest published versions as 0.2.6 for
jackson-databind-nullable and 2.2.31 for the swagger artifacts; update those
version properties to the published versions (or add/configure the correct
repository that hosts the newer releases) so the POM references valid,
resolvable versions and rebuild to verify resolution.
182432e to
ca298c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (1)
146-152: Add error handling for JSON parsing.Line 149 parses
release.getConfigurations()without catchingJsonSyntaxException. Malformed JSON from database corruption or manual edits will cause unexpected 500 errors.Wrap the parsing with error handling:
public static OpenReleaseDTO fromReleaseDTO(ReleaseDTO release) { Preconditions.checkArgument(release != null); OpenReleaseDTO openReleaseDTO = BeanUtils.transform(OpenReleaseDTO.class, release); - Map<String, String> configs = GSON.fromJson(release.getConfigurations(), TYPE); - openReleaseDTO.setConfigurations(configs); + try { + Map<String, String> configs = GSON.fromJson(release.getConfigurations(), TYPE); + openReleaseDTO.setConfigurations(configs); + } catch (com.google.gson.JsonSyntaxException e) { + throw new IllegalArgumentException("Invalid JSON in release configurations for release id: " + release.getId(), e); + } return openReleaseDTO; }apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (1)
102-108: Avoid breaking OpenAPI calls when the owner user is missing.Lines 103-108 throw an
IllegalStateExceptionwheneveruserService.findByUserId(consumer.getOwnerName())returnsnull. This turns a previously tolerable data gap (owner user not yet provisioned or already deleted) into a 500 for every OpenAPI call, contradicting the zero-behavior-change expectation. The legacygetUser()path simply returned a minimalUserInfostub when the lookup missed, allowing downstream logic to keep working.Preserve that behavior here: if the lookup fails, build the fallback
UserInfo(with the owner's id) instead of throwing. That keeps OpenAPI usable while still logging the anomaly.UserInfo consumerUserInfo = userService.findByUserId(consumer.getOwnerName()); - if (consumerUserInfo == null) { - logger.error("Consumer not found or incomplete, consumer={}", consumer.getOwnerName()); - throw new IllegalStateException("Consumer information is missing for owner of consumer " + - consumer.getOwnerName()); - } - return consumerUserInfo; + if (consumerUserInfo != null) { + return consumerUserInfo; + } + + logger.warn("Consumer owner {} not found in portal user service, returning fallback UserInfo", consumer.getOwnerName()); + UserInfo fallback = new UserInfo(); + fallback.setUserId(consumer.getOwnerName()); + return fallback;Based on past review comments.
🧹 Nitpick comments (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (1)
79-81: Remove duplicaterequestURIretrieval.Line 80 retrieves
requestURIbut it's not used afterward. Line 81 recalculates the path fromrequest.getRequestURI()again. Remove the unused variable.- HttpServletRequest request = attributes.getRequest(); - String requestURI = request.getRequestURI(); - String path = request.getRequestURI().substring(request.getContextPath().length()); + HttpServletRequest request = attributes.getRequest(); + String path = request.getRequestURI().substring(request.getContextPath().length()); + String requestURI = request.getRequestURI();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java(1 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java(5 hunks)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/UserInfo.java (1)
UserInfo(19-79)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (14)
apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ClusterDTO.java (1)
ClusterDTO(23-80)apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/GrayReleaseRuleDTO.java (1)
GrayReleaseRuleDTO(24-81)apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/GrayReleaseRuleItemDTO.java (1)
GrayReleaseRuleItemDTO(28-85)apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemDTO.java (1)
ItemDTO(20-103)apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/NamespaceDTO.java (1)
NamespaceDTO(22-66)apollo-common/src/main/java/com/ctrip/framework/apollo/common/utils/BeanUtils.java (1)
BeanUtils(37-255)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/KVEntity.java (1)
KVEntity(19-44)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/ItemBO.java (1)
ItemBO(21-76)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/NamespaceBO.java (1)
NamespaceBO(23-103)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/bo/ReleaseBO.java (1)
ReleaseBO(24-46)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/model/NamespaceSyncModel.java (1)
NamespaceSyncModel(26-70)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/model/NamespaceTextModel.java (1)
NamespaceTextModel(24-104)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/EnvClusterInfo.java (1)
EnvClusterInfo(24-48)apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/NamespaceIdentifier.java (1)
NamespaceIdentifier(23-76)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (1)
SpringSecurityUserInfoHolder(37-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (8)
🔇 Additional comments (15)
apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/util/OpenApiModelConverters.java (10)
86-114: LGTM!The Item conversion methods are well-implemented with proper null checks, empty collection handling, and consistent use of BeanUtils for object mapping. The batch conversion methods correctly leverage stream operations.
116-142: LGTM!The App and AppNamespace conversion methods are correctly implemented with proper null checks and consistent BeanUtils usage.
155-179: LGTM!The
fromReleaseBOandfromReleaseBOsmethods correctly transform internal ReleaseBO objects to OpenAPI models, with proper null-safe collection handling for the KVEntity items.
182-290: LGTM!The Namespace conversion methods are well-structured with comprehensive null safety. The manual field mapping in
toNamespaceIdentifierandfromNamespaceIdentifiercorrectly handles the Env enum conversion, and the nested transformations (items, sync models) are properly delegated to their respective converters.
292-323: LGTM!The gray release rule conversions correctly handle null rule items and nested IP/label lists with defensive checks at lines 308, 311-316. The past review concerns about NPE risks have been properly addressed.
325-337: LGTM!The Cluster conversion methods are straightforward bidirectional transformations with proper null guards.
339-353: LGTM!The Organization conversion methods follow the established pattern with proper null and empty collection handling.
355-369: LGTM!The Instance conversion methods are correctly implemented following the established conversion patterns with appropriate defensive checks.
371-385: LGTM!The Env/Cluster info conversion methods correctly transform the domain objects with proper null and empty collection handling.
387-425: LGTM!The Item diffs and change sets conversion methods properly handle nested transformations with appropriate null checks for all optional fields (createItems, updateItems, deleteItems, namespace, diffs). The delegation to specialized converters maintains consistency across the utility class.
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/springsecurity/SpringSecurityUserInfoHolder.java (3)
44-48: LGTM! Lazy wiring prevents circular dependency.The
@Lazyannotation on theConsumerServiceparameter correctly breaks potential circular dependencies while preserving the ability to resolve consumer-based user info for OpenAPI requests.
50-68: LGTM! Two-stage resolution preserves backward compatibility.The refactored
getUser()correctly attempts consumer-based resolution for OpenAPI requests before falling back to Spring Security context, maintaining existing behavior for non-OpenAPI flows.
114-123: LGTM! Extracted method improves readability.The
getCurrentUsername()method correctly extracts the username from the Spring Security context, handlingUserDetails,Principal, and fallback cases as before.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/spi/configuration/AuthConfiguration.java (2)
22-22: LGTM! Import updates align with new dependencies.The added
ConsumerServiceimport (line 22) and consolidated imports (lines 36, 46) correctly support the updated bean factory methods without introducing unnecessary dependencies.Also applies to: 36-36, 46-46
101-102: LGTM! Bean wiring matches updated constructor.Both
springSecurityUserInfoHolderbean factories (lines 101-102 inSpringSecurityAuthAutoConfigurationand lines 205-206 inSpringSecurityLDAPAuthAutoConfiguration) correctly inject the@Lazy ConsumerService, matching the updatedSpringSecurityUserInfoHolderconstructor signature.Also applies to: 205-206
…model types Co-authored-by: arrow2020 <316166287@qq.com>
ca298c0 to
8043e61
Compare
dosubot
Bot
added
size:L
4 participants
What's the purpose of this PR
Introduce a non-invasive OpenApiModelConverters utility to convert between portal DTO/BO/entities and generated com.ctrip.framework.apollo.openapi.model.* types, enabling subsequent domain refactors to use generated models without
modifying existing OpenApiBeanUtils.
Which issue(s) this PR fixes:
Fixes #5462
Brief changelog
OpenNamespaceIdentifier.
Summary by CodeRabbit