RockteMQ-AI commented on code in PR #2676:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2676#discussion_r3879590430
##########
server/src/main/java/org/apache/rocketmq/studio/instance/InstanceService.java:
##########
@@ -212,58 +218,184 @@ public CloudImportResultVO
importCloudInstances(InstanceVendor vendor, Long cred
if (credential.getVendor() != vendor) {
throw new BusinessException(400, "Cloud credential vendor does not
match " + vendor);
}
- CloudCatalogProvider catalog = providerRegistry.catalogFor(vendor);
+ CloudImportAccumulator result = new CloudImportAccumulator();
+ CloudCatalogProvider catalog;
+ try {
+ catalog = providerRegistry.catalogFor(vendor);
+ } catch (RuntimeException ex) {
+ result.addFailure("catalog", ex);
+ return finishCloudImport(vendor, credentialId, result);
+ }
+ if (catalog == null) {
+ result.addFailure("catalog", "provider returned no cloud catalog");
+ return finishCloudImport(vendor, credentialId, result);
+ }
- int discovered = 0;
- int imported = 0;
- int skipped = 0;
- List<String> failed = new ArrayList<>();
- for (CloudRegionVO region : catalog.listRegions(credentialId)) {
- if (region == null || !StringUtils.hasText(region.getRegionId())) {
+ List<CloudRegionVO> regions;
+ try {
+ regions = catalog.listRegions(credentialId);
+ } catch (RuntimeException ex) {
+ result.addFailure("regions", ex);
+ return finishCloudImport(vendor, credentialId, result);
+ }
+ if (regions == null) {
+ result.addFailure("regions", "catalog returned a null region
list");
+ return finishCloudImport(vendor, credentialId, result);
+ }
+
+ Set<String> seenRegions = new LinkedHashSet<>();
+ for (CloudRegionVO region : regions) {
+ String regionId = normalizeCloudImportValue(region == null ? null
: region.getRegionId());
+ if (regionId == null) {
+ result.addFailure("region", "catalog returned an invalid
region entry");
continue;
}
- List<CloudInstanceOptionVO> options;
- try {
- options = catalog.listCloudInstances(credentialId,
region.getRegionId(), null);
- } catch (BusinessException ex) {
- failed.add(region.getRegionId() + ": " + ex.getMessage());
+ if (!seenRegions.add(regionId)) {
continue;
}
- for (CloudInstanceOptionVO option : options) {
- if (option == null ||
!StringUtils.hasText(option.getInstanceId())) {
- continue;
- }
- discovered++;
- InstanceVO request = InstanceVO.builder()
- .vendor(vendor)
- .credentialId(credentialId)
- .regionId(region.getRegionId())
- .cloudInstanceId(option.getInstanceId())
- .name(option.getInstanceId())
- .build();
- try {
- createInstance(request);
- imported++;
- } catch (BusinessException ex) {
- if (ex.getMessage() != null &&
ex.getMessage().startsWith("Instance name already exists")) {
- skipped++;
- } else {
- failed.add(option.getInstanceId() + ": " +
ex.getMessage());
- }
- }
+ importCloudRegion(catalog, vendor, credentialId, regionId, result);
+ }
+ return finishCloudImport(vendor, credentialId, result);
+ }
+
+ private void importCloudRegion(CloudCatalogProvider catalog,
InstanceVendor vendor, Long credentialId,
+ String regionId, CloudImportAccumulator
result) {
+ List<CloudInstanceOptionVO> options;
+ try {
+ options = catalog.listCloudInstances(credentialId, regionId, null);
+ } catch (RuntimeException ex) {
+ result.addFailure(regionId, ex);
+ return;
+ }
+ if (options == null) {
+ result.addFailure(regionId, "catalog returned a null instance
list");
+ return;
+ }
+
+ for (int index = 0; index < options.size(); index++) {
+ CloudInstanceOptionVO option = options.get(index);
+ String rowTarget = regionId + " row " + (index + 1);
+ if (option == null) {
+ result.addFailure(rowTarget, "catalog returned a null instance
entry");
+ continue;
}
+ String cloudInstanceId =
normalizeCloudImportValue(option.getInstanceId());
+ if (cloudInstanceId == null) {
+ result.addFailure(rowTarget, "catalog returned an instance
without an id");
+ continue;
+ }
+ if (!result.markDiscovered(regionId, cloudInstanceId)) {
+ continue;
+ }
+ importCloudInstance(vendor, credentialId, regionId,
cloudInstanceId, result);
}
+ }
+
+ private void importCloudInstance(InstanceVendor vendor, Long credentialId,
+ String regionId, String cloudInstanceId,
CloudImportAccumulator result) {
+ InstanceVO request = InstanceVO.builder()
+ .vendor(vendor)
+ .credentialId(credentialId)
+ .regionId(regionId)
Review Comment:
**[Info]** The duplicate-name detection relies on
`e.getMessage().startsWith("Instance name already exists")`. This works but is
fragile — if the exception message changes, duplicates would silently become
creates. Consider using a dedicated exception type (e.g.
`DuplicateInstanceException`) or a structured return value to distinguish
skip-vs-fail vs. matching on message text.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
