From 0407d2b6e6d70715eeb11439ea0cf6e0a64eebd7 Mon Sep 17 00:00:00 2001 From: "Jain, Rajiv" Date: Fri, 22 May 2026 17:23:48 +0530 Subject: [PATCH] CSTACKEX-191: logical access methods can have return type as primitive --- .../storage/feign/model/VolumeConcise.java | 4 +-- .../OntapPrimaryDatastoreLifecycle.java | 2 +- .../storage/service/StorageStrategy.java | 4 +-- .../storage/service/UnifiedNASStrategy.java | 6 ++-- .../storage/service/UnifiedSANStrategy.java | 31 +++++++++---------- .../storage/service/StorageStrategyTest.java | 4 +-- .../service/UnifiedSANStrategyTest.java | 17 +++++----- 7 files changed, 31 insertions(+), 37 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java index b76fb4f03d6b..82cea1260e10 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java @@ -30,7 +30,6 @@ public class VolumeConcise { private String uuid; @JsonProperty("name") - private String name; public String getUuid() { @@ -46,5 +45,6 @@ public String getName() { } public void setName(String name) { - this.name = name; } + this.name = name; + } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java index 1e44ef3aaa83..d7b89de25461 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java @@ -231,7 +231,7 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust throw new CloudRuntimeException("Storage pool name is null or empty, cannot create primary storage"); } - if (StringUtils.isBlank(providerName )) { + if (StringUtils.isBlank(providerName)) { throw new CloudRuntimeException("Provider name is null or empty, cannot create primary storage"); } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index 02c201adaa13..16aa5ec3b7ec 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -595,7 +595,7 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam * @param values map including SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS * @return map containing logical unit number for the new/existing mapping (SAN) or relevant info for NAS */ - abstract public Map enableLogicalAccess(Map values); + abstract public String enableLogicalAccess(Map values); /** * Method encapsulates the behavior based on the opted protocol in subclasses @@ -610,7 +610,7 @@ public abstract JobResponse revertSnapshotForCloudStackVolume(String snapshotNam * @param values map with SVM name, LUN name, and igroup name (for SAN) or equivalent for NAS * @return map containing logical unit number if mapping exists; otherwise null */ - abstract public Map getLogicalAccess(Map values); + abstract public String getLogicalAccess(Map values); // ── FlexVolume Snapshot accessors ──────────────────────────────────────── diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java index f305dd6d070d..477e92630387 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedNASStrategy.java @@ -202,7 +202,7 @@ public AccessGroup getAccessGroup(Map values) { } @Override - public Map enableLogicalAccess(Map values) { + public String enableLogicalAccess(Map values) { return null; } @@ -211,8 +211,8 @@ public void disableLogicalAccess(Map values) { } @Override - public Map getLogicalAccess(Map values) { - return Map.of(); + public String getLogicalAccess(Map values) { + return null; } private ExportPolicy createExportPolicy(String svmName, ExportPolicy policy) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java index 02661909d7e6..46d6a275fe04 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java @@ -383,10 +383,10 @@ public AccessGroup getAccessGroup(Map values) { } } - public Map enableLogicalAccess(Map values) { + public String enableLogicalAccess(Map values) { logger.info("enableLogicalAccess : Create LunMap"); logger.debug("enableLogicalAccess : Creating LunMap with values {} ", values); - Map response = null; + String lunNumber = null; if (values == null) { logger.error("enableLogicalAccess: LunMap creation failed. Invalid request values: null"); throw new CloudRuntimeException("Failed to create LunMap, invalid request"); @@ -435,9 +435,7 @@ public Map enableLogicalAccess(Map values) { OntapStorageConstants.IGROUP_DOT_NAME, igroupName, OntapStorageConstants.FIELDS, OntapStorageConstants.LOGICAL_UNIT_NUMBER )); - response = Map.of( - OntapStorageConstants.LOGICAL_UNIT_NUMBER, lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString() - ); + lunNumber = lunMapResponse.getRecords().get(0).getLogicalUnitNumber().toString(); } catch (Exception e) { logger.error("enableLogicalAccess: Failed to fetch LunMap details for Lun: {} and igroup: {}, Exception: {}", lunName, igroupName, e); throw new CloudRuntimeException("Failed to fetch LunMap details for Lun: " + lunName + " and igroup: " + igroupName); @@ -448,7 +446,7 @@ public Map enableLogicalAccess(Map values) { logger.error("Exception occurred while creating LunMap", e); throw new CloudRuntimeException("Failed to create LunMap: " + e.getMessage()); } - return response; + return lunNumber; } public void disableLogicalAccess(Map values) { @@ -482,8 +480,9 @@ public void disableLogicalAccess(Map values) { } // GET-only helper: fetch LUN-map and return logical unit number if it exists; otherwise return null - public Map getLogicalAccess(Map values) { + public String getLogicalAccess(Map values) { logger.info("getLogicalAccess : Fetch LunMap"); + String lunNumber = null; logger.debug("getLogicalAccess : Fetching LunMap with values {} ", values); if (values == null) { logger.error("getLogicalAccess: Invalid request values: null"); @@ -507,13 +506,12 @@ public Map getLogicalAccess(Map values) { )); if (lunMapResponse != null && lunMapResponse.getRecords() != null && !lunMapResponse.getRecords().isEmpty()) { Integer lunLogicalUnitNum = lunMapResponse.getRecords().get(0).getLogicalUnitNumber(); - String lunNumber = lunLogicalUnitNum != null ? lunLogicalUnitNum.toString() : null; - return lunNumber != null ? Map.of(OntapStorageConstants.LOGICAL_UNIT_NUMBER, lunNumber) : null; + lunNumber = lunLogicalUnitNum != null ? lunLogicalUnitNum.toString() : null; } } catch (Exception e) { logger.warn("getLogicalAccess: LunMap not found for Lun: {} and igroup: {} ({}).", lunName, igroupName, e.getMessage()); } - return null; + return lunNumber; } @Override @@ -526,9 +524,8 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName ); - Map mapResp = getLogicalAccess(getMap); - if (mapResp != null && mapResp.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER)) { - String lunNumber = mapResp.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER); + String lunNumber = getLogicalAccess(getMap); + if (lunNumber != null) { logger.info("ensureLunMapped: Existing LunMap found for LUN [{}] in igroup [{}] with LUN number [{}]", lunName, accessGroupName, lunNumber); return lunNumber; } @@ -539,12 +536,12 @@ public String ensureLunMapped(String svmName, String lunName, String accessGroup OntapStorageConstants.SVM_DOT_NAME, svmName, OntapStorageConstants.IGROUP_DOT_NAME, accessGroupName ); - Map response = enableLogicalAccess(enableMap); - if (response == null || !response.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER)) { + String response = enableLogicalAccess(enableMap); + if (response == null ) { throw new CloudRuntimeException("Failed to map LUN [" + lunName + "] to iGroup [" + accessGroupName + "]"); } - logger.info("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); - return response.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER); + logger.info("ensureLunMapped: Successfully mapped LUN [{}] to igroup [{}] with LUN number [{}]", lunName, accessGroupName, response); + return response; } /** * Reverts a LUN to a snapshot using the ONTAP CLI-based snapshot file restore API. diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java index b859f57b37b1..41b5aac40321 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/StorageStrategyTest.java @@ -169,7 +169,7 @@ public AccessGroup getAccessGroup(Map values) { } @Override - public Map enableLogicalAccess(Map values) { + public String enableLogicalAccess(Map values) { return null; } @@ -178,7 +178,7 @@ public void disableLogicalAccess(Map values) { } @Override - public Map getLogicalAccess(Map values) { + public String getLogicalAccess(Map values) { return null; } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java index b3f2364656a7..d0f84ab0e455 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/service/UnifiedSANStrategyTest.java @@ -516,12 +516,10 @@ void testEnableLogicalAccess_Success() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - Map result = unifiedSANStrategy.enableLogicalAccess(values); + String result = unifiedSANStrategy.enableLogicalAccess(values); // Verify assertNotNull(result); - assertTrue(result.containsKey(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); - assertEquals("0", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); verify(sanFeignClient).createLunMap(eq(authHeader), eq(true), any(LunMap.class)); } @@ -550,11 +548,10 @@ void testEnableLogicalAccess_AlreadyMapped_ReturnsLunNumber() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - Map result = unifiedSANStrategy.enableLogicalAccess(values); + String result = unifiedSANStrategy.enableLogicalAccess(values); // Verify assertNotNull(result); - assertEquals("5", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); } } @@ -621,11 +618,11 @@ void testGetLogicalAccess_Success() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(response); // Execute - Map result = unifiedSANStrategy.getLogicalAccess(values); + String result = unifiedSANStrategy.getLogicalAccess(values); // Verify assertNotNull(result); - assertEquals("3", result.get(OntapStorageConstants.LOGICAL_UNIT_NUMBER)); + assertEquals("3", result); } } @@ -645,7 +642,7 @@ void testGetLogicalAccess_NotFound_ReturnsNull() { .thenThrow(new RuntimeException("Not found")); // Execute - Map result = unifiedSANStrategy.getLogicalAccess(values); + String result = unifiedSANStrategy.getLogicalAccess(values); // Verify assertNull(result); @@ -1671,7 +1668,7 @@ void testGetLogicalAccess_EmptyResponse_ReturnsNull() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())).thenReturn(emptyResponse); - Map result = unifiedSANStrategy.getLogicalAccess(values); + String result = unifiedSANStrategy.getLogicalAccess(values); assertNull(result); } @@ -1691,7 +1688,7 @@ void testGetLogicalAccess_ExceptionThrown_ReturnsNull() { when(sanFeignClient.getLunMapResponse(eq(authHeader), anyMap())) .thenThrow(new RuntimeException("Connection failed")); - Map result = unifiedSANStrategy.getLogicalAccess(values); + String result = unifiedSANStrategy.getLogicalAccess(values); assertNull(result); }