fix review findings

This commit is contained in:
Eduard Heimbuch
2020-09-24 10:26:07 +02:00
parent ff6aaf7bc5
commit 804a6498a7
19 changed files with 176 additions and 128 deletions

View File

@@ -37,6 +37,7 @@ public class XmlUTCDateAdapter extends XmlAdapter<String, Date> {
@Override
public String marshal(Date date) {
return date.toString();
SimpleDateFormat formatter = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz");
return formatter.format(date);
}
}

View File

@@ -61,7 +61,7 @@
"realmDescriptionHelpText": "Beschreibung des Authentication Realm.",
"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. Damit kann über neuere Versionen informiert werden.",
"releaseFeedUrlHelpText": "Die URL des RSS Release Feed des SCM-Manager. Darüber wird über die neue SCM-Manager Versionen informiert. Um diese Funktion zu deaktivieren lassen Sie dieses Feld leer.",
"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.",

View File

@@ -61,7 +61,7 @@
"realmDescriptionHelpText": "Enter authentication realm description.",
"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 is needed to inform about newer versions.",
"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.",
"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.",

View File

@@ -26,20 +26,20 @@ import { connect } from "react-redux";
import { WithTranslation, withTranslation } from "react-i18next";
import styled from "styled-components";
import { apiClient, Image, Loading, Subtitle, Title } from "@scm-manager/ui-components";
import { getAppVersion, getReleaseInfoLink } from "../../modules/indexResource";
import { getAppVersion, getUpdateInfoLink } from "../../modules/indexResource";
type Props = WithTranslation & {
version: string;
releaseInfoLink?: string;
updateInfoLink?: string;
};
type State = {
loading: boolean;
releaseInfo?: ReleaseInfo;
updateInfo?: UpdateInfo;
};
type ReleaseInfo = {
version: string;
type UpdateInfo = {
latestVersion: string;
link: string;
};
@@ -69,34 +69,31 @@ class AdminDetails extends React.Component<Props, State> {
}
componentDidMount() {
const { releaseInfoLink } = this.props;
const { updateInfoLink } = this.props;
if (releaseInfoLink) {
this.setState({ loading: true });
if (updateInfoLink) {
this.setState({ loading: true }, () =>
apiClient
.get(releaseInfoLink)
.get(updateInfoLink)
.then(r => r.json())
.then(releaseInfo => this.setState({ releaseInfo }))
.then(updateInfo => this.setState({ updateInfo }))
.then(() => this.setState({ loading: false }))
// ignore errors for this action
.catch(() => this.setState({ loading: false }));
.catch(() => this.setState({ loading: false }))
);
}
}
render() {
const { version, t } = this.props;
const { loading, releaseInfo } = this.state;
renderUpdateInfo() {
const { loading, updateInfo } = this.state;
const { t } = this.props;
if (loading) {
return <Loading />;
}
return (
<>
<Title title={t("admin.info.title")} />
<NoBottomMarginSubtitle subtitle={t("admin.info.currentAppVersion")} />
<BottomMarginDiv>{version}</BottomMarginDiv>
{releaseInfo && (
updateInfo && (
<>
<BoxShadowBox className="box">
<article className="media">
@@ -108,10 +105,10 @@ class AdminDetails extends React.Component<Props, State> {
<h3 className="has-text-weight-medium">{t("admin.info.newRelease.title")}</h3>
<p>
{t("admin.info.newRelease.description", {
version: releaseInfo?.version
version: updateInfo?.latestVersion
})}
</p>
<a className="button is-info is-pulled-right" target="_blank" href={releaseInfo?.link}>
<a className="button is-warning is-pulled-right" target="_blank" href={updateInfo?.link}>
{t("admin.info.newRelease.downloadButton")}
</a>
</div>
@@ -120,7 +117,19 @@ class AdminDetails extends React.Component<Props, State> {
</BoxShadowBox>
<hr />
</>
)}
)
);
}
render() {
const { version, t } = this.props;
return (
<>
<Title title={t("admin.info.title")} />
<NoBottomMarginSubtitle subtitle={t("admin.info.currentAppVersion")} />
<BottomMarginDiv>{version}</BottomMarginDiv>
{this.renderUpdateInfo()}
<BoxShadowBox className="box">
<article className="media">
<ImageWrapper className="media-left">
@@ -168,10 +177,10 @@ class AdminDetails extends React.Component<Props, State> {
const mapStateToProps = (state: any) => {
const version = getAppVersion(state);
const releaseInfoLink = getReleaseInfoLink(state);
const updateInfoLink = getUpdateInfoLink(state);
return {
version,
releaseInfoLink
updateInfoLink
};
};

View File

@@ -158,8 +158,8 @@ export function getAppVersion(state: object) {
return state.indexResources.version;
}
export function getReleaseInfoLink(state: object) {
return getLink(state, "releaseInfo");
export function getUpdateInfoLink(state: object) {
return getLink(state, "updateInfo");
}
export function getUiPluginsLink(state: object) {

View File

@@ -60,12 +60,6 @@ public final class ReleaseFeedDto {
@Setter
public static class Channel {
private String title;
private String description;
private String link;
private String generator;
@XmlJavaTypeAdapter(XmlUTCDateAdapter.class)
private Date lastBuildDate;
@XmlElement(name = "item")
private List<Release> releases;
}

View File

@@ -24,9 +24,11 @@
package sonia.scm.admin;
import com.google.common.base.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.net.ahc.AdvancedHttpClient;
import sonia.scm.version.Version;
import javax.inject.Inject;
import java.io.IOException;
@@ -44,18 +46,21 @@ public class ReleaseFeedParser {
this.client = client;
}
Optional<ReleaseInfo> findLatestRelease(String url) {
Optional<UpdateInfo> findLatestRelease(String url) {
LOG.info("Search for newer versions of SCM-Manager");
Optional<ReleaseFeedDto.Release> latestRelease = parseLatestReleaseFromRssFeed(url);
return latestRelease.map(release -> new ReleaseInfo(release.getTitle(), release.getLink()));
return latestRelease.map(release -> new UpdateInfo(release.getTitle(), release.getLink()));
}
private Optional<ReleaseFeedDto.Release> parseLatestReleaseFromRssFeed(String url) {
try {
if (Strings.isNullOrEmpty(url)) {
return Optional.empty();
}
ReleaseFeedDto releaseFeed = client.get(url).request().contentFromXml(ReleaseFeedDto.class);
return filterForLatestRelease(releaseFeed);
} catch (IOException e) {
LOG.error(String.format("Could not parse release feed from %s", url));
LOG.error("Could not parse release feed from {}", url, e);
return Optional.empty();
}
}
@@ -63,6 +68,6 @@ public class ReleaseFeedParser {
private Optional<ReleaseFeedDto.Release> filterForLatestRelease(ReleaseFeedDto releaseFeed) {
return releaseFeed.getChannel().getReleases()
.stream()
.max(Comparator.comparing(ReleaseFeedDto.Release::getPubDate));
.min(Comparator.comparing(release -> Version.parse(release.getTitle().trim())));
}
}

View File

@@ -39,12 +39,13 @@ import java.util.Optional;
public class ReleaseVersionChecker {
private static final Logger LOG = LoggerFactory.getLogger(ReleaseVersionChecker.class);
private static final String CACHE_NAME = "sonia.cache.releaseInfo";
private static final String CACHE_NAME = "sonia.cache.updateInfo";
private static final String CACHE_KEY = "latestRelease";
private final ReleaseFeedParser releaseFeedParser;
private final ScmConfiguration scmConfiguration;
private final SCMContextProvider scmContextProvider;
private Cache<String, ReleaseInfo> cache;
private Cache<String, Optional<UpdateInfo>> cache;
@Inject
public ReleaseVersionChecker(ReleaseFeedParser releaseFeedParser, ScmConfiguration scmConfiguration, SCMContextProvider scmContextProvider, CacheManager cacheManager) {
@@ -55,32 +56,32 @@ public class ReleaseVersionChecker {
}
@VisibleForTesting
void setCache(Cache<String, ReleaseInfo> cache) {
void setCache(Cache<String, Optional<UpdateInfo>> cache) {
this.cache = cache;
}
public Optional<ReleaseInfo> checkForNewerVersion() {
ReleaseInfo cachedReleaseInfo = cache.get("latest");
if (cachedReleaseInfo != null) {
return Optional.of(cachedReleaseInfo);
} else {
public Optional<UpdateInfo> checkForNewerVersion() {
if (cache.size() > 0) {
return cache.get(CACHE_KEY);
}
return findLatestRelease();
}
}
private Optional<ReleaseInfo> findLatestRelease() {
private Optional<UpdateInfo> findLatestRelease() {
String releaseFeedUrl = scmConfiguration.getReleaseFeedUrl();
Optional<ReleaseInfo> latestRelease = releaseFeedParser.findLatestRelease(releaseFeedUrl);
Optional<UpdateInfo> latestRelease = releaseFeedParser.findLatestRelease(releaseFeedUrl);
if (latestRelease.isPresent() && isNewerVersion(latestRelease.get())) {
cache.put("latest", latestRelease.get());
cache.put(CACHE_KEY, latestRelease);
return latestRelease;
}
// we cache that no new version was available to prevent request every time
LOG.info("No newer version found for SCM-Manager");
cache.put(CACHE_KEY, Optional.empty());
return Optional.empty();
}
private boolean isNewerVersion(ReleaseInfo releaseInfo) {
Version versionFromReleaseFeed = Version.parse(releaseInfo.getVersion());
private boolean isNewerVersion(UpdateInfo updateInfo) {
Version versionFromReleaseFeed = Version.parse(updateInfo.getLatestVersion());
return versionFromReleaseFeed.isNewer(scmContextProvider.getVersion());
}
}

View File

@@ -29,7 +29,7 @@ import lombok.Getter;
@AllArgsConstructor
@Getter
public class ReleaseInfo {
private final String version;
public class UpdateInfo {
private final String latestVersion;
private final String link;
}

View File

@@ -30,8 +30,7 @@ import io.swagger.v3.oas.annotations.media.Content;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.tags.Tag;
import sonia.scm.admin.ReleaseInfo;
import sonia.scm.admin.ReleaseInfoMapper;
import sonia.scm.admin.UpdateInfo;
import sonia.scm.admin.ReleaseVersionChecker;
import sonia.scm.web.VndMediaType;
@@ -44,14 +43,14 @@ import java.util.Optional;
@OpenAPIDefinition(tags = {
@Tag(name = "AdminInfo", description = "Admin information endpoints")
})
@Path("")
@Path("v2")
public class AdminInfoResource {
private final ReleaseVersionChecker checker;
private final ReleaseInfoMapper mapper;
private final UpdateInfoMapper mapper;
@Inject
public AdminInfoResource(ReleaseVersionChecker checker, ReleaseInfoMapper mapper) {
public AdminInfoResource(ReleaseVersionChecker checker, UpdateInfoMapper mapper) {
this.checker = checker;
this.mapper = mapper;
}
@@ -60,12 +59,11 @@ public class AdminInfoResource {
* Checks for a newer core version of SCM-Manager.
*/
@GET
@Path("releaseInfo")
@Path("updateInfo")
@Produces(VndMediaType.ADMIN_INFO)
@Operation(summary = "Returns release info.", description = "Returns information about the latest release if a newer version of SCM-Manager is available.", tags = "AdminInfo")
@ApiResponse(responseCode = "200", description = "success")
@ApiResponse(responseCode = "401", description = "not authenticated / invalid credentials")
@ApiResponse(responseCode = "403", description = "not authorized, the current user has no privileges to read the information")
@ApiResponse(
responseCode = "500",
description = "internal server error",
@@ -73,8 +71,8 @@ public class AdminInfoResource {
mediaType = VndMediaType.ERROR_TYPE,
schema = @Schema(implementation = ErrorDto.class)
))
public ReleaseInfoDto getReleaseInfo() {
Optional<ReleaseInfo> releaseInfo = checker.checkForNewerVersion();
return releaseInfo.map(mapper::map).orElse(null);
public UpdateInfoDto getUpdateInfo() {
Optional<UpdateInfo> updateInfo = checker.checkForNewerVersion();
return updateInfo.map(mapper::map).orElse(null);
}
}

View File

@@ -102,7 +102,7 @@ public class IndexDtoGenerator extends HalAppenderMapper {
}
if (ConfigurationPermissions.list().isPermitted()) {
builder.single(link("config", resourceLinks.config().self()));
builder.single(link("releaseInfo", resourceLinks.adminInfo().releaseInfo()));
builder.single(link("updateInfo", resourceLinks.adminInfo().updateInfo()));
}
builder.single(link("repositories", resourceLinks.repositoryCollection().self()));
builder.single(link("namespaces", resourceLinks.namespaceCollection().self()));

View File

@@ -27,7 +27,6 @@ package sonia.scm.api.v2.resources;
import com.google.inject.AbstractModule;
import com.google.inject.servlet.ServletScopes;
import org.mapstruct.factory.Mappers;
import sonia.scm.admin.ReleaseInfoMapper;
import sonia.scm.security.gpg.PublicKeyMapper;
import sonia.scm.web.api.RepositoryToHalMapper;
@@ -77,7 +76,7 @@ public class MapperModule extends AbstractModule {
bind(RepositoryToHalMapper.class).to(Mappers.getMapperClass(RepositoryToRepositoryDtoMapper.class));
bind(BlameResultToBlameDtoMapper.class).to(Mappers.getMapperClass(BlameResultToBlameDtoMapper.class));
bind(ReleaseInfoMapper.class).to(Mappers.getMapperClass(ReleaseInfoMapper.class));
bind(UpdateInfoMapper.class).to(Mappers.getMapperClass(UpdateInfoMapper.class));
// no mapstruct required
bind(MeDtoFactory.class);

View File

@@ -275,8 +275,8 @@ class ResourceLinks {
adminInfoLinkBuilder = new LinkBuilder(pathInfo, AdminInfoResource.class);
}
String releaseInfo() {
return adminInfoLinkBuilder.method("getReleaseInfo").parameters().href();
String updateInfo() {
return adminInfoLinkBuilder.method("getUpdateInfo").parameters().href();
}
}

View File

@@ -24,17 +24,25 @@
package sonia.scm.api.v2.resources;
import de.otto.edison.hal.Embedded;
import de.otto.edison.hal.HalRepresentation;
import de.otto.edison.hal.Links;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
@AllArgsConstructor
@NoArgsConstructor
@Setter
@Getter
@SuppressWarnings("squid:S2160") // we do not need equals for dto
public class ReleaseInfoDto extends HalRepresentation {
private String version;
public class UpdateInfoDto extends HalRepresentation {
private String latestVersion;
private String link;
UpdateInfoDto(Links links, Embedded embedded) {
super(links, embedded);
}
}

View File

@@ -22,16 +22,33 @@
* SOFTWARE.
*/
package sonia.scm.admin;
package sonia.scm.api.v2.resources;
import de.otto.edison.hal.Embedded;
import de.otto.edison.hal.Links;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import sonia.scm.api.v2.resources.HalAppenderMapper;
import sonia.scm.api.v2.resources.ReleaseInfoDto;
import org.mapstruct.ObjectFactory;
import sonia.scm.admin.UpdateInfo;
import javax.inject.Inject;
import static de.otto.edison.hal.Links.linkingTo;
@Mapper
public abstract class ReleaseInfoMapper extends HalAppenderMapper {
public abstract class UpdateInfoMapper extends HalAppenderMapper {
@Inject
private ResourceLinks resourceLinks;
@Mapping(target = "attributes", ignore = true) // We do not map HAL attributes
public abstract ReleaseInfoDto map(ReleaseInfo releaseInfo);
public abstract UpdateInfoDto map(UpdateInfo updateInfo);
@ObjectFactory
UpdateInfoDto createDto() {
Links.Builder linksBuilder = linkingTo()
.self(resourceLinks.adminInfo().updateInfo());
return new UpdateInfoDto(linksBuilder.build(), Embedded.emptyEmbedded());
}
}

View File

@@ -67,11 +67,11 @@
/>
<!--
ReleaseInfo cache
UpdateInfo cache
average: 30K
-->
<cache
name="sonia.cache.releaseInfo"
name="sonia.cache.updateInfo"
maximumSize="1"
expireAfterWrite="3600"
/>

View File

@@ -55,10 +55,10 @@ class ReleaseFeedParserTest {
when(client.get(url).request().contentFromXml(ReleaseFeedDto.class)).thenReturn(createReleaseFeedDto());
Optional<ReleaseInfo> release = releaseFeedParser.findLatestRelease(url);
Optional<UpdateInfo> release = releaseFeedParser.findLatestRelease(url);
assertThat(release).isPresent();
assertThat(release.get().getVersion()).isEqualTo("3");
assertThat(release.get().getLatestVersion()).isEqualTo("3");
assertThat(release.get().getLink()).isEqualTo("download-3");
}
@@ -66,7 +66,7 @@ class ReleaseFeedParserTest {
ReleaseFeedDto.Release release1 = createRelease("1", "download-1", new Date(1000000000L));
ReleaseFeedDto.Release release2 = createRelease("2", "download-2", new Date(2000000000L));
ReleaseFeedDto.Release release3 = createRelease("3", "download-3", new Date(3000000000L));
ReleaseFeedDto.Channel channel = new ReleaseFeedDto.Channel("scm", "scm releases", "scm-download", "gatsby", new Date(1L), ImmutableList.of(release1, release2, release3));
ReleaseFeedDto.Channel channel = new ReleaseFeedDto.Channel(ImmutableList.of(release1, release2, release3));
return new ReleaseFeedDto(channel);
}

View File

@@ -60,40 +60,40 @@ class ReleaseVersionCheckerTest {
}
@Test
void shouldReturnEmptyOptional() throws IOException {
void shouldReturnEmptyOptional() {
when(scmConfiguration.getReleaseFeedUrl()).thenReturn("releaseFeed");
when(feedReader.findLatestRelease("releaseFeed")).thenReturn(Optional.empty());
Optional<ReleaseInfo> releaseInfo = checker.checkForNewerVersion();
Optional<UpdateInfo> updateInfo = checker.checkForNewerVersion();
assertThat(releaseInfo).isNotPresent();
assertThat(updateInfo).isNotPresent();
}
@Test
void shouldReturnReleaseInfoFromCache() {
ReleaseInfo cachedReleaseInfo = new ReleaseInfo("1.42.9", "download-link");
Cache<String, ReleaseInfo> cache = new MapCacheManager().getCache("sonia.cache.releaseInfo");
cache.put("latest", cachedReleaseInfo);
void shouldReturnUpdateInfoFromCache() {
UpdateInfo cachedUpdateInfo = new UpdateInfo("1.42.9", "download-link");
Cache<String, Optional<UpdateInfo>> cache = new MapCacheManager().getCache("sonia.cache.updateInfo");
cache.put("latestRelease", Optional.of(cachedUpdateInfo));
checker.setCache(cache);
Optional<ReleaseInfo> releaseInfo = checker.checkForNewerVersion();
Optional<UpdateInfo> updateInfo = checker.checkForNewerVersion();
assertThat(releaseInfo).isPresent();
assertThat(releaseInfo.get().getVersion()).isEqualTo("1.42.9");
assertThat(releaseInfo.get().getLink()).isEqualTo("download-link");
assertThat(updateInfo).isPresent();
assertThat(updateInfo.get().getLatestVersion()).isEqualTo("1.42.9");
assertThat(updateInfo.get().getLink()).isEqualTo("download-link");
}
@Test
void shouldReturnReleaseInfo() throws IOException {
ReleaseInfo releaseInfo = new ReleaseInfo("2.5.0", "download-link");
void shouldReturnUpdateInfo() {
UpdateInfo updateInfo = new UpdateInfo("2.5.0", "download-link");
when(scmConfiguration.getReleaseFeedUrl()).thenReturn("releaseFeed");
when(feedReader.findLatestRelease("releaseFeed")).thenReturn(Optional.of(releaseInfo));
when(feedReader.findLatestRelease("releaseFeed")).thenReturn(Optional.of(updateInfo));
when(contextProvider.getVersion()).thenReturn("1.9.0");
Optional<ReleaseInfo> latestRelease = checker.checkForNewerVersion();
Optional<UpdateInfo> latestRelease = checker.checkForNewerVersion();
assertThat(latestRelease).isPresent();
assertThat(latestRelease.get().getVersion()).isEqualTo("2.5.0");
assertThat(latestRelease.get().getLatestVersion()).isEqualTo("2.5.0");
assertThat(latestRelease.get().getLink()).isEqualTo("download-link");
}
}

View File

@@ -22,25 +22,41 @@
* SOFTWARE.
*/
package sonia.scm.admin;
package sonia.scm.api.v2.resources;
import org.junit.jupiter.api.Test;
import sonia.scm.api.v2.resources.ReleaseInfoDto;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.junit.jupiter.MockitoExtension;
import sonia.scm.admin.UpdateInfo;
import sonia.scm.api.v2.resources.UpdateInfoDto;
import sonia.scm.api.v2.resources.UpdateInfoMapper;
import sonia.scm.api.v2.resources.UpdateInfoMapperImpl;
import sonia.scm.api.v2.resources.ResourceLinks;
import sonia.scm.api.v2.resources.ResourceLinksMock;
import java.time.Instant;
import java.net.URI;
import static org.assertj.core.api.Assertions.assertThat;
class ReleaseInfoMapperTest {
@ExtendWith(MockitoExtension.class)
class UpdateInfoMapperTest {
ReleaseInfoMapper mapper = new ReleaseInfoMapperImpl();
private final URI baseUri = URI.create("https://hitchhiker.com/scm/");
@SuppressWarnings("unused") // Is injected
private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri);
@InjectMocks
private UpdateInfoMapperImpl mapper;
@Test
void shouldMapToDto() {
ReleaseInfo releaseInfo = new ReleaseInfo("1.2.3", "download-link");
ReleaseInfoDto dto = mapper.map(releaseInfo);
UpdateInfo updateInfo = new UpdateInfo("1.2.3", "download-link");
UpdateInfoDto dto = mapper.map(updateInfo);
assertThat(dto.getLink()).isEqualTo(releaseInfo.getLink());
assertThat(dto.getVersion()).isEqualTo(releaseInfo.getVersion());
assertThat(dto.getLink()).isEqualTo(updateInfo.getLink());
assertThat(dto.getLatestVersion()).isEqualTo(updateInfo.getLatestVersion());
assertThat(dto.getLinks().getLinkBy("self").get().getHref()).isEqualTo("https://hitchhiker.com/scm/updateInfo");
}
}