Skip to content

Commit c6b677b

Browse files
committed
refactor
Signed-off-by: Abhishek Kumar <[email protected]>
1 parent 6276cd6 commit c6b677b

File tree

2 files changed

+26
-22
lines changed

2 files changed

+26
-22
lines changed

framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsShareManagerImpl.java

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ public class ExtensionsShareManagerImpl extends ManagerBase implements Extension
102102

103103
protected static final String EXTENSIONS_SHARE_SUBDIR = "extensions";
104104
protected static final int DEFAULT_SHARE_LINK_VALIDITY_SECONDS = 3600; // 1 hour
105+
protected static final String IMAGE_STORE_DOWNLOAD_URL_DETAIL_KEY = "imagestoredownloadurl";
106+
protected static final String IMAGE_STORE_DOWNLOAD_TIMESTAMP_DETAIL_KEY = "imagestoredownloadtimestamp";
107+
protected static final String IMAGE_STORE_DOWNLOAD_PATH_DETAIL_KEY = "imagestoredownloadpath";
108+
105109

106110
ConfigKey<Integer> ShareLinkValidityInterval = new ConfigKey<>("Advanced", Integer.class,
107111
"extension.share.link.validity.interval", String.valueOf(DEFAULT_SHARE_LINK_VALIDITY_SECONDS),
@@ -567,17 +571,17 @@ protected AsyncCallFuture<DataObject> downloadExtensionArchiveOnSecondaryStorage
567571

568572
protected void cleanupExistingExtensionDownloadArchiveAndDetails(Extension extension, Long cutoff) {
569573
Map<String, String> details = extensionDetailsDao.listDetailsKeyPairs(extension.getId(), false);
570-
if (!details.containsKey("imagestoredownloadurl")) {
574+
if (!details.containsKey(IMAGE_STORE_DOWNLOAD_URL_DETAIL_KEY)) {
571575
return;
572576
}
573-
final String url = details.get("imagestoredownloadurl");
577+
final String url = details.get(IMAGE_STORE_DOWNLOAD_URL_DETAIL_KEY);
574578
final String storeIdStr = details.get(ApiConstants.IMAGE_STORE_ID);
575-
final String timestampStr = details.get("imagestoredownloadurltimestamp");
579+
final String timestampStr = details.get(IMAGE_STORE_DOWNLOAD_TIMESTAMP_DETAIL_KEY);
576580
final long timestamp = StringUtils.isNotBlank(timestampStr) ? Long.parseLong(timestampStr) : -1L;
577581
if (cutoff != null && timestamp != -1L && timestamp >= cutoff) {
578582
return;
579583
}
580-
final String installPath = details.get("imagestorepath");
584+
final String installPath = details.get(IMAGE_STORE_DOWNLOAD_PATH_DETAIL_KEY);
581585
final long storeId = StringUtils.isNotBlank(storeIdStr) ? Long.parseLong(storeIdStr) : -1L;
582586
if (StringUtils.isNotBlank(url) && storeId != -1L && timestamp != -1L && StringUtils.isNotBlank(installPath)) {
583587
try {
@@ -596,10 +600,10 @@ protected void cleanupExistingExtensionDownloadArchiveAndDetails(Extension exten
596600
Transaction.execute(new TransactionCallbackNoReturn() {
597601
@Override
598602
public void doInTransactionWithoutResult(TransactionStatus status) {
599-
extensionDetailsDao.removeDetail(extension.getId(), "imagestoredownloadurl");
600-
extensionDetailsDao.removeDetail(extension.getId(), "imagestoredownloadurltimestamp");
603+
extensionDetailsDao.removeDetail(extension.getId(), IMAGE_STORE_DOWNLOAD_URL_DETAIL_KEY);
604+
extensionDetailsDao.removeDetail(extension.getId(), IMAGE_STORE_DOWNLOAD_TIMESTAMP_DETAIL_KEY);
601605
extensionDetailsDao.removeDetail(extension.getId(), ApiConstants.IMAGE_STORE_ID);
602-
extensionDetailsDao.removeDetail(extension.getId(), "imagestorepath");
606+
extensionDetailsDao.removeDetail(extension.getId(), IMAGE_STORE_DOWNLOAD_PATH_DETAIL_KEY);
603607
}
604608
});
605609
}
@@ -655,10 +659,10 @@ protected Pair<Boolean, String> downloadExtensionViaSecondaryStorage(Extension e
655659
Transaction.execute(new TransactionCallbackNoReturn() {
656660
@Override
657661
public void doInTransactionWithoutResult(TransactionStatus status) {
658-
extensionDetailsDao.addDetail(extension.getId(), "imagestoredownloadurl", url, false);
659-
extensionDetailsDao.addDetail(extension.getId(), "imagestoredownloadurltimestamp", Long.toString(System.currentTimeMillis()), false);
662+
extensionDetailsDao.addDetail(extension.getId(), IMAGE_STORE_DOWNLOAD_URL_DETAIL_KEY, url, false);
663+
extensionDetailsDao.addDetail(extension.getId(), IMAGE_STORE_DOWNLOAD_TIMESTAMP_DETAIL_KEY, Long.toString(System.currentTimeMillis()), false);
660664
extensionDetailsDao.addDetail(extension.getId(), ApiConstants.IMAGE_STORE_ID, Long.toString(imageStoreId), false);
661-
extensionDetailsDao.addDetail(extension.getId(), "imagestorepath", installPath, false);
665+
extensionDetailsDao.addDetail(extension.getId(), IMAGE_STORE_DOWNLOAD_PATH_DETAIL_KEY, installPath, false);
662666
}
663667
});
664668
return new Pair<>(true, url);
@@ -716,15 +720,15 @@ public Pair<Boolean, String> syncExtension(Extension extension, ManagementServer
716720
try {
717721
archiveInfo = createArchiveForSync(extension, files);
718722
} catch (IOException e) {
719-
String msg = "Failed to create archive";
723+
String msg = "Archive creation failed";
720724
logger.error("{} for {}", extension, msg, e);
721725
return new Pair<>(false, msg);
722726
}
723727
String signedUrl;
724728
try {
725729
signedUrl = generateSignedArchiveUrl(sourceManagementServer, archiveInfo.getPath());
726730
} catch (DecoderException | NoSuchAlgorithmException | InvalidKeyException | CloudRuntimeException e) {
727-
String msg = "Failed to generate signed URL";
731+
String msg = "Signed URL generation failed";
728732
logger.error("{} for {} using {}", msg, extension, sourceManagementServer, e);
729733
return new Pair<>(false, msg);
730734
}
@@ -772,7 +776,7 @@ public Pair<Boolean, String> downloadAndApplyExtensionSync(Extension extension,
772776

773777
applyExtensionSync(extension, cmd.getSyncType(), tmpArchive, extensionRootPath);
774778
} catch (IOException e) {
775-
String msg = String.format("Failed to download/apply sync for %s from %s: %s", extension,
779+
String msg = String.format("Download/apply sync for %s from %s failed: %s", extension,
776780
cmd.getDownloadUrl(), e.getMessage());
777781
logger.error(msg, e);
778782
return new Pair<>(false, msg);
@@ -790,15 +794,15 @@ public Pair<Boolean, String> downloadExtension(Extension extension, ManagementSe
790794
try {
791795
archiveInfo = createArchiveForDownload(extension);
792796
} catch (IOException e) {
793-
String msg = "Failed to create archive";
797+
String msg = "Archive creation failed";
794798
logger.error("{} for {}", extension, msg, e);
795799
return new Pair<>(false, msg);
796800
}
797801
String signedUrl;
798802
try {
799803
signedUrl = generateSignedArchiveUrl(managementServer, archiveInfo.getPath());
800804
} catch (DecoderException | NoSuchAlgorithmException | InvalidKeyException e) {
801-
String msg = "Failed to generate signed URL";
805+
String msg = "Signed URL generation failed";
802806
logger.error("{} for {} using {}", msg, extension, managementServer, e);
803807
return new Pair<>(false, msg);
804808
}

framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsShareManagerImplTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ public void generateSignedArchiveUrlReturnsValidUrlWithSignature() throws Except
376376
Path archivePath = Path.of("/share/extensions/test-archive.tgz");
377377
String baseUrl = "http://abc";
378378
doReturn(baseUrl).when(extensionsShareManager).getManagementServerBaseUrl(managementServer);
379-
try (MockedStatic<ServerPropertiesUtil> serverPropertiesUtilMock = mockStatic(ServerPropertiesUtil.class);
379+
try (MockedStatic<ServerPropertiesUtil> serverPropertiesUtilMock = mockStatic(ServerPropertiesUtil.class, Mockito.CALLS_REAL_METHODS);
380380
MockedStatic<HMACSignUtil> hmacSignUtilMock = mockStatic(HMACSignUtil.class)) {
381381
serverPropertiesUtilMock.when(() -> ServerPropertiesUtil.getShareSecret()).thenReturn("secretKey");
382382
hmacSignUtilMock.when(() -> HMACSignUtil.generateSignature(anyString(), anyString()))
@@ -855,7 +855,7 @@ public void syncExtensionReturnsFailureWhenArchiveCreationFails() throws Excepti
855855
Pair<Boolean, String> result = extensionsShareManager.syncExtension(extension, sourceManagementServer,
856856
targetManagementServers, files);
857857
assertFalse(result.first());
858-
assertEquals("Failed to create archive", result.second());
858+
assertEquals("Archive creation failed", result.second());
859859
}
860860

861861
@Test
@@ -872,7 +872,7 @@ public void syncExtensionReturnsFailureWhenSignedUrlGenerationFails() throws Exc
872872
Pair<Boolean, String> result = extensionsShareManager.syncExtension(extension, sourceManagementServer,
873873
targetManagementServers, files);
874874
assertFalse(result.first());
875-
assertEquals("Failed to generate signed URL", result.second());
875+
assertEquals("Signed URL generation failed", result.second());
876876
}
877877

878878
@Test
@@ -981,7 +981,7 @@ public void downloadAndApplyExtensionSyncHandlesIOExceptionDuringDownload() thro
981981
doReturn(tmpArchive).when(extensionsFilesystemManager).getExtensionRootPath(extension);
982982
Pair<Boolean, String> result = extensionsShareManager.downloadAndApplyExtensionSync(extension, cmd);
983983
assertFalse(result.first());
984-
assertTrue(result.second().contains("Failed to download/apply sync"));
984+
assertTrue(result.second().startsWith("Download/apply sync for "));
985985
} finally {
986986
Files.deleteIfExists(tmpArchive);
987987
}
@@ -1008,7 +1008,7 @@ public void downloadAndApplyExtensionSyncHandlesIOExceptionDuringApplication() t
10081008
any(Path.class));
10091009
Pair<Boolean, String> result = extensionsShareManager.downloadAndApplyExtensionSync(extension, cmd);
10101010
assertFalse(result.first());
1011-
assertTrue(result.second().contains("Failed to download/apply sync"));
1011+
assertTrue(result.second().startsWith("Download/apply sync for "));
10121012
} finally {
10131013
Files.deleteIfExists(tmpArchive);
10141014
}
@@ -1037,7 +1037,7 @@ public void downloadExtensionFailsWhenArchiveCreationThrowsIOException() throws
10371037
.createArchiveForDownload(extension);
10381038
Pair<Boolean, String> result = extensionsShareManager.downloadExtension(extension, managementServer);
10391039
assertFalse(result.first());
1040-
assertEquals("Failed to create archive", result.second());
1040+
assertEquals("Archive creation failed", result.second());
10411041
}
10421042

10431043
@Test
@@ -1052,6 +1052,6 @@ public void downloadExtensionFailsWhenSignedUrlGenerationThrowsException() throw
10521052
.generateSignedArchiveUrl(managementServer, archivePath);
10531053
Pair<Boolean, String> result = extensionsShareManager.downloadExtension(extension, managementServer);
10541054
assertFalse(result.first());
1055-
assertEquals("Failed to generate signed URL", result.second());
1055+
assertEquals("Signed URL generation failed", result.second());
10561056
}
10571057
}

0 commit comments

Comments
 (0)