From ce5274fed94119e67f4912f14f84c6c0cb4745c2 Mon Sep 17 00:00:00 2001 From: Tomas Bjerre Date: Mon, 31 Jul 2017 07:41:04 +0200 Subject: [PATCH] Blocking KEEP_THIS_TO_LEAVE_UNCHANGED from being accidently saved --- CHANGELOG.md | 8 +- .../bjurr/prnfb/service/SettingsService.java | 135 ++++++++++-------- .../prnfb/settings/PrnfbNotification.java | 3 +- .../prnfb/service/SettingsServiceTest.java | 46 +++--- 4 files changed, 106 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 021eb23..e0ae41f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,16 @@ Changelog of Pull Request Notifier for Bitbucket. ## Unreleased +### No issue + Blocking KEEP_THIS_TO_LEAVE_UNCHANGED from being accidently saved + + [d3b0537c21dded1](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/d3b0537c21dded1) Tomas Bjerre *2017-07-31 05:41:23* + +## 3.8 ### No issue doc - [ec92968f70f4ddf](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/ec92968f70f4ddf) Tomas Bjerre *2017-07-30 19:27:54* + [a9fbe75c3fcca7d](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/a9fbe75c3fcca7d) Tomas Bjerre *2017-07-30 19:28:12* ## 3.7 ### GitHub [#238](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/issues/238) Project and Repo not shown correctly in dropboxes in global admin GUI diff --git a/src/main/java/se/bjurr/prnfb/service/SettingsService.java b/src/main/java/se/bjurr/prnfb/service/SettingsService.java index 00ef78c..8ce5142 100644 --- a/src/main/java/se/bjurr/prnfb/service/SettingsService.java +++ b/src/main/java/se/bjurr/prnfb/service/SettingsService.java @@ -82,7 +82,7 @@ public PrnfbNotification addOrUpdateNotification(PrnfbNotification prnfbNotifica public PrnfbNotification doInTransaction() { try { return doAddOrUpdateNotification(prnfbNotification); - } catch (ValidationException e) { + } catch (final ValidationException e) { propagate(e); } return null; @@ -121,7 +121,7 @@ public Optional findNotification(UUID notificationUuid) { } public PrnfbButton getButton(UUID buttionUuid) { - Optional foundOpt = findButton(buttionUuid); + final Optional foundOpt = findButton(buttionUuid); if (!foundOpt.isPresent()) { throw new RuntimeException(buttionUuid + " not fond in:\n" + on('\n').join(getButtons())); } @@ -133,8 +133,8 @@ public List getButtons() { } public List getButtons(String projectKey) { - List found = newArrayList(); - for (PrnfbButton candidate : getPrnfbSettings().getButtons()) { + final List found = newArrayList(); + for (final PrnfbButton candidate : getPrnfbSettings().getButtons()) { if (candidate.getProjectKey().isPresent() && candidate.getProjectKey().get().equals(projectKey)) { found.add(candidate); @@ -144,8 +144,8 @@ public List getButtons(String projectKey) { } public List getButtons(String projectKey, String repositorySlug) { - List found = newArrayList(); - for (PrnfbButton candidate : getPrnfbSettings().getButtons()) { + final List found = newArrayList(); + for (final PrnfbButton candidate : getPrnfbSettings().getButtons()) { if (candidate.getProjectKey().isPresent() && candidate.getProjectKey().get().equals(projectKey) // && candidate.getRepositorySlug().isPresent() @@ -165,8 +165,8 @@ public List getNotifications() { } public List getNotifications(String projectKey) { - List found = newArrayList(); - for (PrnfbNotification candidate : getPrnfbSettings().getNotifications()) { + final List found = newArrayList(); + for (final PrnfbNotification candidate : getPrnfbSettings().getNotifications()) { if (candidate.getProjectKey().isPresent() && candidate.getProjectKey().get().equals(projectKey)) { found.add(candidate); @@ -176,8 +176,8 @@ public List getNotifications(String projectKey) { } public List getNotifications(String projectKey, String repositorySlug) { - List found = newArrayList(); - for (PrnfbNotification candidate : getPrnfbSettings().getNotifications()) { + final List found = newArrayList(); + for (final PrnfbNotification candidate : getPrnfbSettings().getNotifications()) { if (candidate.getProjectKey().isPresent() && candidate.getProjectKey().get().equals(projectKey) // && candidate.getRepositorySlug().isPresent() @@ -208,8 +208,8 @@ public void setPrnfbSettingsData(PrnfbSettingsData prnfbSettingsData) { new TransactionCallback() { @Override public Void doInTransaction() { - PrnfbSettings oldSettings = doGetPrnfbSettings(); - PrnfbSettings newPrnfbSettings = + final PrnfbSettings oldSettings = doGetPrnfbSettings(); + final PrnfbSettings newPrnfbSettings = prnfbSettingsBuilder(oldSettings) // .setPrnfbSettingsData(prnfbSettingsData) // .build(); @@ -224,8 +224,8 @@ private PrnfbButton doAddOrUpdateButton(PrnfbButton prnfbButton) { doDeleteButton(prnfbButton.getUuid()); } - PrnfbSettings originalSettings = doGetPrnfbSettings(); - PrnfbSettings updated = + final PrnfbSettings originalSettings = doGetPrnfbSettings(); + final PrnfbSettings updated = prnfbSettingsBuilder(originalSettings) // .withButton(prnfbButton) // .build(); @@ -236,28 +236,39 @@ private PrnfbButton doAddOrUpdateButton(PrnfbButton prnfbButton) { private PrnfbNotification doAddOrUpdateNotification(PrnfbNotification newNotification) throws ValidationException { - Optional oldNotification = findNotification(newNotification.getUuid()); + final UUID notificationUuid = newNotification.getUuid(); + + Optional oldUser = Optional.absent(); + Optional oldPassword = Optional.absent(); + Optional oldProxyUser = Optional.absent(); + Optional oldProxyPassword = Optional.absent(); + final Optional oldNotification = findNotification(notificationUuid); + if (oldNotification.isPresent()) { + oldUser = oldNotification.get().getUser(); + oldPassword = oldNotification.get().getPassword(); + oldProxyUser = oldNotification.get().getProxyUser(); + oldProxyPassword = oldNotification.get().getProxyPassword(); + } + + final String user = keepIfUnchanged(newNotification.getUser(), oldUser); + final String password = keepIfUnchanged(newNotification.getPassword(), oldPassword); + final String proxyUser = keepIfUnchanged(newNotification.getProxyUser(), oldProxyUser); + final String proxyPassword = + keepIfUnchanged(newNotification.getProxyPassword(), oldProxyPassword); + newNotification = + prnfbNotificationBuilder(newNotification) // + .withUser(user) // + .withPassword(password) // + .withProxyUser(proxyUser) // + .withProxyPassword(proxyPassword) // + .build(); + if (oldNotification.isPresent()) { - String user = keepIfUnchanged(newNotification.getUser(), oldNotification.get().getUser()); - String password = - keepIfUnchanged(newNotification.getPassword(), oldNotification.get().getPassword()); - String proxyUser = - keepIfUnchanged(newNotification.getProxyUser(), oldNotification.get().getProxyUser()); - String proxyPassword = - keepIfUnchanged( - newNotification.getProxyPassword(), oldNotification.get().getProxyPassword()); - newNotification = - prnfbNotificationBuilder(newNotification) // - .withUser(user) // - .withPassword(password) // - .withProxyUser(proxyUser) // - .withProxyPassword(proxyPassword) // - .build(); - doDeleteNotification(newNotification.getUuid()); + doDeleteNotification(notificationUuid); } - PrnfbSettings originalSettings = doGetPrnfbSettings(); - PrnfbSettings updated = + final PrnfbSettings originalSettings = doGetPrnfbSettings(); + final PrnfbSettings updated = prnfbSettingsBuilder(originalSettings) // .withNotification(newNotification) // .build(); @@ -267,7 +278,7 @@ private PrnfbNotification doAddOrUpdateNotification(PrnfbNotification newNotific } private String keepIfUnchanged(Optional newValue, Optional oldValue) { - boolean isUnchanged = newValue.isPresent() && newValue.get().equals(UNCHANGED); + final boolean isUnchanged = newValue.isPresent() && newValue.get().equals(UNCHANGED); if (isUnchanged) { return oldValue.orNull(); } @@ -275,10 +286,10 @@ private String keepIfUnchanged(Optional newValue, Optional oldVa } private void doDeleteButton(UUID uuid) { - PrnfbSettings originalSettings = doGetPrnfbSettings(); - List keep = + final PrnfbSettings originalSettings = doGetPrnfbSettings(); + final List keep = newArrayList(filter(originalSettings.getButtons(), not(withUuid(uuid)))); - PrnfbSettings withoutDeleted = + final PrnfbSettings withoutDeleted = prnfbSettingsBuilder(originalSettings) // .setButtons(keep) // .build(); @@ -286,10 +297,10 @@ private void doDeleteButton(UUID uuid) { } private void doDeleteNotification(UUID uuid) { - PrnfbSettings originalSettings = doGetPrnfbSettings(); - List keep = + final PrnfbSettings originalSettings = doGetPrnfbSettings(); + final List keep = newArrayList(filter(originalSettings.getNotifications(), not(withUuid(uuid)))); - PrnfbSettings withoutDeleted = + final PrnfbSettings withoutDeleted = prnfbSettingsBuilder(originalSettings) // .setNotifications(keep) // .build(); @@ -307,12 +318,12 @@ private PrnfbSettings doGetPrnfbSettings() { != null) { try { this.logger.info("Using legacy settings."); - se.bjurr.prnfb.settings.legacy.PrnfbSettings legacySettings = + final se.bjurr.prnfb.settings.legacy.PrnfbSettings legacySettings = SettingsStorage.getPrnfbSettings(this.pluginSettings); - PrnfbSettings fromLegacy = settingsFromLegacy(legacySettings); + final PrnfbSettings fromLegacy = settingsFromLegacy(legacySettings); doSetPrnfbSettings(fromLegacy); storedSettings = this.pluginSettings.get(STORAGE_KEY); - } catch (Exception e) { + } catch (final Exception e) { this.logger.error("", e); } } else { @@ -329,23 +340,23 @@ private PrnfbSettings doGetPrnfbSettings() { } private void doSetPrnfbSettings(PrnfbSettings newSettings) { - PrnfbSettingsData oldSettingsData = doGetPrnfbSettings().getPrnfbSettingsData(); - PrnfbSettingsData newSettingsData = newSettings.getPrnfbSettingsData(); - String keyStorePassword = + final PrnfbSettingsData oldSettingsData = doGetPrnfbSettings().getPrnfbSettingsData(); + final PrnfbSettingsData newSettingsData = newSettings.getPrnfbSettingsData(); + final String keyStorePassword = keepIfUnchanged( newSettingsData.getKeyStorePassword(), oldSettingsData.getKeyStorePassword()); - PrnfbSettingsData adjustedSettingsData = + final PrnfbSettingsData adjustedSettingsData = prnfbSettingsDataBuilder(newSettingsData) // .setKeyStorePassword(keyStorePassword) // .build(); - PrnfbSettings adjustedSettings = + final PrnfbSettings adjustedSettings = prnfbSettingsBuilder(newSettings) // .setPrnfbSettingsData(adjustedSettingsData) // .build(); - String data = gson.toJson(adjustedSettings); + final String data = gson.toJson(adjustedSettings); this.pluginSettings.put(STORAGE_KEY, data); } @@ -363,9 +374,9 @@ public T perform() throws RuntimeException { private PrnfbSettings settingsFromLegacy( se.bjurr.prnfb.settings.legacy.PrnfbSettings oldSettings) { - String ks = oldSettings.getKeyStore().orNull(); - String ksp = oldSettings.getKeyStorePassword().orNull(); - String kst = oldSettings.getKeyStoreType(); + final String ks = oldSettings.getKeyStore().orNull(); + final String ksp = oldSettings.getKeyStorePassword().orNull(); + final String kst = oldSettings.getKeyStoreType(); USER_LEVEL adminRestr = USER_LEVEL.SYSTEM_ADMIN; if (oldSettings.isAdminsAllowed()) { adminRestr = USER_LEVEL.ADMIN; @@ -374,10 +385,10 @@ private PrnfbSettings settingsFromLegacy( adminRestr = USER_LEVEL.EVERYONE; } - boolean shouldAcceptAnyCertificate = false; + final boolean shouldAcceptAnyCertificate = false; - List newButtons = newArrayList(); - for (se.bjurr.prnfb.settings.legacy.PrnfbButton oldButton : oldSettings.getButtons()) { + final List newButtons = newArrayList(); + for (final se.bjurr.prnfb.settings.legacy.PrnfbButton oldButton : oldSettings.getButtons()) { USER_LEVEL userLevel = USER_LEVEL.SYSTEM_ADMIN; if (oldButton.getVisibility() == BUTTON_VISIBILITY.ADMIN) { userLevel = USER_LEVEL.ADMIN; @@ -397,11 +408,11 @@ private PrnfbSettings settingsFromLegacy( null)); } - List newNotifications = newArrayList(); - for (se.bjurr.prnfb.settings.legacy.PrnfbNotification oldNotification : + final List newNotifications = newArrayList(); + for (final se.bjurr.prnfb.settings.legacy.PrnfbNotification oldNotification : oldSettings.getNotifications()) { try { - PrnfbNotificationBuilder builder = + final PrnfbNotificationBuilder builder = prnfbNotificationBuilder() // .withFilterRegexp(oldNotification.getFilterRegexp().orNull()) // .withFilterString(oldNotification.getFilterString().orNull()) // @@ -420,20 +431,20 @@ private PrnfbSettings settingsFromLegacy( .withUrl(oldNotification.getUrl()) // .withUser(oldNotification.getUser().orNull()); - for (Header h : oldNotification.getHeaders()) { + for (final Header h : oldNotification.getHeaders()) { builder.withHeader(h.getName(), h.getValue()); } - for (PullRequestState t : oldNotification.getTriggerIgnoreStateList()) { + for (final PullRequestState t : oldNotification.getTriggerIgnoreStateList()) { builder.withTriggerIgnoreState(t); } - for (PrnfbPullRequestAction t : oldNotification.getTriggers()) { + for (final PrnfbPullRequestAction t : oldNotification.getTriggers()) { builder.withTrigger(t); } newNotifications.add(builder.build()); - } catch (ValidationException e) { + } catch (final ValidationException e) { this.logger.error("", e); } } diff --git a/src/main/java/se/bjurr/prnfb/settings/PrnfbNotification.java b/src/main/java/se/bjurr/prnfb/settings/PrnfbNotification.java index d0d407f..2436287 100644 --- a/src/main/java/se/bjurr/prnfb/settings/PrnfbNotification.java +++ b/src/main/java/se/bjurr/prnfb/settings/PrnfbNotification.java @@ -8,6 +8,7 @@ import static java.util.UUID.randomUUID; import static java.util.regex.Pattern.compile; import static se.bjurr.prnfb.http.UrlInvoker.HTTP_METHOD.GET; +import static se.bjurr.prnfb.service.PrnfbRenderer.ENCODE_FOR.NONE; import static se.bjurr.prnfb.settings.TRIGGER_IF_MERGE.ALWAYS; import java.net.URL; @@ -92,7 +93,7 @@ public PrnfbNotification(PrnfbNotificationBuilder builder) throws ValidationExce this.injectionUrl = emptyToNull(nullToEmpty(builder.getInjectionUrl()).trim()); this.injectionUrlRegexp = emptyToNull(nullToEmpty(builder.getInjectionUrlRegexp()).trim()); this.triggerIgnoreStateList = builder.getTriggerIgnoreStateList(); - this.postContentEncoding = builder.getPostContentEncoding(); + this.postContentEncoding = firstNonNull(builder.getPostContentEncoding(), NONE); } @Override diff --git a/src/test/java/se/bjurr/prnfb/service/SettingsServiceTest.java b/src/test/java/se/bjurr/prnfb/service/SettingsServiceTest.java index dee70fd..bab111f 100644 --- a/src/test/java/se/bjurr/prnfb/service/SettingsServiceTest.java +++ b/src/test/java/se/bjurr/prnfb/service/SettingsServiceTest.java @@ -72,7 +72,7 @@ public T execute(TransactionCallback action) { @Test public void testThatButtonCanBeAddedUpdatedAndDeleted() { - PrnfbButton button1 = + final PrnfbButton button1 = new PrnfbButton( null, "title", EVERYONE, ON_OR_OFF.off, "p1", "r1", "confirmationText", null); assertThat(this.sut.getButtons()) // @@ -82,14 +82,14 @@ public void testThatButtonCanBeAddedUpdatedAndDeleted() { assertThat(this.sut.getButtons()) // .containsExactly(button1); - PrnfbButton button2 = + final PrnfbButton button2 = new PrnfbButton( null, "title", EVERYONE, ON_OR_OFF.off, "p1", "r1", "confirmationText", null); this.sut.addOrUpdateButton(button2); assertThat(this.sut.getButtons()) // .containsExactly(button1, button2); - PrnfbButton updated = + final PrnfbButton updated = new PrnfbButton( button1.getUuid(), "title2", @@ -107,7 +107,7 @@ public void testThatButtonCanBeAddedUpdatedAndDeleted() { assertThat(this.sut.getButtons()) // .containsExactly(button2); - PrnfbButton b2 = this.sut.getButton(button2.getUuid()); + final PrnfbButton b2 = this.sut.getButton(button2.getUuid()); assertThat(b2) // .isEqualTo(button2); assertThat(b2.hashCode()) // @@ -118,10 +118,10 @@ public void testThatButtonCanBeAddedUpdatedAndDeleted() { @Test public void testThatButtonsCanBeRetrievedByProject() { - PrnfbButton button1 = populatedInstanceOf(PrnfbButton.class); + final PrnfbButton button1 = populatedInstanceOf(PrnfbButton.class); this.sut.addOrUpdateButton(button1); - List actual = this.sut.getButtons(button1.getProjectKey().get()); + final List actual = this.sut.getButtons(button1.getProjectKey().get()); assertThat(actual) // .containsOnly(button1); @@ -129,10 +129,10 @@ public void testThatButtonsCanBeRetrievedByProject() { @Test public void testThatButtonsCanBeRetrievedByProjectAndRepo() { - PrnfbButton button1 = populatedInstanceOf(PrnfbButton.class); + final PrnfbButton button1 = populatedInstanceOf(PrnfbButton.class); this.sut.addOrUpdateButton(button1); - List actual = + final List actual = this.sut.getButtons(button1.getProjectKey().get(), button1.getRepositorySlug().get()); assertThat(actual) // @@ -148,7 +148,7 @@ public void testThatNotificationCanBeAddedUpdatedAndDeleted() throws ValidationE assertThat(this.sut.getNotifications()) // .containsExactly(this.notification1); - PrnfbNotification notification2 = + final PrnfbNotification notification2 = prnfbNotificationBuilder() // .withUrl("http://hej.com/") // .withTrigger(APPROVED) // @@ -157,7 +157,7 @@ public void testThatNotificationCanBeAddedUpdatedAndDeleted() throws ValidationE assertThat(this.sut.getNotifications()) // .containsExactly(this.notification1, notification2); - PrnfbNotification updated = + final PrnfbNotification updated = prnfbNotificationBuilder() // .withUuid(this.notification1.getUuid()) // .withUrl("http://hej2.com/") // @@ -185,7 +185,7 @@ public void testThatNotificationCanBeAddedUpdatedAndDeleted() throws ValidationE public void testThatNotificationsCanBeRetrievedByProject() throws ValidationException { this.sut.addOrUpdateNotification(this.notification1); - List actual = + final List actual = this.sut.getNotifications(this.notification1.getProjectKey().orNull()); assertThat(actual) // @@ -196,20 +196,22 @@ public void testThatNotificationsCanBeRetrievedByProject() throws ValidationExce public void testThatNotificationsCanBeRetrievedByProjectAndRepo() throws ValidationException { this.sut.addOrUpdateNotification(this.notification1); - List actual = + final List actual = this.sut.getNotifications( this.notification1.getProjectKey().orNull(), this.notification1.getRepositorySlug().orNull()); assertThat(actual) // - .containsOnly(this.notification1); + .hasSize(1); + assertThat(actual.get(0)) // + .isEqualTo(this.notification1); } @Test public void testThatNotificationsCanBeRetrievedByUuid() throws ValidationException { this.sut.addOrUpdateNotification(this.notification1); - PrnfbNotification actual = this.sut.getNotification(this.notification1.getUuid()); + final PrnfbNotification actual = this.sut.getNotification(this.notification1.getUuid()); assertThat(actual) // .isEqualTo(this.notification1); @@ -217,7 +219,7 @@ public void testThatNotificationsCanBeRetrievedByUuid() throws ValidationExcepti @Test public void testThatPluginSettingsDataCanBeStored() { - PrnfbSettings oldSettings = + final PrnfbSettings oldSettings = prnfbSettingsBuilder() // .setPrnfbSettingsData( // prnfbSettingsDataBuilder() // @@ -229,11 +231,11 @@ public void testThatPluginSettingsDataCanBeStored() { .build() // ) // .build(); - String oldSettingsString = new Gson().toJson(oldSettings); + final String oldSettingsString = new Gson().toJson(oldSettings); this.pluginSettings.getPluginSettingsMap().put(STORAGE_KEY, oldSettingsString); - PrnfbSettings newSettings = + final PrnfbSettings newSettings = prnfbSettingsBuilder() // .setPrnfbSettingsData( // prnfbSettingsDataBuilder() // @@ -248,14 +250,14 @@ public void testThatPluginSettingsDataCanBeStored() { this.sut.setPrnfbSettingsData(newSettings.getPrnfbSettingsData()); - String expectedSettingsString = new Gson().toJson(newSettings); + final String expectedSettingsString = new Gson().toJson(newSettings); assertThat(this.pluginSettings.getPluginSettingsMap().get(STORAGE_KEY)) // .isEqualTo(expectedSettingsString); } @Test public void testThatSettingsCanBeRead() { - PrnfbSettings oldSettings = + final PrnfbSettings oldSettings = prnfbSettingsBuilder() // .setPrnfbSettingsData( // prnfbSettingsDataBuilder() // @@ -267,10 +269,10 @@ public void testThatSettingsCanBeRead() { .build() // ) // .build(); - String oldSettingsString = new Gson().toJson(oldSettings); + final String oldSettingsString = new Gson().toJson(oldSettings); this.pluginSettings.getPluginSettingsMap().put(STORAGE_KEY, oldSettingsString); - PrnfbSettings actual = this.sut.getPrnfbSettings(); + final PrnfbSettings actual = this.sut.getPrnfbSettings(); assertThat(actual) // .isEqualTo(oldSettings); @@ -291,7 +293,7 @@ public void testThatSettingsCanBeRead() { public void testThatSettingsCanBeReadWhenNoneAreSaved() { this.pluginSettings.getPluginSettingsMap().put(STORAGE_KEY, null); - PrnfbSettings actual = this.sut.getPrnfbSettings(); + final PrnfbSettings actual = this.sut.getPrnfbSettings(); assertThat(actual) // .isNotNull(); }