Omer Frenkel has uploaded a new change for review. Change subject: core: use String instead of StringBuilder in SysprepHandler (#873742) ......................................................................
core: use String instead of StringBuilder in SysprepHandler (#873742) SysprepHandler uses StringBuilder as it is a string: it creates new instances for replacements. this patch changes StringBuilder to String to make it clear to the user it is regular string that need to be set. also added return value to populateSysPrepDomainProperties as it manipulates the string inside it as well, caused changes to domain, user and pass in the sysprep file to be lost. Change-Id: Ie447c30759d19e4d42a722deb550a6233becb490 Bug-Url:https://bugzilla.redhat.com/show_bug.cgi?id=873742 Signed-off-by: Omer Frenkel <[email protected]> --- M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandler.java M backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandlerTest.java 2 files changed, 27 insertions(+), 32 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/9102/1 diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandler.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandler.java index 01012e6..79a5318 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandler.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandler.java @@ -64,44 +64,44 @@ } public static String GetSysPrep(VM vm, String hostName, String domain, SysPrepParams sysPrepParams) { - StringBuilder sysPrepContent = new StringBuilder(); + String sysPrepContent = ""; switch (vm.getStaticData().getos()) { case WindowsXP: - sysPrepContent.append(LoadFile(Config.<String> GetValue(ConfigValues.SysPrepXPPath))); + sysPrepContent = LoadFile(Config.<String> GetValue(ConfigValues.SysPrepXPPath)); sysPrepContent = replace(sysPrepContent, "$ProductKey$", Config.<String> GetValue(ConfigValues.ProductKey)); break; case Windows2003: - sysPrepContent.append(LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K3Path))); + sysPrepContent = LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K3Path)); sysPrepContent = replace(sysPrepContent, "$ProductKey$", Config.<String> GetValue(ConfigValues.ProductKey2003)); break; case Windows2003x64: - sysPrepContent.append(LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K3Path))); + sysPrepContent = LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K3Path)); sysPrepContent = replace(sysPrepContent, "$ProductKey$", Config.<String> GetValue(ConfigValues.ProductKey2003x64)); break; case Windows2008: - sysPrepContent.append(LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K8Path))); + sysPrepContent = LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K8Path)); sysPrepContent = replace(sysPrepContent, "$ProductKey$", Config.<String> GetValue(ConfigValues.ProductKey2008)); break; case Windows2008x64: - sysPrepContent.append(LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K8x64Path))); + sysPrepContent = LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K8x64Path)); sysPrepContent = replace(sysPrepContent, "$ProductKey$", Config.<String> GetValue(ConfigValues.ProductKey2008x64)); break; case Windows2008R2x64: - sysPrepContent.append(LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K8R2Path))); + sysPrepContent = LoadFile(Config.<String> GetValue(ConfigValues.SysPrep2K8R2Path)); sysPrepContent = replace(sysPrepContent, "$ProductKey$", Config.<String> GetValue(ConfigValues.ProductKey2008R2)); break; case Windows7: - sysPrepContent.append(LoadFile(Config.<String> GetValue(ConfigValues.SysPrepWindows7Path))); + sysPrepContent = LoadFile(Config.<String> GetValue(ConfigValues.SysPrepWindows7Path)); sysPrepContent = replace(sysPrepContent, "$ProductKey$", Config.<String> GetValue(ConfigValues.ProductKeyWindow7)); break; case Windows7x64: - sysPrepContent.append(LoadFile(Config.<String> GetValue(ConfigValues.SysPrepWindows7x64Path))); + sysPrepContent = LoadFile(Config.<String> GetValue(ConfigValues.SysPrepWindows7x64Path)); sysPrepContent = replace(sysPrepContent, "$ProductKey$", Config.<String> GetValue(ConfigValues.ProductKeyWindow7x64)); break; @@ -111,7 +111,7 @@ if (sysPrepContent.length() > 0) { - populateSysPrepDomainProperties(sysPrepContent, domain, sysPrepParams); + sysPrepContent = populateSysPrepDomainProperties(sysPrepContent, domain, sysPrepParams); sysPrepContent = replace(sysPrepContent, "$ComputerName$", hostName != null ? hostName : ""); sysPrepContent = replace(sysPrepContent, "$AdminPassword$", Config.<String> GetValue(ConfigValues.LocalAdminPassword)); @@ -121,10 +121,10 @@ sysPrepContent = replace(sysPrepContent, "$OrgName$", Config.<String> GetValue(ConfigValues.OrganizationName)); } - return sysPrepContent.toString(); + return sysPrepContent; } - private static void populateSysPrepDomainProperties(StringBuilder sysPrepContent, + private static String populateSysPrepDomainProperties(String sysPrepContent, String domain, SysPrepParams sysPrepParams) { @@ -154,10 +154,12 @@ sysPrepContent = replace(sysPrepContent, "$JoinDomain$", domainName); sysPrepContent = replace(sysPrepContent, "$DomainAdmin$", adminUserName); sysPrepContent = replace(sysPrepContent, "$DomainAdminPassword$", adminPassword); + + return sysPrepContent; } - static StringBuilder replace(StringBuilder sysPrepContent, String pattern, String value) { - return new StringBuilder(sysPrepContent.toString().replaceAll(Pattern.quote(pattern), Matcher.quoteReplacement(value))); + static String replace(String sysPrepContent, String pattern, String value) { + return sysPrepContent.replaceAll(Pattern.quote(pattern), Matcher.quoteReplacement(value)); } private static String useDefaultIfNull(String key, String value, String defaultValue, diff --git a/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandlerTest.java b/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandlerTest.java index 5e0c38f..15a701d 100644 --- a/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandlerTest.java +++ b/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SysprepHandlerTest.java @@ -26,27 +26,27 @@ @Test public void replace_emptyBuilder() { - runAndCheck(new StringBuilder(), "a", "b", ""); + runAndCheck(new String(), "a", "b", ""); } @Test public void replace_patternNotPresent() { - runAndCheck(sb("abcd"), "X", "Y", "abcd"); + runAndCheck("abcd", "X", "Y", "abcd"); } @Test public void replace_valueNotContainsDollar() { - runAndCheck(sb("AdminPassword=$AdminPassword$"), "$AdminPassword$", "AAA", "AdminPassword=AAA"); + runAndCheck("AdminPassword=$AdminPassword$", "$AdminPassword$", "AAA", "AdminPassword=AAA"); } @Test public void replace_keyNotContainsDollar() { - runAndCheck(sb("AdminPassword=someKey"), "someKey", "AAA", "AdminPassword=AAA"); + runAndCheck("AdminPassword=someKey", "someKey", "AAA", "AdminPassword=AAA"); } @Test public void replace_valueContainsDollar() { - runAndCheck(sb("AdminPassword=$AdminPassword$"), + runAndCheck("AdminPassword=$AdminPassword$", "$AdminPassword$", "$A$AA$", "AdminPassword=$A$AA$"); @@ -55,8 +55,8 @@ @Test public void replace_callReplaceTwoTimes() { String text = "AdminName=$AdminName$ AdminPassword=$AdminPassword$"; - StringBuilder firstPart = - runAndCheck(sb(text), + String firstPart = + runAndCheck(text, "$AdminPassword$", "$A$AA$", "AdminName=$AdminName$ AdminPassword=$A$AA$"); @@ -65,22 +65,15 @@ @Test public void replace_callReplaceTwoOccurrences() { - runAndCheck(sb("AdminName=$AdminName$ AdminPassword=$AdminName$"), + runAndCheck("AdminName=$AdminName$ AdminPassword=$AdminName$", "$AdminName$", "$B$BB$", "AdminName=$B$BB$ AdminPassword=$B$BB$"); } - // this is here just to simulate perfectly the way how the SysprepHandler creates the StringBuilder - private StringBuilder sb(String string) { - StringBuilder builder = new StringBuilder(); - builder.append(string); - return builder; - } - - private StringBuilder runAndCheck(StringBuilder original, String pattern, String value, String expected) { - StringBuilder res = SysprepHandler.replace(original, pattern, value); - assertThat(res.toString(), is(equalTo(expected))); + private String runAndCheck(String original, String pattern, String value, String expected) { + String res = SysprepHandler.replace(original, pattern, value); + assertThat(res, is(equalTo(expected))); return res; } } -- To view, visit http://gerrit.ovirt.org/9102 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie447c30759d19e4d42a722deb550a6233becb490 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
