From 9c233c19263ed3cfc14ca10ed68176711c45f69b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 12 Oct 2020 20:32:53 +0200 Subject: [PATCH 01/17] Add fallback mail host to global config --- .../sonia/scm/config/ScmConfiguration.java | 28 +++++++++++++++++++ scm-ui/ui-types/src/Config.ts | 1 + .../ui-webapp/public/locales/de/config.json | 2 ++ .../ui-webapp/public/locales/en/config.json | 2 ++ .../src/admin/components/form/ConfigForm.tsx | 1 + .../admin/components/form/GeneralSettings.tsx | 16 ++++++++++- .../sonia/scm/api/v2/resources/ConfigDto.java | 3 +- ...ConfigDtoToScmConfigurationMapperTest.java | 2 ++ ...ScmConfigurationToConfigDtoMapperTest.java | 1 + 9 files changed, 54 insertions(+), 2 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java b/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java index 7bb64a9454..fe67e43820 100644 --- a/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java +++ b/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java @@ -80,6 +80,12 @@ public class ScmConfiguration implements Configuration { */ public static final String DEFAULT_LOGIN_INFO_URL = "https://login-info.scm-manager.org/api/v1/login-info"; + /** + * Default e-mail host that will be used whenever we have to generate an e-mail address for a user that has no mail + * address configured. + */ + public static final String DEFAULT_MAIL_HOST = "scm-manager.local"; + /** * Default plugin url from version 1.0 */ @@ -187,6 +193,8 @@ public class ScmConfiguration implements Configuration { @XmlElement(name = "login-info-url") private String loginInfoUrl = DEFAULT_LOGIN_INFO_URL; + @XmlElement(name = "mail-host") + private String mailHost = DEFAULT_MAIL_HOST; /** * Calls the {@link sonia.scm.ConfigChangedListener#configChanged(Object)} @@ -227,6 +235,7 @@ public class ScmConfiguration implements Configuration { this.namespaceStrategy = other.namespaceStrategy; this.loginInfoUrl = other.loginInfoUrl; this.releaseFeedUrl = other.releaseFeedUrl; + this.mailHost = other.mailHost; } /** @@ -291,6 +300,15 @@ public class ScmConfiguration implements Configuration { return releaseFeedUrl; } + /** + * Returns the mail host, that will be used to create e-mail addresses for users without one whenever one is required. + * + * @since 2.8.0 + */ + public String getMailHost() { + return mailHost; + } + /** * Returns a set of glob patterns for urls which should excluded from * proxy settings. @@ -471,6 +489,16 @@ public class ScmConfiguration implements Configuration { this.releaseFeedUrl = releaseFeedUrl; } + /** + * Sets the mail host, that will be used to create e-mail addresses for users without one whenever one is required. + * + * @param mailHost The new mail host to use. + * @since 2.8.0 + */ + public void setMailHost(String mailHost) { + this.mailHost = mailHost; + } + /** * Set glob patterns for urls which are should be excluded from proxy * settings. diff --git a/scm-ui/ui-types/src/Config.ts b/scm-ui/ui-types/src/Config.ts index 667aa7b199..4369b98e6e 100644 --- a/scm-ui/ui-types/src/Config.ts +++ b/scm-ui/ui-types/src/Config.ts @@ -48,5 +48,6 @@ export type Config = { namespaceStrategy: string; loginInfoUrl: string; releaseFeedUrl: string; + mailHost: string; _links: Links; }; diff --git a/scm-ui/ui-webapp/public/locales/de/config.json b/scm-ui/ui-webapp/public/locales/de/config.json index ad749c12c8..9ab2d38675 100644 --- a/scm-ui/ui-webapp/public/locales/de/config.json +++ b/scm-ui/ui-webapp/public/locales/de/config.json @@ -47,6 +47,7 @@ "skip-failed-authenticators": "Fehlgeschlagene Authentifizierer überspringen", "plugin-url": "Plugin Center URL", "release-feed-url": "Release Feed URL", + "mail-host": "Fallback Mail Host", "enabled-xsrf-protection": "XSRF Protection aktivieren", "namespace-strategy": "Namespace Strategie", "login-info-url": "Login Info URL" @@ -62,6 +63,7 @@ "dateFormatHelpText": "Moments Datumsformat. Zulässige Formate sind in der MomentJS Dokumentation beschrieben.", "pluginUrlHelpText": "Die URL der Plugin Center API. Beschreibung der Platzhalter: version = SCM-Manager Version; os = Betriebssystem; arch = Architektur", "releaseFeedUrlHelpText": "Die URL des RSS Release Feed des SCM-Manager. Darüber wird über die neue SCM-Manager Version informiert. Um diese Funktion zu deaktivieren lassen Sie dieses Feld leer.", + "mailHostHelpText": "Dieser Host Name wird genutzt, wenn für einen User eine E-Mail-Adresse benötigt wird, für den keine hinterlegt ist. Dieser Host wird nicht zum Versenden von E-Mails genutzt und auch keine anderweitige Verbindung aufgebaut.", "enableForwardingHelpText": "mod_proxy Port Weiterleitung aktivieren.", "disableGroupingGridHelpText": "Repository Gruppen deaktivieren. Nach einer Änderung an dieser Einstellung muss die Seite komplett neu geladen werden.", "allowAnonymousAccessHelpText": "Anonyme Benutzer haben Zugriff auf freigegebene Repositories.", diff --git a/scm-ui/ui-webapp/public/locales/en/config.json b/scm-ui/ui-webapp/public/locales/en/config.json index 1150dd4e3d..12d38694a5 100644 --- a/scm-ui/ui-webapp/public/locales/en/config.json +++ b/scm-ui/ui-webapp/public/locales/en/config.json @@ -47,6 +47,7 @@ "skip-failed-authenticators": "Skip Failed Authenticators", "plugin-url": "Plugin Center URL", "release-feed-url": "Release Feed URL", + "mail-host": "Fallback Mail Host", "enabled-xsrf-protection": "Enabled XSRF Protection", "namespace-strategy": "Namespace Strategy", "login-info-url": "Login Info URL" @@ -62,6 +63,7 @@ "dateFormatHelpText": "Moments date format. Please have a look at the MomentJS documentation.", "pluginUrlHelpText": "The url of the Plugin Center API. Explanation of the placeholders: version = SCM-Manager Version; os = Operation System; arch = Architecture", "releaseFeedUrlHelpText": "The url of the RSS Release Feed for SCM-Manager. This provides up-to-date version information. To disable this feature just leave the url blank.", + "mailHostHelpText": "This host name will be used to create email addresses for users when needed and they have none configured. It will not be used to send mails nor will be accessed otherwise.", "enableForwardingHelpText": "Enable mod_proxy port forwarding.", "disableGroupingGridHelpText": "Disable repository Groups. A complete page reload is required after a change of this value.", "allowAnonymousAccessHelpText": "Anonymous users have access on granted repositories.", diff --git a/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx b/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx index f412acf8e8..b38113831f 100644 --- a/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx +++ b/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx @@ -144,6 +144,7 @@ class ConfigForm extends React.Component { skipFailedAuthenticators={config.skipFailedAuthenticators} pluginUrl={config.pluginUrl} releaseFeedUrl={config.releaseFeedUrl} + mailHost={config.mailHost} enabledXsrfProtection={config.enabledXsrfProtection} namespaceStrategy={config.namespaceStrategy} onChange={(isValid, changedValue, name) => this.onChange(isValid, changedValue, name)} diff --git a/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx b/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx index 580a3546da..49871ed2aa 100644 --- a/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx +++ b/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx @@ -36,6 +36,7 @@ type Props = WithTranslation & { skipFailedAuthenticators: boolean; pluginUrl: string; releaseFeedUrl: string; + mailHost: string; enabledXsrfProtection: boolean; namespaceStrategy: string; namespaceStrategies?: NamespaceStrategies; @@ -51,6 +52,7 @@ class GeneralSettings extends React.Component { loginInfoUrl, pluginUrl, releaseFeedUrl, + mailHost, enabledXsrfProtection, anonymousMode, namespaceStrategy, @@ -129,7 +131,7 @@ class GeneralSettings extends React.Component {
-
+
{ helpText={t("help.releaseFeedUrlHelpText")} />
+
+ +
); @@ -164,6 +175,9 @@ class GeneralSettings extends React.Component { handleReleaseFeedUrlChange = (value: string) => { this.props.onChange(true, value, "releaseFeedUrl"); }; + handleMailHostChange = (value: string) => { + this.props.onChange(true, value, "mailHost"); + }; } export default withTranslation("config")(GeneralSettings); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java index bec72a552b..42ae1ca6f0 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import de.otto.edison.hal.HalRepresentation; @@ -59,6 +59,7 @@ public class ConfigDto extends HalRepresentation { private String namespaceStrategy; private String loginInfoUrl; private String releaseFeedUrl; + private String mailHost; @Override @SuppressWarnings("squid:S1185") // We want to have this method available in this package diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java index 3e01a1e4cc..e693d78cea 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java @@ -75,6 +75,7 @@ public class ConfigDtoToScmConfigurationMapperTest { assertTrue(config.isEnabledXsrfProtection()); assertEquals("username", config.getNamespaceStrategy()); assertEquals("https://scm-manager.org/login-info", config.getLoginInfoUrl()); + assertEquals("hitchhiker.mail", config.getMailHost()); } @Test @@ -113,6 +114,7 @@ public class ConfigDtoToScmConfigurationMapperTest { configDto.setEnabledXsrfProtection(true); configDto.setNamespaceStrategy("username"); configDto.setLoginInfoUrl("https://scm-manager.org/login-info"); + configDto.setMailHost("hitchhiker.mail"); return configDto; } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java index cef98c4062..42bf6e7e3b 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java @@ -106,6 +106,7 @@ public class ScmConfigurationToConfigDtoMapperTest { assertEquals("username", dto.getNamespaceStrategy()); assertEquals("https://scm-manager.org/login-info", dto.getLoginInfoUrl()); assertEquals("https://www.scm-manager.org/download/rss.xml", dto.getReleaseFeedUrl()); + assertEquals("scm-manager.local", dto.getMailHost()); assertEquals(expectedBaseUri.toString(), dto.getLinks().getLinkBy("self").get().getHref()); assertEquals(expectedBaseUri.toString(), dto.getLinks().getLinkBy("update").get().getHref()); From 343d0b942544cc1e74ed5ec151c6db4abdd99bc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 12 Oct 2020 20:48:17 +0200 Subject: [PATCH 02/17] Create util class to get mail address --- .../src/main/java/sonia/scm/user/EMail.java | 52 ++++++++++++++++ .../test/java/sonia/scm/user/EMailTest.java | 62 +++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 scm-core/src/main/java/sonia/scm/user/EMail.java create mode 100644 scm-core/src/test/java/sonia/scm/user/EMailTest.java diff --git a/scm-core/src/main/java/sonia/scm/user/EMail.java b/scm-core/src/main/java/sonia/scm/user/EMail.java new file mode 100644 index 0000000000..121825157c --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/user/EMail.java @@ -0,0 +1,52 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.user; + +import com.google.common.base.Strings; +import sonia.scm.config.ScmConfiguration; + +import javax.inject.Inject; + +public class EMail { + + private final ScmConfiguration scmConfiguration; + + @Inject + public EMail(ScmConfiguration scmConfiguration) { + this.scmConfiguration = scmConfiguration; + } + + public String createFallbackMailAddress(User user) { + if (Strings.isNullOrEmpty(user.getMail())) { + if (user.getId().contains("@")) { + return user.getId(); + } else { + return user.getId() + "@" + scmConfiguration.getMailHost(); + } + } else { + return user.getMail(); + } + } +} diff --git a/scm-core/src/test/java/sonia/scm/user/EMailTest.java b/scm-core/src/test/java/sonia/scm/user/EMailTest.java new file mode 100644 index 0000000000..21f6181757 --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/user/EMailTest.java @@ -0,0 +1,62 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.user; + +import org.junit.jupiter.api.Test; +import sonia.scm.config.ScmConfiguration; + +import static org.assertj.core.api.Assertions.assertThat; + +class EMailTest { + + EMail eMail = new EMail(new ScmConfiguration()); + + @Test + void shouldUserUsersAddressIfAvailable() { + User user = new User("dent", "Arthur Dent", "arthur@hitchhiker.com"); + + String mailAddress = eMail.createFallbackMailAddress(user); + + assertThat(mailAddress).isEqualTo("arthur@hitchhiker.com"); + } + + @Test + void shouldCreateAddressIfNoneAvailable() { + User user = new User("dent", "Arthur Dent", ""); + + String mailAddress = eMail.createFallbackMailAddress(user); + + assertThat(mailAddress).isEqualTo("dent@scm-manager.local"); + } + + @Test + void shouldUserUsersIdIfItLooksLikeAnMailAddress() { + User user = new User("dent@hitchhiker.com", "Arthur Dent", ""); + + String mailAddress = eMail.createFallbackMailAddress(user); + + assertThat(mailAddress).isEqualTo("dent@hitchhiker.com"); + } +} From 7fc7e33c0cf22a6db61a998b215acf5d021a6db1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 13 Oct 2020 08:51:48 +0200 Subject: [PATCH 03/17] Add fallback email to me dto --- scm-ui/ui-types/src/Me.ts | 3 ++- .../sonia/scm/api/v2/resources/MeDto.java | 6 ++++- .../scm/api/v2/resources/MeDtoFactory.java | 13 ++++++++++- .../api/v2/resources/MeDtoFactoryTest.java | 22 ++++++++++++++++--- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/scm-ui/ui-types/src/Me.ts b/scm-ui/ui-types/src/Me.ts index 38a3a23278..d8595e0771 100644 --- a/scm-ui/ui-types/src/Me.ts +++ b/scm-ui/ui-types/src/Me.ts @@ -27,7 +27,8 @@ import { Links } from "./hal"; export type Me = { name: string; displayName: string; - mail: string; + mail?: string; + fallbackMail?: string; groups: string[]; _links: Links; }; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java index 968d892536..b4e011f7bb 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java @@ -21,9 +21,10 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; +import com.fasterxml.jackson.annotation.JsonInclude; import de.otto.edison.hal.Embedded; import de.otto.edison.hal.HalRepresentation; import de.otto.edison.hal.Links; @@ -40,7 +41,10 @@ public class MeDto extends HalRepresentation { private String name; private String displayName; + @JsonInclude(JsonInclude.Include.NON_NULL) private String mail; + @JsonInclude(JsonInclude.Include.NON_NULL) + private String fallbackMail; private Set groups; MeDto(Links links, Embedded embedded) { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java index 2101ba2ffb..2bc72d509a 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java @@ -24,12 +24,14 @@ package sonia.scm.api.v2.resources; +import com.google.common.base.Strings; import de.otto.edison.hal.Embedded; import de.otto.edison.hal.Links; import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.Subject; import sonia.scm.group.GroupCollector; +import sonia.scm.user.EMail; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.user.UserPermissions; @@ -46,12 +48,14 @@ public class MeDtoFactory extends HalAppenderMapper { private final ResourceLinks resourceLinks; private final UserManager userManager; private final GroupCollector groupCollector; + private final EMail eMail; @Inject - public MeDtoFactory(ResourceLinks resourceLinks, UserManager userManager, GroupCollector groupCollector) { + public MeDtoFactory(ResourceLinks resourceLinks, UserManager userManager, GroupCollector groupCollector, EMail eMail) { this.resourceLinks = resourceLinks; this.userManager = userManager; this.groupCollector = groupCollector; + this.eMail = eMail; } public MeDto create() { @@ -61,6 +65,7 @@ public class MeDtoFactory extends HalAppenderMapper { MeDto dto = createDto(user); mapUserProperties(user, dto); mapGroups(user, dto); + setGeneratedMail(user, dto); return dto; } @@ -79,6 +84,12 @@ public class MeDtoFactory extends HalAppenderMapper { return subject.getPrincipals(); } + private void setGeneratedMail(User user, MeDto dto) { + if (Strings.isNullOrEmpty(user.getMail())) { + dto.setFallbackMail(eMail.createFallbackMailAddress(user)); + } + } + private MeDto createDto(User user) { Links.Builder linksBuilder = linkingTo().self(resourceLinks.me().self()); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java index 901d625579..2a4cea9626 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import com.google.common.collect.ImmutableSet; @@ -38,9 +38,9 @@ import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; import sonia.scm.SCMContext; import sonia.scm.group.GroupCollector; +import sonia.scm.user.EMail; import sonia.scm.user.User; import sonia.scm.user.UserManager; -import sonia.scm.user.UserPermissions; import sonia.scm.user.UserTestData; import java.net.URI; @@ -65,13 +65,16 @@ class MeDtoFactoryTest { @Mock private Subject subject; + @Mock + private EMail eMail; + private MeDtoFactory meDtoFactory; @BeforeEach void setUpContext() { ThreadContext.bind(subject); ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri); - meDtoFactory = new MeDtoFactory(resourceLinks, userManager, groupCollector); + meDtoFactory = new MeDtoFactory(resourceLinks, userManager, groupCollector, eMail); } @AfterEach @@ -235,4 +238,17 @@ class MeDtoFactoryTest { MeDto dto = meDtoFactory.create(); assertThat(dto.getLinks().getLinkBy("profile").get().getHref()).isEqualTo("http://hitchhiker.com/users/trillian"); } + + @Test + void shouldUserGeneratedMailOnlyWhenUserHasNone() { + User user = UserTestData.createTrillian(); + user.setMail(null); + prepareSubject(user); + when(eMail.createFallbackMailAddress(user)).thenReturn("trillian@hitchhiker.local"); + + MeDto dto = meDtoFactory.create(); + + assertThat(dto.getMail()).isNull(); + assertThat(dto.getFallbackMail()).isEqualTo("trillian@hitchhiker.local"); + } } From bb2b8450157e953ce61a8d2e288e824c341d80ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 13 Oct 2020 16:27:24 +0200 Subject: [PATCH 04/17] Use email util to compute fallback mail address --- .../repository/api/MergeCommandBuilder.java | 8 +- .../repository/api/ModifyCommandBuilder.java | 8 +- .../scm/repository/api/RepositoryService.java | 14 +- .../api/RepositoryServiceFactory.java | 170 +++++----- .../sonia/scm/repository/util/AuthorUtil.java | 13 +- .../api/ModifyCommandBuilderTest.java | 315 +++++++++++------- .../api/RepositoryServiceFactoryTest.java | 9 +- .../repository/api/RepositoryServiceTest.java | 16 +- 8 files changed, 315 insertions(+), 238 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java index 06a3b489a1..a751832d8f 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java @@ -30,6 +30,7 @@ import sonia.scm.repository.spi.MergeCommand; import sonia.scm.repository.spi.MergeCommandRequest; import sonia.scm.repository.spi.MergeConflictResult; import sonia.scm.repository.util.AuthorUtil; +import sonia.scm.user.EMail; import java.util.Set; @@ -75,11 +76,14 @@ import java.util.Set; */ public class MergeCommandBuilder { + private final EMail eMail; + private final MergeCommand mergeCommand; private final MergeCommandRequest request = new MergeCommandRequest(); - MergeCommandBuilder(MergeCommand mergeCommand) { + MergeCommandBuilder(MergeCommand mergeCommand, EMail eMail) { this.mergeCommand = mergeCommand; + this.eMail = eMail; } /** @@ -209,7 +213,7 @@ public class MergeCommandBuilder { * @return The result of the merge. */ public MergeCommandResult executeMerge() { - AuthorUtil.setAuthorIfNotAvailable(request); + AuthorUtil.setAuthorIfNotAvailable(request, eMail); Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); return mergeCommand.merge(request); } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java index aeccb24b99..795de5d43d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java @@ -35,6 +35,7 @@ import sonia.scm.repository.spi.ModifyCommand; import sonia.scm.repository.spi.ModifyCommandRequest; import sonia.scm.repository.util.AuthorUtil; import sonia.scm.repository.work.WorkdirProvider; +import sonia.scm.user.EMail; import sonia.scm.util.IOUtil; import java.io.File; @@ -72,14 +73,17 @@ public class ModifyCommandBuilder { private static final Logger LOG = LoggerFactory.getLogger(ModifyCommandBuilder.class); + private final EMail eMail; + private final ModifyCommand command; private final File workdir; private final ModifyCommandRequest request = new ModifyCommandRequest(); - ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider) { + ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider, EMail eMail) { this.command = command; this.workdir = workdirProvider.createNewWorkdir(); + this.eMail = eMail; } /** @@ -124,7 +128,7 @@ public class ModifyCommandBuilder { * @return The revision of the new commit. */ public String execute() { - AuthorUtil.setAuthorIfNotAvailable(request); + AuthorUtil.setAuthorIfNotAvailable(request, eMail); try { Preconditions.checkArgument(request.isValid(), "commit message and at least one request are required"); return command.execute(request); diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java index 15f84a4ecd..029dc6fa34 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java @@ -34,6 +34,7 @@ import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.spi.RepositoryServiceProvider; import sonia.scm.repository.work.WorkdirProvider; +import sonia.scm.user.EMail; import java.io.Closeable; import java.io.IOException; @@ -87,6 +88,7 @@ public final class RepositoryService implements Closeable { @SuppressWarnings("rawtypes") private final Set protocolProviders; private final WorkdirProvider workdirProvider; + private final EMail eMail; /** * Constructs a new {@link RepositoryService}. This constructor should only @@ -94,20 +96,22 @@ public final class RepositoryService implements Closeable { * @param cacheManager cache manager * @param provider implementation for {@link RepositoryServiceProvider} * @param repository the repository - * @param workdirProvider + * @param workdirProvider provider for workdirs + * @param eMail utility to compute email addresses if missing */ RepositoryService(CacheManager cacheManager, RepositoryServiceProvider provider, Repository repository, PreProcessorUtil preProcessorUtil, @SuppressWarnings("rawtypes") Set protocolProviders, - WorkdirProvider workdirProvider - ) { + WorkdirProvider workdirProvider, + EMail eMail) { this.cacheManager = cacheManager; this.provider = provider; this.repository = repository; this.preProcessorUtil = preProcessorUtil; this.protocolProviders = protocolProviders; this.workdirProvider = workdirProvider; + this.eMail = eMail; } /** @@ -397,7 +401,7 @@ public final class RepositoryService implements Closeable { LOG.debug("create merge command for repository {}", repository.getNamespaceAndName()); - return new MergeCommandBuilder(provider.getMergeCommand()); + return new MergeCommandBuilder(provider.getMergeCommand(), eMail); } /** @@ -418,7 +422,7 @@ public final class RepositoryService implements Closeable { LOG.debug("create modify command for repository {}", repository.getNamespaceAndName()); - return new ModifyCommandBuilder(provider.getModifyCommand(), workdirProvider); + return new ModifyCommandBuilder(provider.getModifyCommand(), workdirProvider, eMail); } /** diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java index b030d1cb0c..15cec9d210 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java @@ -40,9 +40,7 @@ import sonia.scm.HandlerEventType; import sonia.scm.NotFoundException; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; -import sonia.scm.config.ScmConfiguration; import sonia.scm.event.ScmEventBus; -import sonia.scm.repository.BranchCreatedEvent; import sonia.scm.repository.ClearRepositoryCacheEvent; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.PostReceiveRepositoryHookEvent; @@ -58,6 +56,7 @@ import sonia.scm.repository.work.WorkdirProvider; import sonia.scm.security.PublicKeyCreatedEvent; import sonia.scm.security.PublicKeyDeletedEvent; import sonia.scm.security.ScmSecurityException; +import sonia.scm.user.EMail; import java.util.Set; @@ -117,47 +116,9 @@ public final class RepositoryServiceFactory { //~--- constructors --------------------------------------------------------- - /** - * Constructs a new {@link RepositoryServiceFactory}. This constructor - * should not be called manually, it should only be used by the injection - * container. - * - * @param configuration configuration - * @param cacheManager cache manager - * @param repositoryManager manager for repositories - * @param resolvers a set of {@link RepositoryServiceResolver} - * @param preProcessorUtil helper object for pre processor handling - * @param protocolProviders - * @param workdirProvider - * @since 1.21 - */ - @Inject - public RepositoryServiceFactory(ScmConfiguration configuration, - CacheManager cacheManager, RepositoryManager repositoryManager, - Set resolvers, PreProcessorUtil preProcessorUtil, - @SuppressWarnings("rawtypes") Set protocolProviders, WorkdirProvider workdirProvider) { - this( - configuration, cacheManager, repositoryManager, resolvers, - preProcessorUtil, protocolProviders, workdirProvider, ScmEventBus.getInstance() - ); - } - - @VisibleForTesting - RepositoryServiceFactory(ScmConfiguration configuration, - CacheManager cacheManager, RepositoryManager repositoryManager, - Set resolvers, PreProcessorUtil preProcessorUtil, - Set protocolProviders, WorkdirProvider workdirProvider, - ScmEventBus eventBus) { - this.configuration = configuration; - this.cacheManager = cacheManager; - this.repositoryManager = repositoryManager; - this.resolvers = resolvers; - this.preProcessorUtil = preProcessorUtil; - this.protocolProviders = protocolProviders; - this.workdirProvider = workdirProvider; - - eventBus.register(new CacheClearHook(cacheManager)); - } + private final EMail eMail; + @SuppressWarnings("rawtypes") + private Set protocolProviders; //~--- methods -------------------------------------------------------------- @@ -216,47 +177,27 @@ public final class RepositoryServiceFactory { } /** - * Creates a new RepositoryService for the given repository. + * Constructs a new {@link RepositoryServiceFactory}. This constructor + * should not be called manually, it should only be used by the injection + * container. * - * @param repository the repository - * @return a implementation of RepositoryService - * for the given type of repository - * @throws RepositoryServiceNotFoundException if no repository service - * implementation for this kind of repository is available - * @throws NullPointerException if the repository is null - * @throws ScmSecurityException if current user has not read permissions - * for that repository + * @param cacheManager cache manager + * @param repositoryManager manager for repositories + * @param resolvers a set of {@link RepositoryServiceResolver} + * @param preProcessorUtil helper object for pre processor handling + * @param protocolProviders + * @param workdirProvider + * @since 1.21 */ - public RepositoryService create(Repository repository) { - Preconditions.checkNotNull(repository, "repository is required"); - - // check for read permissions of current user - RepositoryPermissions.read(repository).check(); - - RepositoryService service = null; - - for (RepositoryServiceResolver resolver : resolvers) { - RepositoryServiceProvider provider = resolver.resolve(repository); - - if (provider != null) { - if (logger.isDebugEnabled()) { - logger.debug( - "create new repository service for repository {} of type {}", - repository.getName(), repository.getType()); - } - - service = new RepositoryService(cacheManager, provider, repository, - preProcessorUtil, protocolProviders, workdirProvider); - - break; - } - } - - if (service == null) { - throw new RepositoryServiceNotFoundException(repository); - } - - return service; + @Inject + public RepositoryServiceFactory(CacheManager cacheManager, RepositoryManager repositoryManager, + Set resolvers, PreProcessorUtil preProcessorUtil, + @SuppressWarnings("rawtypes") Set protocolProviders, + WorkdirProvider workdirProvider, EMail eMail) { + this( + cacheManager, repositoryManager, resolvers, + preProcessorUtil, protocolProviders, workdirProvider, eMail, ScmEventBus.getInstance() + ); } //~--- inner classes -------------------------------------------------------- @@ -355,11 +296,6 @@ public final class RepositoryServiceFactory { */ private final CacheManager cacheManager; - /** - * scm-manager configuration - */ - private final ScmConfiguration configuration; - /** * pre processor util */ @@ -375,7 +311,65 @@ public final class RepositoryServiceFactory { */ private final Set resolvers; - private Set protocolProviders; + @VisibleForTesting + RepositoryServiceFactory(CacheManager cacheManager, RepositoryManager repositoryManager, + Set resolvers, PreProcessorUtil preProcessorUtil, + @SuppressWarnings("rawtypes") Set protocolProviders, + WorkdirProvider workdirProvider, EMail eMail, ScmEventBus eventBus) { + this.cacheManager = cacheManager; + this.repositoryManager = repositoryManager; + this.resolvers = resolvers; + this.preProcessorUtil = preProcessorUtil; + this.protocolProviders = protocolProviders; + this.workdirProvider = workdirProvider; + this.eMail = eMail; + + eventBus.register(new CacheClearHook(cacheManager)); + } private final WorkdirProvider workdirProvider; + + /** + * Creates a new RepositoryService for the given repository. + * + * @param repository the repository + * @return a implementation of RepositoryService + * for the given type of repository + * @throws RepositoryServiceNotFoundException if no repository service + * implementation for this kind of repository is available + * @throws NullPointerException if the repository is null + * @throws ScmSecurityException if current user has not read permissions + * for that repository + */ + public RepositoryService create(Repository repository) { + Preconditions.checkNotNull(repository, "repository is required"); + + // check for read permissions of current user + RepositoryPermissions.read(repository).check(); + + RepositoryService service = null; + + for (RepositoryServiceResolver resolver : resolvers) { + RepositoryServiceProvider provider = resolver.resolve(repository); + + if (provider != null) { + if (logger.isDebugEnabled()) { + logger.debug( + "create new repository service for repository {} of type {}", + repository.getName(), repository.getType()); + } + + service = new RepositoryService(cacheManager, provider, repository, + preProcessorUtil, protocolProviders, workdirProvider, eMail); + + break; + } + } + + if (service == null) { + throw new RepositoryServiceNotFoundException(repository); + } + + return service; + } } diff --git a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java index fa2c598f03..5bc62fbecf 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java +++ b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java @@ -21,28 +21,29 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.util; import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.Subject; import sonia.scm.repository.Person; +import sonia.scm.user.EMail; import sonia.scm.user.User; public class AuthorUtil { - public static void setAuthorIfNotAvailable(CommandWithAuthor request) { + public static void setAuthorIfNotAvailable(CommandWithAuthor request, EMail eMail) { if (request.getAuthor() == null) { - request.setAuthor(createAuthorFromSubject()); + request.setAuthor(createAuthorFromSubject(eMail)); } } - private static Person createAuthorFromSubject() { + private static Person createAuthorFromSubject(EMail eMail) { Subject subject = SecurityUtils.getSubject(); User user = subject.getPrincipals().oneByType(User.class); String name = user.getDisplayName(); - String email = user.getMail(); - return new Person(name, email); + String mailAddress = eMail.createFallbackMailAddress(user); + return new Person(name, mailAddress); } public interface CommandWithAuthor { diff --git a/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java index 5583261e52..15a4b909f1 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java @@ -25,7 +25,12 @@ package sonia.scm.repository.api; import com.google.common.io.ByteSource; +import org.apache.shiro.subject.PrincipalCollection; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; @@ -34,10 +39,12 @@ import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.stubbing.Answer; -import sonia.scm.repository.Person; +import sonia.scm.config.ScmConfiguration; import sonia.scm.repository.spi.ModifyCommand; import sonia.scm.repository.spi.ModifyCommandRequest; import sonia.scm.repository.work.WorkdirProvider; +import sonia.scm.user.EMail; +import sonia.scm.user.User; import java.io.ByteArrayInputStream; import java.io.File; @@ -50,15 +57,19 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class ModifyCommandBuilderTest { + private static final ScmConfiguration SCM_CONFIGURATION = new ScmConfiguration(); + @Mock ModifyCommand command; @Mock @@ -73,7 +84,7 @@ class ModifyCommandBuilderTest { void initWorkdir(@TempDir Path temp) throws IOException { workdir = Files.createDirectory(temp.resolve("workdir")); lenient().when(workdirProvider.createNewWorkdir()).thenReturn(workdir.toFile()); - commandBuilder = new ModifyCommandBuilder(command, workdirProvider); + commandBuilder = new ModifyCommandBuilder(command, workdirProvider, new EMail(SCM_CONFIGURATION)); } @BeforeEach @@ -89,136 +100,27 @@ class ModifyCommandBuilderTest { ); } - @Test - void shouldReturnTargetRevisionFromCommit() { - String targetRevision = initCommand() - .deleteFile("toBeDeleted") - .execute(); - - assertThat(targetRevision).isEqualTo("target"); - } - - @Test - void shouldExecuteDelete() throws IOException { - initCommand() - .deleteFile("toBeDeleted") - .execute(); - - verify(worker).delete("toBeDeleted"); - } - - @Test - void shouldExecuteCreateWithByteSourceContent() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); - - initCommand() - .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); - assertThat(contentCaptor).contains("content"); - } - - @Test - void shouldExecuteCreateWithInputStreamContent() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); - - initCommand() - .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); - assertThat(contentCaptor).contains("content"); - } - - @Test - void shouldExecuteCreateWithOverwriteFalseAsDefault() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); - - initCommand() - .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); - assertThat(overwriteCaptor.getValue()).isFalse(); - assertThat(contentCaptor).contains("content"); - } - - @Test - void shouldExecuteCreateWithOverwriteIfSet() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); - - initCommand() - .createFile("toBeCreated").setOverwrite(true).withData(new ByteArrayInputStream("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); - assertThat(overwriteCaptor.getValue()).isTrue(); - assertThat(contentCaptor).contains("content"); - } - - @Test - void shouldExecuteCreateMultipleTimes() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); - - initCommand() - .createFile("toBeCreated_1").withData(new ByteArrayInputStream("content_1".getBytes())) - .createFile("toBeCreated_2").withData(new ByteArrayInputStream("content_2".getBytes())) - .execute(); - - List createdNames = nameCaptor.getAllValues(); - assertThat(createdNames.get(0)).isEqualTo("toBeCreated_1"); - assertThat(createdNames.get(1)).isEqualTo("toBeCreated_2"); - assertThat(contentCaptor).contains("content_1", "content_2"); - } - - @Test - void shouldExecuteModify() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).modify(nameCaptor.capture(), any()); - - initCommand() - .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeModified"); - assertThat(contentCaptor).contains("content"); - } - private ModifyCommandBuilder initCommand() { return commandBuilder .setBranch("branch") - .setCommitMessage("message") - .setAuthor(new Person()); + .setCommitMessage("message"); } - @Test - void shouldDeleteTemporaryFiles(@TempDir Path temp) throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - ArgumentCaptor fileCaptor = ArgumentCaptor.forClass(File.class); - doNothing().when(worker).modify(nameCaptor.capture(), fileCaptor.capture()); + private void mockLoggedInUser(User loggedInUser) { + Subject subject = mock(Subject.class); + ThreadContext.bind(subject); + PrincipalCollection principals = mock(PrincipalCollection.class); + when(subject.getPrincipals()).thenReturn(principals); + when(principals.oneByType(User.class)).thenReturn(loggedInUser); + } - initCommand() - .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) - .execute(); - - assertThat(Files.list(temp)).isEmpty(); + @AfterEach + void unbindSubjec() { + ThreadContext.unbindSubject(); } private static class ExtractContent implements Answer { + private final List contentCaptor; public ExtractContent(List contentCaptor) { @@ -230,4 +132,171 @@ class ModifyCommandBuilderTest { return contentCaptor.add(Files.readAllLines(((File) invocation.getArgument(1)).toPath()).get(0)); } } + + @Nested + class WithUserWithMail { + + @BeforeEach + void initSubject() { + User loggedInUser = new User("dent", "Arthur", "dent@hitchhiker.com"); + mockLoggedInUser(loggedInUser); + } + + @Test + void shouldReturnTargetRevisionFromCommit() { + String targetRevision = initCommand() + .deleteFile("toBeDeleted") + .execute(); + + assertThat(targetRevision).isEqualTo("target"); + } + + @Test + void shouldExecuteDelete() throws IOException { + initCommand() + .deleteFile("toBeDeleted") + .execute(); + + verify(worker).delete("toBeDeleted"); + } + + @Test + void shouldExecuteCreateWithByteSourceContent() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); + + initCommand() + .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateWithInputStreamContent() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); + + initCommand() + .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateWithOverwriteFalseAsDefault() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); + + initCommand() + .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(overwriteCaptor.getValue()).isFalse(); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateWithOverwriteIfSet() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); + + initCommand() + .createFile("toBeCreated").setOverwrite(true).withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(overwriteCaptor.getValue()).isTrue(); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateMultipleTimes() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); + + initCommand() + .createFile("toBeCreated_1").withData(new ByteArrayInputStream("content_1".getBytes())) + .createFile("toBeCreated_2").withData(new ByteArrayInputStream("content_2".getBytes())) + .execute(); + + List createdNames = nameCaptor.getAllValues(); + assertThat(createdNames.get(0)).isEqualTo("toBeCreated_1"); + assertThat(createdNames.get(1)).isEqualTo("toBeCreated_2"); + assertThat(contentCaptor).contains("content_1", "content_2"); + } + + @Test + void shouldExecuteModify() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).modify(nameCaptor.capture(), any()); + + initCommand() + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeModified"); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldDeleteTemporaryFiles(@TempDir Path temp) throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor fileCaptor = ArgumentCaptor.forClass(File.class); + doNothing().when(worker).modify(nameCaptor.capture(), fileCaptor.capture()); + + initCommand() + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(Files.list(temp)).isEmpty(); + } + + @Test + void shouldUseMailFromUser() throws IOException { + initCommand() + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + verify(command).execute(argThat(modifyCommandRequest -> { + assertThat(modifyCommandRequest.getAuthor().getMail()).isEqualTo("dent@hitchhiker.com"); + return true; + })); + } + } + + @Nested + class WithUserWithoutMail { + + @BeforeEach + void initSubject() { + User loggedInUser = new User("dent", "Arthur", null); + mockLoggedInUser(loggedInUser); + } + + @Test + void shouldUseMailFromUser() throws IOException { + SCM_CONFIGURATION.setMailHost("heart-of-gold.local"); + initCommand() + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + verify(command).execute(argThat(modifyCommandRequest -> { + assertThat(modifyCommandRequest.getAuthor().getMail()).isEqualTo("dent@heart-of-gold.local"); + return true; + })); + } + } } diff --git a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceFactoryTest.java b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceFactoryTest.java index 492d23dec8..d77d0d072b 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceFactoryTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceFactoryTest.java @@ -46,6 +46,7 @@ import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.spi.RepositoryServiceProvider; import sonia.scm.repository.spi.RepositoryServiceResolver; import sonia.scm.repository.work.WorkdirProvider; +import sonia.scm.user.EMail; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -56,9 +57,6 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class RepositoryServiceFactoryTest { - @Mock - private ScmConfiguration configuration; - @Mock(answer = Answers.RETURNS_MOCKS) private CacheManager cacheManager; @@ -94,8 +92,9 @@ class RepositoryServiceFactoryTest { builder.add(repositoryServiceResolver); } return new RepositoryServiceFactory( - configuration, cacheManager, repositoryManager, builder.build(), - preProcessorUtil, ImmutableSet.of(), workdirProvider, eventBus + cacheManager, repositoryManager, builder.build(), + preProcessorUtil, ImmutableSet.of(), workdirProvider, + new EMail(new ScmConfiguration()), eventBus ); } diff --git a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java index 901666cb87..8394e3c582 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java @@ -21,13 +21,15 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.api; import org.junit.Test; +import sonia.scm.config.ScmConfiguration; import sonia.scm.repository.Repository; import sonia.scm.repository.spi.HttpScmProtocol; import sonia.scm.repository.spi.RepositoryServiceProvider; +import sonia.scm.user.EMail; import javax.servlet.ServletConfig; import javax.servlet.http.HttpServletRequest; @@ -46,9 +48,11 @@ public class RepositoryServiceTest { private final RepositoryServiceProvider provider = mock(RepositoryServiceProvider.class); private final Repository repository = new Repository("", "git", "space", "repo"); + private final EMail eMail = new EMail(new ScmConfiguration()); + @Test public void shouldReturnMatchingProtocolsFromProvider() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null, eMail); Stream supportedProtocols = repositoryService.getSupportedProtocols(); assertThat(sizeOf(supportedProtocols.collect(Collectors.toList()))).isEqualTo(1); @@ -56,7 +60,7 @@ public class RepositoryServiceTest { @Test public void shouldFindKnownProtocol() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null, eMail); HttpScmProtocol protocol = repositoryService.getProtocol(HttpScmProtocol.class); @@ -65,11 +69,9 @@ public class RepositoryServiceTest { @Test public void shouldFailForUnknownProtocol() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null, eMail); - assertThrows(IllegalArgumentException.class, () -> { - repositoryService.getProtocol(UnknownScmProtocol.class); - }); + assertThrows(IllegalArgumentException.class, () -> repositoryService.getProtocol(UnknownScmProtocol.class)); } private static class DummyHttpProtocol extends HttpScmProtocol { From e60ea987dda1aa633da78ab91e56d7c26506c4a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 13 Oct 2020 20:40:30 +0200 Subject: [PATCH 05/17] Make mail optional for user --- scm-ui/ui-types/src/User.ts | 4 ++-- scm-ui/ui-webapp/src/users/components/UserForm.tsx | 6 +++--- .../src/main/java/sonia/scm/api/v2/resources/UserDto.java | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/scm-ui/ui-types/src/User.ts b/scm-ui/ui-types/src/User.ts index 43ff64b9fa..29009b5d0d 100644 --- a/scm-ui/ui-types/src/User.ts +++ b/scm-ui/ui-types/src/User.ts @@ -27,13 +27,13 @@ import { Links } from "./hal"; export type DisplayedUser = { id: string; displayName: string; - mail: string; + mail?: string; }; export type User = { displayName: string; name: string; - mail: string; + mail?: string; password: string; active: boolean; type?: string; diff --git a/scm-ui/ui-webapp/src/users/components/UserForm.tsx b/scm-ui/ui-webapp/src/users/components/UserForm.tsx index cd0856fa84..9c78ef8476 100644 --- a/scm-ui/ui-webapp/src/users/components/UserForm.tsx +++ b/scm-ui/ui-webapp/src/users/components/UserForm.tsx @@ -113,8 +113,7 @@ class UserForm extends React.Component { this.editUserComponentsAreUnchanged() || this.state.mailValidationError || this.state.displayNameValidationError || - this.isFalsy(user.displayName) || - this.isFalsy(user.mail) + this.isFalsy(user.displayName) ); }; @@ -152,6 +151,7 @@ class UserForm extends React.Component { // edit existing user subtitle = ; } + return ( <> {subtitle} @@ -218,7 +218,7 @@ class UserForm extends React.Component { handleEmailChange = (mail: string) => { this.setState({ - mailValidationError: !validator.isMailValid(mail), + mailValidationError: !!mail && !validator.isMailValid(mail), user: { ...this.state.user, mail diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java index 0e4e9b34c2..027cc59621 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import com.fasterxml.jackson.annotation.JsonInclude; @@ -46,7 +46,8 @@ public class UserDto extends HalRepresentation { private String displayName; @JsonInclude(JsonInclude.Include.NON_NULL) private Instant lastModified; - @NotEmpty @Email + @JsonInclude(JsonInclude.Include.NON_NULL) + @Email private String mail; @Pattern(regexp = ValidationUtil.REGEX_NAME) private String name; From be6bb8bf372a9b63332f8a7ee212419845f0e085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 14 Oct 2020 08:17:33 +0200 Subject: [PATCH 06/17] Rename "mail host" to "mail domain name" --- .../sonia/scm/config/ScmConfiguration.java | 22 +++++++++---------- .../src/main/java/sonia/scm/user/EMail.java | 2 +- .../api/ModifyCommandBuilderTest.java | 2 +- scm-ui/ui-types/src/Config.ts | 2 +- .../ui-webapp/public/locales/de/config.json | 4 ++-- .../ui-webapp/public/locales/en/config.json | 4 ++-- .../src/admin/components/form/ConfigForm.tsx | 2 +- .../admin/components/form/GeneralSettings.tsx | 16 +++++++------- .../sonia/scm/api/v2/resources/ConfigDto.java | 2 +- ...ConfigDtoToScmConfigurationMapperTest.java | 4 ++-- ...ScmConfigurationToConfigDtoMapperTest.java | 2 +- 11 files changed, 31 insertions(+), 31 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java b/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java index fe67e43820..2fed4058f7 100644 --- a/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java +++ b/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java @@ -81,10 +81,10 @@ public class ScmConfiguration implements Configuration { public static final String DEFAULT_LOGIN_INFO_URL = "https://login-info.scm-manager.org/api/v1/login-info"; /** - * Default e-mail host that will be used whenever we have to generate an e-mail address for a user that has no mail - * address configured. + * Default e-mail domain name that will be used whenever we have to generate an e-mail address for a user that has no + * mail address configured. */ - public static final String DEFAULT_MAIL_HOST = "scm-manager.local"; + public static final String DEFAULT_MAIL_DOMAIN_NAME = "scm-manager.local"; /** * Default plugin url from version 1.0 @@ -193,8 +193,8 @@ public class ScmConfiguration implements Configuration { @XmlElement(name = "login-info-url") private String loginInfoUrl = DEFAULT_LOGIN_INFO_URL; - @XmlElement(name = "mail-host") - private String mailHost = DEFAULT_MAIL_HOST; + @XmlElement(name = "mail-domain-name") + private String mailDomainName = DEFAULT_MAIL_DOMAIN_NAME; /** * Calls the {@link sonia.scm.ConfigChangedListener#configChanged(Object)} @@ -235,7 +235,7 @@ public class ScmConfiguration implements Configuration { this.namespaceStrategy = other.namespaceStrategy; this.loginInfoUrl = other.loginInfoUrl; this.releaseFeedUrl = other.releaseFeedUrl; - this.mailHost = other.mailHost; + this.mailDomainName = other.mailDomainName; } /** @@ -305,8 +305,8 @@ public class ScmConfiguration implements Configuration { * * @since 2.8.0 */ - public String getMailHost() { - return mailHost; + public String getMailDomainName() { + return mailDomainName; } /** @@ -492,11 +492,11 @@ public class ScmConfiguration implements Configuration { /** * Sets the mail host, that will be used to create e-mail addresses for users without one whenever one is required. * - * @param mailHost The new mail host to use. + * @param mailDomainName The new mail host to use. * @since 2.8.0 */ - public void setMailHost(String mailHost) { - this.mailHost = mailHost; + public void setMailDomainName(String mailDomainName) { + this.mailDomainName = mailDomainName; } /** diff --git a/scm-core/src/main/java/sonia/scm/user/EMail.java b/scm-core/src/main/java/sonia/scm/user/EMail.java index 121825157c..0718a3c722 100644 --- a/scm-core/src/main/java/sonia/scm/user/EMail.java +++ b/scm-core/src/main/java/sonia/scm/user/EMail.java @@ -43,7 +43,7 @@ public class EMail { if (user.getId().contains("@")) { return user.getId(); } else { - return user.getId() + "@" + scmConfiguration.getMailHost(); + return user.getId() + "@" + scmConfiguration.getMailDomainName(); } } else { return user.getMail(); diff --git a/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java index 15a4b909f1..281e793cbb 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java @@ -288,7 +288,7 @@ class ModifyCommandBuilderTest { @Test void shouldUseMailFromUser() throws IOException { - SCM_CONFIGURATION.setMailHost("heart-of-gold.local"); + SCM_CONFIGURATION.setMailDomainName("heart-of-gold.local"); initCommand() .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) .execute(); diff --git a/scm-ui/ui-types/src/Config.ts b/scm-ui/ui-types/src/Config.ts index 4369b98e6e..92b678e491 100644 --- a/scm-ui/ui-types/src/Config.ts +++ b/scm-ui/ui-types/src/Config.ts @@ -48,6 +48,6 @@ export type Config = { namespaceStrategy: string; loginInfoUrl: string; releaseFeedUrl: string; - mailHost: string; + mailDomainName: string; _links: Links; }; diff --git a/scm-ui/ui-webapp/public/locales/de/config.json b/scm-ui/ui-webapp/public/locales/de/config.json index 9ab2d38675..24830f2f4d 100644 --- a/scm-ui/ui-webapp/public/locales/de/config.json +++ b/scm-ui/ui-webapp/public/locales/de/config.json @@ -47,7 +47,7 @@ "skip-failed-authenticators": "Fehlgeschlagene Authentifizierer überspringen", "plugin-url": "Plugin Center URL", "release-feed-url": "Release Feed URL", - "mail-host": "Fallback Mail Host", + "mail-domain-name": "Fallback E-Mail Domain Name", "enabled-xsrf-protection": "XSRF Protection aktivieren", "namespace-strategy": "Namespace Strategie", "login-info-url": "Login Info URL" @@ -63,7 +63,7 @@ "dateFormatHelpText": "Moments Datumsformat. Zulässige Formate sind in der MomentJS Dokumentation beschrieben.", "pluginUrlHelpText": "Die URL der Plugin Center API. Beschreibung der Platzhalter: version = SCM-Manager Version; os = Betriebssystem; arch = Architektur", "releaseFeedUrlHelpText": "Die URL des RSS Release Feed des SCM-Manager. Darüber wird über die neue SCM-Manager Version informiert. Um diese Funktion zu deaktivieren lassen Sie dieses Feld leer.", - "mailHostHelpText": "Dieser Host Name wird genutzt, wenn für einen User eine E-Mail-Adresse benötigt wird, für den keine hinterlegt ist. Dieser Host wird nicht zum Versenden von E-Mails genutzt und auch keine anderweitige Verbindung aufgebaut.", + "mailDomainNameHelpText": "Dieser Domain Name wird genutzt, wenn für einen User eine E-Mail-Adresse benötigt wird, für den keine hinterlegt ist. Diese Domain wird nicht zum Versenden von E-Mails genutzt und auch keine anderweitige Verbindung aufgebaut.", "enableForwardingHelpText": "mod_proxy Port Weiterleitung aktivieren.", "disableGroupingGridHelpText": "Repository Gruppen deaktivieren. Nach einer Änderung an dieser Einstellung muss die Seite komplett neu geladen werden.", "allowAnonymousAccessHelpText": "Anonyme Benutzer haben Zugriff auf freigegebene Repositories.", diff --git a/scm-ui/ui-webapp/public/locales/en/config.json b/scm-ui/ui-webapp/public/locales/en/config.json index 12d38694a5..6cb06ecb25 100644 --- a/scm-ui/ui-webapp/public/locales/en/config.json +++ b/scm-ui/ui-webapp/public/locales/en/config.json @@ -47,7 +47,7 @@ "skip-failed-authenticators": "Skip Failed Authenticators", "plugin-url": "Plugin Center URL", "release-feed-url": "Release Feed URL", - "mail-host": "Fallback Mail Host", + "mail-domain-name": "Fallback Mail Domain Name", "enabled-xsrf-protection": "Enabled XSRF Protection", "namespace-strategy": "Namespace Strategy", "login-info-url": "Login Info URL" @@ -63,7 +63,7 @@ "dateFormatHelpText": "Moments date format. Please have a look at the MomentJS documentation.", "pluginUrlHelpText": "The url of the Plugin Center API. Explanation of the placeholders: version = SCM-Manager Version; os = Operation System; arch = Architecture", "releaseFeedUrlHelpText": "The url of the RSS Release Feed for SCM-Manager. This provides up-to-date version information. To disable this feature just leave the url blank.", - "mailHostHelpText": "This host name will be used to create email addresses for users when needed and they have none configured. It will not be used to send mails nor will be accessed otherwise.", + "mailDomainNameHelpText": "This domain name will be used to create email addresses for users without one when needed. It will not be used to send mails nor will be accessed otherwise.", "enableForwardingHelpText": "Enable mod_proxy port forwarding.", "disableGroupingGridHelpText": "Disable repository Groups. A complete page reload is required after a change of this value.", "allowAnonymousAccessHelpText": "Anonymous users have access on granted repositories.", diff --git a/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx b/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx index b38113831f..c284df3180 100644 --- a/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx +++ b/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx @@ -144,7 +144,7 @@ class ConfigForm extends React.Component { skipFailedAuthenticators={config.skipFailedAuthenticators} pluginUrl={config.pluginUrl} releaseFeedUrl={config.releaseFeedUrl} - mailHost={config.mailHost} + mailDomainName={config.mailDomainName} enabledXsrfProtection={config.enabledXsrfProtection} namespaceStrategy={config.namespaceStrategy} onChange={(isValid, changedValue, name) => this.onChange(isValid, changedValue, name)} diff --git a/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx b/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx index 49871ed2aa..e9ed50a3b5 100644 --- a/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx +++ b/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx @@ -36,7 +36,7 @@ type Props = WithTranslation & { skipFailedAuthenticators: boolean; pluginUrl: string; releaseFeedUrl: string; - mailHost: string; + mailDomainName: string; enabledXsrfProtection: boolean; namespaceStrategy: string; namespaceStrategies?: NamespaceStrategies; @@ -52,7 +52,7 @@ class GeneralSettings extends React.Component { loginInfoUrl, pluginUrl, releaseFeedUrl, - mailHost, + mailDomainName, enabledXsrfProtection, anonymousMode, namespaceStrategy, @@ -142,11 +142,11 @@ class GeneralSettings extends React.Component {
@@ -175,8 +175,8 @@ class GeneralSettings extends React.Component { handleReleaseFeedUrlChange = (value: string) => { this.props.onChange(true, value, "releaseFeedUrl"); }; - handleMailHostChange = (value: string) => { - this.props.onChange(true, value, "mailHost"); + handleMailDomainNameChange = (value: string) => { + this.props.onChange(true, value, "mailDomainName"); }; } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java index 42ae1ca6f0..f7025f141f 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java @@ -59,7 +59,7 @@ public class ConfigDto extends HalRepresentation { private String namespaceStrategy; private String loginInfoUrl; private String releaseFeedUrl; - private String mailHost; + private String mailDomainName; @Override @SuppressWarnings("squid:S1185") // We want to have this method available in this package diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java index e693d78cea..f938c7f655 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java @@ -75,7 +75,7 @@ public class ConfigDtoToScmConfigurationMapperTest { assertTrue(config.isEnabledXsrfProtection()); assertEquals("username", config.getNamespaceStrategy()); assertEquals("https://scm-manager.org/login-info", config.getLoginInfoUrl()); - assertEquals("hitchhiker.mail", config.getMailHost()); + assertEquals("hitchhiker.mail", config.getMailDomainName()); } @Test @@ -114,7 +114,7 @@ public class ConfigDtoToScmConfigurationMapperTest { configDto.setEnabledXsrfProtection(true); configDto.setNamespaceStrategy("username"); configDto.setLoginInfoUrl("https://scm-manager.org/login-info"); - configDto.setMailHost("hitchhiker.mail"); + configDto.setMailDomainName("hitchhiker.mail"); return configDto; } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java index 42bf6e7e3b..49769bf1b3 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java @@ -106,7 +106,7 @@ public class ScmConfigurationToConfigDtoMapperTest { assertEquals("username", dto.getNamespaceStrategy()); assertEquals("https://scm-manager.org/login-info", dto.getLoginInfoUrl()); assertEquals("https://www.scm-manager.org/download/rss.xml", dto.getReleaseFeedUrl()); - assertEquals("scm-manager.local", dto.getMailHost()); + assertEquals("scm-manager.local", dto.getMailDomainName()); assertEquals(expectedBaseUri.toString(), dto.getLinks().getLinkBy("self").get().getHref()); assertEquals(expectedBaseUri.toString(), dto.getLinks().getLinkBy("update").get().getHref()); From 1d717acca0bcbf4a3c67cf6cb14fc1a1c6c781bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 14 Oct 2020 08:59:34 +0200 Subject: [PATCH 07/17] Log change --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 148eb799f3..11ae2b2fbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased +### Added +- Generation of email addresses for users, where none is configured ([#1370](https://github.com/scm-manager/scm-manager/pull/1370/files)) + ## [2.7.1] - 2020-10-14 ### Fixed - Null Pointer Exception on anonymous migration with deleted repositories ([#1371](https://github.com/scm-manager/scm-manager/pull/1371)) From 8e93248dd6f33b719e8072b2f426fa1777ee2370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 14 Oct 2020 16:26:25 +0200 Subject: [PATCH 08/17] Add component for commit author --- .../ui-components/src/repos/CommitAuthor.tsx | 63 +++++++++++++++++++ scm-ui/ui-components/src/repos/index.ts | 1 + scm-ui/ui-webapp/public/locales/de/repos.json | 8 ++- scm-ui/ui-webapp/public/locales/en/repos.json | 6 ++ 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 scm-ui/ui-components/src/repos/CommitAuthor.tsx diff --git a/scm-ui/ui-components/src/repos/CommitAuthor.tsx b/scm-ui/ui-components/src/repos/CommitAuthor.tsx new file mode 100644 index 0000000000..e371069ad3 --- /dev/null +++ b/scm-ui/ui-components/src/repos/CommitAuthor.tsx @@ -0,0 +1,63 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import React from "react"; +import { WithTranslation, withTranslation } from "react-i18next"; +import Notification from "../Notification"; +import { Me } from "@scm-manager/ui-types"; +import { connect } from "react-redux"; +import { compose } from "redux"; + +type Props = WithTranslation & { + // props from global state + me: Me; +}; + +class CommitAuthor extends React.Component { + render() { + const { me, t } = this.props; + + const mail = me.mail ? me.mail : me.fallbackMail; + + return ( + <> + {!me.mail && {t("commit.commitAuthor.noMail")}} + + {t("commit.commitAuthor.author")} {`${me.displayName} <${mail}>`} + + + ); + } +} + +const mapStateToProps = (state: any) => { + const { auth } = state; + const me = auth.me; + + return { + me + }; +}; + +export default compose(connect(mapStateToProps), withTranslation("repos"))(CommitAuthor); diff --git a/scm-ui/ui-components/src/repos/index.ts b/scm-ui/ui-components/src/repos/index.ts index 36b26495b8..f33ffbc4cb 100644 --- a/scm-ui/ui-components/src/repos/index.ts +++ b/scm-ui/ui-components/src/repos/index.ts @@ -52,6 +52,7 @@ export { default as RepositoryAvatar } from "./RepositoryAvatar"; export { default as RepositoryEntry } from "./RepositoryEntry"; export { default as RepositoryEntryLink } from "./RepositoryEntryLink"; export { default as JumpToFileButton } from "./JumpToFileButton"; +export { default as CommitAuthor } from "./CommitAuthor"; export { File, diff --git a/scm-ui/ui-webapp/public/locales/de/repos.json b/scm-ui/ui-webapp/public/locales/de/repos.json index a1303555f4..ae0f222765 100644 --- a/scm-ui/ui-webapp/public/locales/de/repos.json +++ b/scm-ui/ui-webapp/public/locales/de/repos.json @@ -135,6 +135,12 @@ "sources": "Sources" } }, + "commit": { + "commitAuthor": { + "author": "Autor", + "noMail": "Für den aktuellen Benutzer existiert keine E-Mail-Adresse. Es wird die unten angezeigte generierte Adresse genutzt." + } + }, "repositoryForm": { "subtitle": "Repository bearbeiten", "submit": "Speichern", @@ -269,4 +275,4 @@ "clickHere": "Klicken Sie hier um Ihre Datei hochzuladen.", "dragAndDrop": "Sie können Ihre Datei auch direkt in die Dropzone ziehen." } -} +}, diff --git a/scm-ui/ui-webapp/public/locales/en/repos.json b/scm-ui/ui-webapp/public/locales/en/repos.json index bbc21d3cad..3b05c1f48c 100644 --- a/scm-ui/ui-webapp/public/locales/en/repos.json +++ b/scm-ui/ui-webapp/public/locales/en/repos.json @@ -135,6 +135,12 @@ "count_plural": "{{count}} Contributors" } }, + "commit": { + "commitAuthor": { + "author": "Author", + "noMail": "We have found no email address for your current user. We will use the generated address shown below." + } + }, "repositoryForm": { "subtitle": "Edit Repository", "submit": "Save", From e67ca28c5f338db0c5ebf6bd4d68b6c69b36dde1 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 08:29:21 +0200 Subject: [PATCH 09/17] Do not break existing core api --- .../sonia/scm/repository/util/AuthorUtil.java | 6 +- .../scm/repository/util/AuthorUtilTest.java | 116 ++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java diff --git a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java index 5bc62fbecf..1df0e5ef76 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java +++ b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java @@ -32,6 +32,10 @@ import sonia.scm.user.User; public class AuthorUtil { + public static void setAuthorIfNotAvailable(CommandWithAuthor request) { + setAuthorIfNotAvailable(request, null); + } + public static void setAuthorIfNotAvailable(CommandWithAuthor request, EMail eMail) { if (request.getAuthor() == null) { request.setAuthor(createAuthorFromSubject(eMail)); @@ -42,7 +46,7 @@ public class AuthorUtil { Subject subject = SecurityUtils.getSubject(); User user = subject.getPrincipals().oneByType(User.class); String name = user.getDisplayName(); - String mailAddress = eMail.createFallbackMailAddress(user); + String mailAddress = eMail != null ? eMail.createFallbackMailAddress(user) : user.getMail(); return new Person(name, mailAddress); } diff --git a/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java b/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java new file mode 100644 index 0000000000..efd5b0e69a --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java @@ -0,0 +1,116 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.repository.util; + +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Answers; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.repository.Person; +import sonia.scm.user.EMail; +import sonia.scm.user.User; +import sonia.scm.web.UserAgentParserTest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class AuthorUtilTest { + + @Mock + private EMail eMail; + + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + private Subject subject; + + @BeforeEach + void setUpSubject() { + ThreadContext.bind(subject); + } + + @AfterEach + void tearDownSubject() { + ThreadContext.unbindSubject(); + } + + @Test + void shouldCreateMailAddressFromEmail() { + User trillian = new User("trillian"); + when(subject.getPrincipals().oneByType(User.class)).thenReturn(trillian); + when(eMail.createFallbackMailAddress(trillian)).thenReturn("tricia@hitchhicker.com"); + + Command command = new Command(null); + AuthorUtil.setAuthorIfNotAvailable(command, eMail); + + assertThat(command.getAuthor().getMail()).isEqualTo("tricia@hitchhicker.com"); + } + + @Test + void shouldUseUsersMailAddressWithoutEMail() { + User trillian = new User("trillian", "Trillian", "trillian.mcmillan@hitchhiker.com"); + when(subject.getPrincipals().oneByType(User.class)).thenReturn(trillian); + + Command command = new Command(null); + AuthorUtil.setAuthorIfNotAvailable(command); + + assertThat(command.getAuthor().getMail()).isEqualTo("trillian.mcmillan@hitchhiker.com"); + } + + @Test + void shouldKeepExistingAuthor() { + Person person = new Person("Trillian McMillan", "trillian.mcmillian@hitchhiker.com"); + + Command command = new Command(person); + AuthorUtil.setAuthorIfNotAvailable(command); + + assertThat(command.getAuthor()).isSameAs(person); + } + + public static class Command implements AuthorUtil.CommandWithAuthor { + + private Person person; + + public Command(Person person) { + this.person = person; + } + + @Override + public Person getAuthor() { + return person; + } + + @Override + public void setAuthor(Person person) { + this.person = person; + } + } + +} From ffd1daba522be1751e446c51a5fb922234f0b62f Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 08:34:36 +0200 Subject: [PATCH 10/17] There is no need for a class component here We should use function component as much as possible, because their are easier to read and less confusing. --- .../ui-components/src/repos/CommitAuthor.tsx | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/scm-ui/ui-components/src/repos/CommitAuthor.tsx b/scm-ui/ui-components/src/repos/CommitAuthor.tsx index e371069ad3..f845582267 100644 --- a/scm-ui/ui-components/src/repos/CommitAuthor.tsx +++ b/scm-ui/ui-components/src/repos/CommitAuthor.tsx @@ -22,34 +22,31 @@ * SOFTWARE. */ -import React from "react"; -import { WithTranslation, withTranslation } from "react-i18next"; +import React, { FC } from "react"; +import { useTranslation } from "react-i18next"; import Notification from "../Notification"; import { Me } from "@scm-manager/ui-types"; import { connect } from "react-redux"; -import { compose } from "redux"; -type Props = WithTranslation & { +type Props = { // props from global state me: Me; }; -class CommitAuthor extends React.Component { - render() { - const { me, t } = this.props; +const CommitAuthor: FC = ({ me }) => { + const [t] = useTranslation("repos"); - const mail = me.mail ? me.mail : me.fallbackMail; + const mail = me.mail ? me.mail : me.fallbackMail; - return ( - <> - {!me.mail && {t("commit.commitAuthor.noMail")}} - - {t("commit.commitAuthor.author")} {`${me.displayName} <${mail}>`} - - - ); - } -} + return ( + <> + {!me.mail && {t("commit.commitAuthor.noMail")}} + + {t("commit.commitAuthor.author")} {`${me.displayName} <${mail}>`} + + + ); +}; const mapStateToProps = (state: any) => { const { auth } = state; @@ -60,4 +57,4 @@ const mapStateToProps = (state: any) => { }; }; -export default compose(connect(mapStateToProps), withTranslation("repos"))(CommitAuthor); +export default connect(mapStateToProps)(CommitAuthor); From 208fa0dcc8a8583144576b3abc346254411c560b Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 11:22:01 +0200 Subject: [PATCH 11/17] Fix breaking change in RepositoryServiceFactory constructor Deprecate old RepositoryServiceFactory constructor and create a new one to avoid breaking change in api --- .../api/RepositoryServiceFactory.java | 231 +++++++++--------- 1 file changed, 120 insertions(+), 111 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java index 15cec9d210..2c13cb63be 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java @@ -40,6 +40,7 @@ import sonia.scm.HandlerEventType; import sonia.scm.NotFoundException; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; +import sonia.scm.config.ScmConfiguration; import sonia.scm.event.ScmEventBus; import sonia.scm.repository.ClearRepositoryCacheEvent; import sonia.scm.repository.NamespaceAndName; @@ -58,6 +59,7 @@ import sonia.scm.security.PublicKeyDeletedEvent; import sonia.scm.security.ScmSecurityException; import sonia.scm.user.EMail; +import javax.annotation.Nullable; import java.util.Set; import static sonia.scm.ContextEntry.ContextBuilder.entity; @@ -114,13 +116,88 @@ public final class RepositoryServiceFactory { private static final Logger logger = LoggerFactory.getLogger(RepositoryServiceFactory.class); - //~--- constructors --------------------------------------------------------- + private final CacheManager cacheManager; + private final RepositoryManager repositoryManager; + private final Set resolvers; + private final PreProcessorUtil preProcessorUtil; + @SuppressWarnings({"rawtypes", "java:S3740"}) + private final Set protocolProviders; + private final WorkdirProvider workdirProvider; + @Nullable private final EMail eMail; - @SuppressWarnings("rawtypes") - private Set protocolProviders; - //~--- methods -------------------------------------------------------------- + + /** + * Constructs a new {@link RepositoryServiceFactory}. This constructor + * should not be called manually, it should only be used by the injection + * container. + * + * @param configuration configuration + * @param cacheManager cache manager + * @param repositoryManager manager for repositories + * @param resolvers a set of {@link RepositoryServiceResolver} + * @param preProcessorUtil helper object for pre processor handling + * @param protocolProviders providers for repository protocols + * @param workdirProvider provider for working directories + * + * @deprecated use {@link RepositoryServiceFactory#RepositoryServiceFactory(CacheManager, RepositoryManager, Set, PreProcessorUtil, Set, WorkdirProvider, EMail)} instead + * @since 1.21 + */ + @Deprecated + public RepositoryServiceFactory(ScmConfiguration configuration, + CacheManager cacheManager, RepositoryManager repositoryManager, + Set resolvers, PreProcessorUtil preProcessorUtil, + @SuppressWarnings({"rawtypes", "java:S3740"}) Set protocolProviders, + WorkdirProvider workdirProvider) { + this( + cacheManager, repositoryManager, resolvers, + preProcessorUtil, protocolProviders, workdirProvider, null, ScmEventBus.getInstance() + ); + } + + /** + * Constructs a new {@link RepositoryServiceFactory}. This constructor + * should not be called manually, it should only be used by the injection + * container. + * + * @param cacheManager cache manager + * @param repositoryManager manager for repositories + * @param resolvers a set of {@link RepositoryServiceResolver} + * @param preProcessorUtil helper object for pre processor handling + * @param protocolProviders providers for repository protocols + * @param workdirProvider provider for working directories + * @param eMail handling user emails + * @since 2.8.0 + */ + @Inject + public RepositoryServiceFactory(CacheManager cacheManager, RepositoryManager repositoryManager, + Set resolvers, PreProcessorUtil preProcessorUtil, + @SuppressWarnings({"rawtypes", "java:S3740"}) Set protocolProviders, + WorkdirProvider workdirProvider, EMail eMail) { + this( + cacheManager, repositoryManager, resolvers, + preProcessorUtil, protocolProviders, workdirProvider, + eMail, ScmEventBus.getInstance() + ); + } + + @VisibleForTesting + @SuppressWarnings("java:S107") // to keep backward compatibility, we can not reduce amount of parameters + RepositoryServiceFactory(CacheManager cacheManager, RepositoryManager repositoryManager, + Set resolvers, PreProcessorUtil preProcessorUtil, + @SuppressWarnings({"rawtypes", "java:S3740"}) Set protocolProviders, + WorkdirProvider workdirProvider, @Nullable EMail eMail, ScmEventBus eventBus) { + this.cacheManager = cacheManager; + this.repositoryManager = repositoryManager; + this.resolvers = resolvers; + this.preProcessorUtil = preProcessorUtil; + this.protocolProviders = protocolProviders; + this.workdirProvider = workdirProvider; + this.eMail = eMail; + + eventBus.register(new CacheClearHook(cacheManager)); + } /** * Creates a new RepositoryService for the given repository. @@ -177,30 +254,48 @@ public final class RepositoryServiceFactory { } /** - * Constructs a new {@link RepositoryServiceFactory}. This constructor - * should not be called manually, it should only be used by the injection - * container. + * Creates a new RepositoryService for the given repository. * - * @param cacheManager cache manager - * @param repositoryManager manager for repositories - * @param resolvers a set of {@link RepositoryServiceResolver} - * @param preProcessorUtil helper object for pre processor handling - * @param protocolProviders - * @param workdirProvider - * @since 1.21 + * @param repository the repository + * @return a implementation of RepositoryService + * for the given type of repository + * @throws RepositoryServiceNotFoundException if no repository service + * implementation for this kind of repository is available + * @throws NullPointerException if the repository is null + * @throws ScmSecurityException if current user has not read permissions + * for that repository */ - @Inject - public RepositoryServiceFactory(CacheManager cacheManager, RepositoryManager repositoryManager, - Set resolvers, PreProcessorUtil preProcessorUtil, - @SuppressWarnings("rawtypes") Set protocolProviders, - WorkdirProvider workdirProvider, EMail eMail) { - this( - cacheManager, repositoryManager, resolvers, - preProcessorUtil, protocolProviders, workdirProvider, eMail, ScmEventBus.getInstance() - ); - } + public RepositoryService create(Repository repository) { + Preconditions.checkNotNull(repository, "repository is required"); - //~--- inner classes -------------------------------------------------------- + // check for read permissions of current user + RepositoryPermissions.read(repository).check(); + + RepositoryService service = null; + + for (RepositoryServiceResolver resolver : resolvers) { + RepositoryServiceProvider provider = resolver.resolve(repository); + + if (provider != null) { + if (logger.isDebugEnabled()) { + logger.debug( + "create new repository service for repository {} of type {}", + repository.getName(), repository.getType()); + } + + service = new RepositoryService(cacheManager, provider, repository, + preProcessorUtil, protocolProviders, workdirProvider, eMail); + + break; + } + } + + if (service == null) { + throw new RepositoryServiceNotFoundException(repository); + } + + return service; + } /** * Hook and listener to clear all relevant repository caches. @@ -225,8 +320,6 @@ public final class RepositoryServiceFactory { this.caches.add(cacheManager.getCache(BranchesCommandBuilder.CACHE_NAME)); } - //~--- methods ------------------------------------------------------------ - /** * Clear caches on explicit repository cache clear event. * @@ -288,88 +381,4 @@ public final class RepositoryServiceFactory { } } - - //~--- fields --------------------------------------------------------------- - - /** - * cache manager - */ - private final CacheManager cacheManager; - - /** - * pre processor util - */ - private final PreProcessorUtil preProcessorUtil; - - /** - * repository manager - */ - private final RepositoryManager repositoryManager; - - /** - * service resolvers - */ - private final Set resolvers; - - @VisibleForTesting - RepositoryServiceFactory(CacheManager cacheManager, RepositoryManager repositoryManager, - Set resolvers, PreProcessorUtil preProcessorUtil, - @SuppressWarnings("rawtypes") Set protocolProviders, - WorkdirProvider workdirProvider, EMail eMail, ScmEventBus eventBus) { - this.cacheManager = cacheManager; - this.repositoryManager = repositoryManager; - this.resolvers = resolvers; - this.preProcessorUtil = preProcessorUtil; - this.protocolProviders = protocolProviders; - this.workdirProvider = workdirProvider; - this.eMail = eMail; - - eventBus.register(new CacheClearHook(cacheManager)); - } - - private final WorkdirProvider workdirProvider; - - /** - * Creates a new RepositoryService for the given repository. - * - * @param repository the repository - * @return a implementation of RepositoryService - * for the given type of repository - * @throws RepositoryServiceNotFoundException if no repository service - * implementation for this kind of repository is available - * @throws NullPointerException if the repository is null - * @throws ScmSecurityException if current user has not read permissions - * for that repository - */ - public RepositoryService create(Repository repository) { - Preconditions.checkNotNull(repository, "repository is required"); - - // check for read permissions of current user - RepositoryPermissions.read(repository).check(); - - RepositoryService service = null; - - for (RepositoryServiceResolver resolver : resolvers) { - RepositoryServiceProvider provider = resolver.resolve(repository); - - if (provider != null) { - if (logger.isDebugEnabled()) { - logger.debug( - "create new repository service for repository {} of type {}", - repository.getName(), repository.getType()); - } - - service = new RepositoryService(cacheManager, provider, repository, - preProcessorUtil, protocolProviders, workdirProvider, eMail); - - break; - } - } - - if (service == null) { - throw new RepositoryServiceNotFoundException(repository); - } - - return service; - } } From e839ad32ea08a13d960c435b9e4299cc710c1d74 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 11:23:27 +0200 Subject: [PATCH 12/17] Use Nullable annotation to make clear that Email must be checked for null --- .../scm/repository/api/MergeCommandBuilder.java | 8 +++++--- .../scm/repository/api/ModifyCommandBuilder.java | 9 +++++---- .../scm/repository/api/RepositoryService.java | 14 +++++++++----- .../java/sonia/scm/repository/util/AuthorUtil.java | 6 ++++-- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java index a751832d8f..6cbf2caa8c 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java @@ -32,6 +32,7 @@ import sonia.scm.repository.spi.MergeConflictResult; import sonia.scm.repository.util.AuthorUtil; import sonia.scm.user.EMail; +import javax.annotation.Nullable; import java.util.Set; /** @@ -76,12 +77,13 @@ import java.util.Set; */ public class MergeCommandBuilder { - private final EMail eMail; - private final MergeCommand mergeCommand; private final MergeCommandRequest request = new MergeCommandRequest(); - MergeCommandBuilder(MergeCommand mergeCommand, EMail eMail) { + @Nullable + private final EMail eMail; + + MergeCommandBuilder(MergeCommand mergeCommand, @Nullable EMail eMail) { this.mergeCommand = mergeCommand; this.eMail = eMail; } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java index 795de5d43d..f2b11a00a0 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java @@ -38,6 +38,7 @@ import sonia.scm.repository.work.WorkdirProvider; import sonia.scm.user.EMail; import sonia.scm.util.IOUtil; +import javax.annotation.Nullable; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -52,7 +53,6 @@ import java.util.function.Consumer; * default a {@link sonia.scm.AlreadyExistsException} will be thrown) *
  • modify existing files ({@link #modifyFile(String)}
    • *
  • delete existing files ({@link #deleteFile(String)}
    • - *
  • move/rename existing files ({@link #moveFile(String, String)}
    • * * * You can collect multiple changes before they are executed with a call to {@link #execute()}. @@ -73,14 +73,15 @@ public class ModifyCommandBuilder { private static final Logger LOG = LoggerFactory.getLogger(ModifyCommandBuilder.class); - private final EMail eMail; - private final ModifyCommand command; private final File workdir; + @Nullable + private final EMail eMail; + private final ModifyCommandRequest request = new ModifyCommandRequest(); - ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider, EMail eMail) { + ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider, @Nullable EMail eMail) { this.command = command; this.workdir = workdirProvider.createNewWorkdir(); this.eMail = eMail; diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java index 029dc6fa34..aa2a41782d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java @@ -36,6 +36,7 @@ import sonia.scm.repository.spi.RepositoryServiceProvider; import sonia.scm.repository.work.WorkdirProvider; import sonia.scm.user.EMail; +import javax.annotation.Nullable; import java.io.Closeable; import java.io.IOException; import java.util.Set; @@ -85,9 +86,11 @@ public final class RepositoryService implements Closeable { private final PreProcessorUtil preProcessorUtil; private final RepositoryServiceProvider provider; private final Repository repository; - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "java:S3740"}) private final Set protocolProviders; private final WorkdirProvider workdirProvider; + + @Nullable private final EMail eMail; /** @@ -100,11 +103,12 @@ public final class RepositoryService implements Closeable { * @param eMail utility to compute email addresses if missing */ RepositoryService(CacheManager cacheManager, - RepositoryServiceProvider provider, Repository repository, + RepositoryServiceProvider provider, + Repository repository, PreProcessorUtil preProcessorUtil, - @SuppressWarnings("rawtypes") Set protocolProviders, + @SuppressWarnings({"rawtypes", "java:S3740"}) Set protocolProviders, WorkdirProvider workdirProvider, - EMail eMail) { + @Nullable EMail eMail) { this.cacheManager = cacheManager; this.provider = provider; this.repository = repository; @@ -452,7 +456,7 @@ public final class RepositoryService implements Closeable { .map(this::createProviderInstanceForRepository); } - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "java:S3740"}) private ScmProtocol createProviderInstanceForRepository(ScmProtocolProvider protocolProvider) { return protocolProvider.get(repository); } diff --git a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java index 1df0e5ef76..21142ba95d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java +++ b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java @@ -30,19 +30,21 @@ import sonia.scm.repository.Person; import sonia.scm.user.EMail; import sonia.scm.user.User; +import javax.annotation.Nullable; + public class AuthorUtil { public static void setAuthorIfNotAvailable(CommandWithAuthor request) { setAuthorIfNotAvailable(request, null); } - public static void setAuthorIfNotAvailable(CommandWithAuthor request, EMail eMail) { + public static void setAuthorIfNotAvailable(CommandWithAuthor request, @Nullable EMail eMail) { if (request.getAuthor() == null) { request.setAuthor(createAuthorFromSubject(eMail)); } } - private static Person createAuthorFromSubject(EMail eMail) { + private static Person createAuthorFromSubject(@Nullable EMail eMail) { Subject subject = SecurityUtils.getSubject(); User user = subject.getPrincipals().oneByType(User.class); String name = user.getDisplayName(); From 3cdc1faaa3ae9a6a159298ebc2deeaf683824f58 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 11:24:07 +0200 Subject: [PATCH 13/17] Improve javadoc of new configuration option mailDomainName --- .../src/main/java/sonia/scm/config/ScmConfiguration.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java b/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java index 2fed4058f7..6f1414b8e0 100644 --- a/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java +++ b/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java @@ -83,6 +83,8 @@ public class ScmConfiguration implements Configuration { /** * Default e-mail domain name that will be used whenever we have to generate an e-mail address for a user that has no * mail address configured. + * + * @since 2.8.0 */ public static final String DEFAULT_MAIL_DOMAIN_NAME = "scm-manager.local"; @@ -301,8 +303,8 @@ public class ScmConfiguration implements Configuration { } /** - * Returns the mail host, that will be used to create e-mail addresses for users without one whenever one is required. - * + * Returns the mail domain, that will be used to create e-mail addresses for users without one whenever one is required. + * @return default mail domain * @since 2.8.0 */ public String getMailDomainName() { @@ -492,7 +494,7 @@ public class ScmConfiguration implements Configuration { /** * Sets the mail host, that will be used to create e-mail addresses for users without one whenever one is required. * - * @param mailDomainName The new mail host to use. + * @param mailDomainName The default mail domain to use * @since 2.8.0 */ public void setMailDomainName(String mailDomainName) { From 775b37980d11959698158ab99d44e8ff4f75baec Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 11:24:37 +0200 Subject: [PATCH 14/17] Use pull request root instead of files --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11ae2b2fbf..e6810df880 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Added -- Generation of email addresses for users, where none is configured ([#1370](https://github.com/scm-manager/scm-manager/pull/1370/files)) +- Generation of email addresses for users, where none is configured ([#1370](https://github.com/scm-manager/scm-manager/pull/1370)) ## [2.7.1] - 2020-10-14 ### Fixed From 465a9e635bcc2d2743623032d35705339a10bf30 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 11:48:17 +0200 Subject: [PATCH 15/17] Rename createFallbackMailAddress to more explicit getMailOrFallback --- .../sonia/scm/repository/util/AuthorUtil.java | 2 +- .../src/main/java/sonia/scm/user/EMail.java | 18 ++++++++++++++++-- .../scm/repository/util/AuthorUtilTest.java | 4 +--- .../test/java/sonia/scm/user/EMailTest.java | 6 +++--- .../scm/api/v2/resources/MeDtoFactory.java | 2 +- .../scm/api/v2/resources/MeDtoFactoryTest.java | 2 +- 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java index 21142ba95d..e5a6501604 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java +++ b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java @@ -48,7 +48,7 @@ public class AuthorUtil { Subject subject = SecurityUtils.getSubject(); User user = subject.getPrincipals().oneByType(User.class); String name = user.getDisplayName(); - String mailAddress = eMail != null ? eMail.createFallbackMailAddress(user) : user.getMail(); + String mailAddress = eMail != null ? eMail.getMailOrFallback(user) : user.getMail(); return new Person(name, mailAddress); } diff --git a/scm-core/src/main/java/sonia/scm/user/EMail.java b/scm-core/src/main/java/sonia/scm/user/EMail.java index 0718a3c722..8da1377c94 100644 --- a/scm-core/src/main/java/sonia/scm/user/EMail.java +++ b/scm-core/src/main/java/sonia/scm/user/EMail.java @@ -29,6 +29,11 @@ import sonia.scm.config.ScmConfiguration; import javax.inject.Inject; +/** + * Email is able to resolve email addresses of users. + * + * @since 2.8.0 + */ public class EMail { private final ScmConfiguration scmConfiguration; @@ -38,15 +43,24 @@ public class EMail { this.scmConfiguration = scmConfiguration; } - public String createFallbackMailAddress(User user) { + /** + * Returns the email address of the given user or a generated fallback address. + * @param user user to resolve address from + * @return email address or fallback + */ + public String getMailOrFallback(User user) { if (Strings.isNullOrEmpty(user.getMail())) { if (user.getId().contains("@")) { return user.getId(); } else { - return user.getId() + "@" + scmConfiguration.getMailDomainName(); + return createFallbackMail(user); } } else { return user.getMail(); } } + + private String createFallbackMail(User user) { + return user.getId() + "@" + scmConfiguration.getMailDomainName(); + } } diff --git a/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java b/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java index efd5b0e69a..08a49cea44 100644 --- a/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java @@ -36,10 +36,8 @@ import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.repository.Person; import sonia.scm.user.EMail; import sonia.scm.user.User; -import sonia.scm.web.UserAgentParserTest; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -65,7 +63,7 @@ class AuthorUtilTest { void shouldCreateMailAddressFromEmail() { User trillian = new User("trillian"); when(subject.getPrincipals().oneByType(User.class)).thenReturn(trillian); - when(eMail.createFallbackMailAddress(trillian)).thenReturn("tricia@hitchhicker.com"); + when(eMail.getMailOrFallback(trillian)).thenReturn("tricia@hitchhicker.com"); Command command = new Command(null); AuthorUtil.setAuthorIfNotAvailable(command, eMail); diff --git a/scm-core/src/test/java/sonia/scm/user/EMailTest.java b/scm-core/src/test/java/sonia/scm/user/EMailTest.java index 21f6181757..49842223ba 100644 --- a/scm-core/src/test/java/sonia/scm/user/EMailTest.java +++ b/scm-core/src/test/java/sonia/scm/user/EMailTest.java @@ -37,7 +37,7 @@ class EMailTest { void shouldUserUsersAddressIfAvailable() { User user = new User("dent", "Arthur Dent", "arthur@hitchhiker.com"); - String mailAddress = eMail.createFallbackMailAddress(user); + String mailAddress = eMail.getMailOrFallback(user); assertThat(mailAddress).isEqualTo("arthur@hitchhiker.com"); } @@ -46,7 +46,7 @@ class EMailTest { void shouldCreateAddressIfNoneAvailable() { User user = new User("dent", "Arthur Dent", ""); - String mailAddress = eMail.createFallbackMailAddress(user); + String mailAddress = eMail.getMailOrFallback(user); assertThat(mailAddress).isEqualTo("dent@scm-manager.local"); } @@ -55,7 +55,7 @@ class EMailTest { void shouldUserUsersIdIfItLooksLikeAnMailAddress() { User user = new User("dent@hitchhiker.com", "Arthur Dent", ""); - String mailAddress = eMail.createFallbackMailAddress(user); + String mailAddress = eMail.getMailOrFallback(user); assertThat(mailAddress).isEqualTo("dent@hitchhiker.com"); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java index 2bc72d509a..ff3914654d 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java @@ -86,7 +86,7 @@ public class MeDtoFactory extends HalAppenderMapper { private void setGeneratedMail(User user, MeDto dto) { if (Strings.isNullOrEmpty(user.getMail())) { - dto.setFallbackMail(eMail.createFallbackMailAddress(user)); + dto.setFallbackMail(eMail.getMailOrFallback(user)); } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java index 2a4cea9626..ea539c4631 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java @@ -244,7 +244,7 @@ class MeDtoFactoryTest { User user = UserTestData.createTrillian(); user.setMail(null); prepareSubject(user); - when(eMail.createFallbackMailAddress(user)).thenReturn("trillian@hitchhiker.local"); + when(eMail.getMailOrFallback(user)).thenReturn("trillian@hitchhiker.local"); MeDto dto = meDtoFactory.create(); From 2ea019231a70361f84f0b7aed640c0edeedbe296 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 11:58:20 +0200 Subject: [PATCH 16/17] Document configuration for null checks --- docs/en/development/intellij-idea-configuration.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/en/development/intellij-idea-configuration.md b/docs/en/development/intellij-idea-configuration.md index f740281518..d415b4d206 100644 --- a/docs/en/development/intellij-idea-configuration.md +++ b/docs/en/development/intellij-idea-configuration.md @@ -11,6 +11,12 @@ title: Intellij IDEA Configuration ### Settings +* Build, Execution, Deployment / Compiler + * Add runtime assertions for non-null-annotated methods and parameters (must be checked) + * Configure annotation ... (of "Add runtime assertions...") + * Nullable annotations: select (✓) `javax.annotation.Nullable` + * NotNull annotations: select (✓) `javax.annotation.Nonnull` and check Instrument + * Run Configurations / Edit Configuration * Add Maven * Name: run-backend From 27236cdbbdaa5a6e54958a3618da5cae8f61fef7 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 15 Oct 2020 14:28:57 +0200 Subject: [PATCH 17/17] Use ValidatioUtil to check if the user id is a mail --- scm-core/src/main/java/sonia/scm/user/EMail.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scm-core/src/main/java/sonia/scm/user/EMail.java b/scm-core/src/main/java/sonia/scm/user/EMail.java index 8da1377c94..e525b8d46c 100644 --- a/scm-core/src/main/java/sonia/scm/user/EMail.java +++ b/scm-core/src/main/java/sonia/scm/user/EMail.java @@ -26,6 +26,7 @@ package sonia.scm.user; import com.google.common.base.Strings; import sonia.scm.config.ScmConfiguration; +import sonia.scm.util.ValidationUtil; import javax.inject.Inject; @@ -50,7 +51,7 @@ public class EMail { */ public String getMailOrFallback(User user) { if (Strings.isNullOrEmpty(user.getMail())) { - if (user.getId().contains("@")) { + if (isMailUsedAsId(user)) { return user.getId(); } else { return createFallbackMail(user); @@ -60,6 +61,10 @@ public class EMail { } } + private boolean isMailUsedAsId(User user) { + return ValidationUtil.isMailAddressValid(user.getId()); + } + private String createFallbackMail(User user) { return user.getId() + "@" + scmConfiguration.getMailDomainName(); }