From 716c7c508326d15978c69487d8f914004fb9692f Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 27 Aug 2020 08:11:23 +0200 Subject: [PATCH 01/30] Fix detection of markdown files for files having content does not start with '#' --- CHANGELOG.md | 1 + .../repos/sources/containers/SourcesView.tsx | 2 +- scm-webapp/pom.xml | 2 +- .../sonia/scm/api/v2/ContentTypeResolver.java | 51 +++++++++++++++ .../scm/api/v2/resources/ContentResource.java | 4 +- .../DiffResultToDiffResultDtoMapper.java | 4 +- .../scm/api/v2/ContentTypeResolverTest.java | 65 +++++++++++++++++++ 7 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/ContentTypeResolver.java create mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/ContentTypeResolverTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 17064befab..c5dec347ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Fixed +- Fix detection of markdown files for files having content does not start with '#' ([#1306](https://github.com/scm-manager/scm-manager/pull/1306)) - Fix broken markdown rendering ([#1303](https://github.com/scm-manager/scm-manager/pull/1303)) - JWT token timeout is now handled properly ([#1297](https://github.com/scm-manager/scm-manager/pull/1297)) - Fix text-overflow in danger zone ([#1298](https://github.com/scm-manager/scm-manager/pull/1298)) diff --git a/scm-ui/ui-webapp/src/repos/sources/containers/SourcesView.tsx b/scm-ui/ui-webapp/src/repos/sources/containers/SourcesView.tsx index 417525c5f6..9cf3cfc3f3 100644 --- a/scm-ui/ui-webapp/src/repos/sources/containers/SourcesView.tsx +++ b/scm-ui/ui-webapp/src/repos/sources/containers/SourcesView.tsx @@ -87,7 +87,7 @@ class SourcesView extends React.Component { const basePath = this.createBasePath(); if (contentType.startsWith("image/")) { return ; - } else if (contentType.includes("markdown")) { + } else if (contentType.includes("markdown") || (language && language.toLowerCase() === "markdown")) { return ; } else if (language) { return ; diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index b34eb7b5e8..e1db3ffc31 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -315,7 +315,7 @@ com.github.sdorra spotter-core - 2.1.2 + 3.0.0 diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/ContentTypeResolver.java b/scm-webapp/src/main/java/sonia/scm/api/v2/ContentTypeResolver.java new file mode 100644 index 0000000000..ff993689d0 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/ContentTypeResolver.java @@ -0,0 +1,51 @@ +/* + * 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.api.v2; + +import com.github.sdorra.spotter.ContentType; +import com.github.sdorra.spotter.ContentTypeDetector; +import com.github.sdorra.spotter.Language; + +public final class ContentTypeResolver { + + private static final ContentTypeDetector PATH_BASED = ContentTypeDetector.builder() + .defaultPathBased().boost(Language.MARKDOWN) + .bestEffortMatch(); + + private static final ContentTypeDetector PATH_AND_CONTENT_BASED = ContentTypeDetector.builder() + .defaultPathAndContentBased().boost(Language.MARKDOWN) + .bestEffortMatch(); + + private ContentTypeResolver() { + } + + public static ContentType resolve(String path) { + return PATH_BASED.detect(path); + } + + public static ContentType resolve(String path, byte[] contentPrefix) { + return PATH_AND_CONTENT_BASED.detect(path, contentPrefix); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java index 74d1d2deeb..b58be79e51 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ContentResource.java @@ -25,7 +25,6 @@ package sonia.scm.api.v2.resources; import com.github.sdorra.spotter.ContentType; -import com.github.sdorra.spotter.ContentTypes; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.media.Content; @@ -34,6 +33,7 @@ import io.swagger.v3.oas.annotations.responses.ApiResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.NotFoundException; +import sonia.scm.api.v2.ContentTypeResolver; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.api.RepositoryService; import sonia.scm.repository.api.RepositoryServiceFactory; @@ -204,7 +204,7 @@ public class ContentResource { } private void appendContentHeader(String path, byte[] head, Response.ResponseBuilder responseBuilder) { - ContentType contentType = ContentTypes.detect(path, head); + ContentType contentType = ContentTypeResolver.resolve(path, head); responseBuilder.header("Content-Type", contentType.getRaw()); contentType.getLanguage().ifPresent( language -> responseBuilder.header(ProgrammingLanguages.HEADER, ProgrammingLanguages.getValue(language)) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java index 0384ed7d45..955f11ae12 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java @@ -24,10 +24,10 @@ package sonia.scm.api.v2.resources; -import com.github.sdorra.spotter.ContentTypes; import com.github.sdorra.spotter.Language; import com.google.inject.Inject; import de.otto.edison.hal.Links; +import sonia.scm.api.v2.ContentTypeResolver; import sonia.scm.repository.Repository; import sonia.scm.repository.api.DiffFile; import sonia.scm.repository.api.DiffLine; @@ -120,7 +120,7 @@ class DiffResultToDiffResultDtoMapper { dto.setOldRevision(file.getOldRevision()); - Optional language = ContentTypes.detect(path).getLanguage(); + Optional language = ContentTypeResolver.resolve(path).getLanguage(); language.ifPresent(value -> dto.setLanguage(ProgrammingLanguages.getValue(value))); List hunks = new ArrayList<>(); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/ContentTypeResolverTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/ContentTypeResolverTest.java new file mode 100644 index 0000000000..d68770214c --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/ContentTypeResolverTest.java @@ -0,0 +1,65 @@ +/* + * 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.api.v2; + +import com.github.sdorra.spotter.ContentType; +import com.github.sdorra.spotter.Language; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; + +import static org.assertj.core.api.Assertions.assertThat; + +class ContentTypeResolverTest { + + @Test + void shouldResolveMarkdown() { + String content = String.join("\n", + "% Markdown content", + "% Which does not start with markdown" + ); + ContentType contentType = ContentTypeResolver.resolve("somedoc.md", content.getBytes(StandardCharsets.UTF_8)); + assertThat(contentType.getLanguage()).contains(Language.MARKDOWN); + } + + @Test + void shouldResolveMarkdownWithoutContent() { + ContentType contentType = ContentTypeResolver.resolve("somedoc.md"); + assertThat(contentType.getLanguage()).contains(Language.MARKDOWN); + } + + @Test + void shouldResolveMarkdownEvenWithDotsInFilename() { + ContentType contentType = ContentTypeResolver.resolve("somedoc.1.1.md"); + assertThat(contentType.getLanguage()).contains(Language.MARKDOWN); + } + + @Test + void shouldResolveDockerfile() { + ContentType contentType = ContentTypeResolver.resolve("Dockerfile"); + assertThat(contentType.getLanguage()).contains(Language.DOCKERFILE); + } + +} From 20d34a84912aea90a1ff2e6a118d260bdf370e01 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 27 Aug 2020 10:51:21 +0200 Subject: [PATCH 02/30] Replace refresh of token expired banner with hard login link --- .../ui-components/src/ErrorNotification.tsx | 17 +++-- scm-ui/ui-components/src/apiclient.ts | 20 ++---- scm-ui/ui-webapp/src/containers/Login.test.ts | 62 +++++++++++++++++++ scm-ui/ui-webapp/src/containers/Login.tsx | 21 +++++-- 4 files changed, 96 insertions(+), 24 deletions(-) create mode 100644 scm-ui/ui-webapp/src/containers/Login.test.ts diff --git a/scm-ui/ui-components/src/ErrorNotification.tsx b/scm-ui/ui-components/src/ErrorNotification.tsx index 00524efa4e..c43ef73119 100644 --- a/scm-ui/ui-components/src/ErrorNotification.tsx +++ b/scm-ui/ui-components/src/ErrorNotification.tsx @@ -21,16 +21,26 @@ * 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 React, { FC } from "react"; +import { useTranslation, WithTranslation, withTranslation } from "react-i18next"; import { BackendError, ForbiddenError, UnauthorizedError } from "./errors"; import Notification from "./Notification"; import BackendErrorNotification from "./BackendErrorNotification"; +import { useLocation } from "react-router-dom"; +import { withContextPath } from "./urls"; type Props = WithTranslation & { error?: Error; }; +const LoginLink: FC = () => { + const [t] = useTranslation("commons"); + const location = useLocation(); + const from = encodeURIComponent(location.pathname); + + return {t("errorNotification.loginLink")}; +}; + class ErrorNotification extends React.Component { render() { const { t, error } = this.props; @@ -40,8 +50,7 @@ class ErrorNotification extends React.Component { } else if (error instanceof UnauthorizedError) { return ( - {t("errorNotification.prefix")}: {t("errorNotification.timeout")}{" "} - {t("errorNotification.loginLink")} + {t("errorNotification.prefix")}: {t("errorNotification.timeout")} ); } else if (error instanceof ForbiddenError) { diff --git a/scm-ui/ui-components/src/apiclient.ts b/scm-ui/ui-components/src/apiclient.ts index a371b06571..361038208e 100644 --- a/scm-ui/ui-components/src/apiclient.ts +++ b/scm-ui/ui-components/src/apiclient.ts @@ -125,23 +125,15 @@ const applyFetchOptions: (p: RequestInit) => RequestInit = o => { function handleFailure(response: Response) { if (!response.ok) { - if (isBackendError(response)) { + if (response.status === 401) { + throw new UnauthorizedError("Unauthorized", 401); + } else if (response.status === 403) { + throw new ForbiddenError("Forbidden", 403); + } else if (isBackendError(response)) { return response.json().then((content: BackendErrorContent) => { - if (content.errorCode === TOKEN_EXPIRED_ERROR_CODE) { - window.location.replace(`${contextPath}/login`); - // Throw error because if redirect is not instantaneous, we want to display something senseful - throw new UnauthorizedError("Unauthorized", 401); - } else { - throw createBackendError(content, response.status); - } + throw createBackendError(content, response.status); }); } else { - if (response.status === 401) { - throw new UnauthorizedError("Unauthorized", 401); - } else if (response.status === 403) { - throw new ForbiddenError("Forbidden", 403); - } - throw new Error("server returned status code " + response.status); } } diff --git a/scm-ui/ui-webapp/src/containers/Login.test.ts b/scm-ui/ui-webapp/src/containers/Login.test.ts new file mode 100644 index 0000000000..5e8d4a2b81 --- /dev/null +++ b/scm-ui/ui-webapp/src/containers/Login.test.ts @@ -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. + */ + +import { from } from "./Login"; + +describe("from tests", () => { + it("should use default location", () => { + const path = from("", {}); + expect(path).toBe("/"); + }); + + it("should use default location without params", () => { + const path = from(); + expect(path).toBe("/"); + }); + + it("should use default location with null params", () => { + const path = from("", null); + expect(path).toBe("/"); + }); + + it("should use location from query parameter", () => { + const path = from("from=/repos", {}); + expect(path).toBe("/repos"); + }); + + it("should use location from state", () => { + const path = from("", { from: "/users" }); + expect(path).toBe("/users"); + }); + + it("should prefer location from query parameter", () => { + const path = from("from=/groups", { from: "/users" }); + expect(path).toBe("/groups"); + }); + + it("should decode query param", () => { + const path = from(`from=${encodeURIComponent("/admin/plugins/installed")}`); + expect(path).toBe("/admin/plugins/installed"); + }); +}); diff --git a/scm-ui/ui-webapp/src/containers/Login.tsx b/scm-ui/ui-webapp/src/containers/Login.tsx index f40eef5921..8c25c6a12e 100644 --- a/scm-ui/ui-webapp/src/containers/Login.tsx +++ b/scm-ui/ui-webapp/src/containers/Login.tsx @@ -30,6 +30,7 @@ import { getLoginFailure, getMe, isAnonymous, isLoginPending, login } from "../m import { getLoginInfoLink, getLoginLink } from "../modules/indexResource"; import LoginInfo from "../components/LoginInfo"; import { Me } from "@scm-manager/ui-types"; +import { parse } from "query-string"; type Props = RouteComponentProps & { authenticated: boolean; @@ -47,6 +48,18 @@ const HeroSection = styled.section` padding-top: 2em; `; +interface FromObject { + from?: string; +} + +/** + * @visibleForTesting + */ +export const from = (queryString?: string, stateParams?: FromObject | null): string => { + const queryParams = parse(queryString || ""); + return queryParams?.from || stateParams?.from || "/"; +}; + class Login extends React.Component { handleLogin = (username: string, password: string): void => { const { link, login } = this.props; @@ -54,12 +67,8 @@ class Login extends React.Component { }; renderRedirect = () => { - const { from } = this.props.location.state || { - from: { - pathname: "/" - } - }; - return ; + const to = from(window.location.search, this.props.location.state); + return ; }; render() { From b4c5f498583c8520bb7b13caff9404dff40c97b5 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Thu, 27 Aug 2020 13:20:43 +0200 Subject: [PATCH 03/30] Bugfix/link not found (#1296) Redirect to login page if anonymous tries to access a page without permission Co-authored-by: Eduard Heimbuch Co-authored-by: Sebastian Sdorra --- scm-ui/ui-components/.storybook/config.js | 3 + scm-ui/ui-components/.storybook/withRedux.js | 41 +++++++++++ scm-ui/ui-components/src/ErrorBoundary.tsx | 69 +++++++++++++++--- scm-ui/ui-components/src/ProtectedRoute.tsx | 17 +++-- scm-ui/ui-components/src/errors.ts | 4 ++ .../ui-webapp/src/admin/containers/Admin.tsx | 6 +- .../src/admin/containers/GlobalConfig.tsx | 4 +- .../plugins/containers/PluginsOverview.tsx | 13 ++-- .../roles/containers/SingleRepositoryRole.tsx | 4 +- scm-ui/ui-webapp/src/containers/Main.tsx | 70 ++++++++++--------- .../src/groups/containers/CreateGroup.tsx | 7 +- .../src/groups/containers/Groups.tsx | 4 +- .../src/groups/containers/SingleGroup.tsx | 4 +- scm-ui/ui-webapp/src/modules/indexResource.ts | 34 ++++++++- .../src/users/containers/CreateUser.tsx | 4 +- .../src/users/containers/SingleUser.tsx | 5 +- .../ui-webapp/src/users/containers/Users.tsx | 4 +- 17 files changed, 216 insertions(+), 77 deletions(-) create mode 100644 scm-ui/ui-components/.storybook/withRedux.js diff --git a/scm-ui/ui-components/.storybook/config.js b/scm-ui/ui-components/.storybook/config.js index ae409e6478..af032d4054 100644 --- a/scm-ui/ui-components/.storybook/config.js +++ b/scm-ui/ui-components/.storybook/config.js @@ -29,6 +29,7 @@ import { withI18next } from "storybook-addon-i18next"; import "!style-loader!css-loader!sass-loader!../../ui-styles/src/scm.scss"; import React from "react"; import { MemoryRouter } from "react-router-dom"; +import withRedux from "./withRedux"; let i18n = i18next; @@ -70,4 +71,6 @@ addDecorator( }) ); +addDecorator(withRedux); + configure(require.context("../src", true, /\.stories\.tsx?$/), module); diff --git a/scm-ui/ui-components/.storybook/withRedux.js b/scm-ui/ui-components/.storybook/withRedux.js new file mode 100644 index 0000000000..0abc2149ca --- /dev/null +++ b/scm-ui/ui-components/.storybook/withRedux.js @@ -0,0 +1,41 @@ +/* + * 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 {createStore} from "redux"; +import { Provider } from 'react-redux' + +const reducer = (state, action) => { + return state; +}; + +const withRedux = (storyFn) => { + return React.createElement(Provider, { + store: createStore(reducer, {}), + children: storyFn() + }); +} + + +export default withRedux; diff --git a/scm-ui/ui-components/src/ErrorBoundary.tsx b/scm-ui/ui-components/src/ErrorBoundary.tsx index e8eda0d366..33c8a48494 100644 --- a/scm-ui/ui-components/src/ErrorBoundary.tsx +++ b/scm-ui/ui-components/src/ErrorBoundary.tsx @@ -21,14 +21,24 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ -import React, { ReactNode } from "react"; +import React, { ComponentType, ReactNode } from "react"; import ErrorNotification from "./ErrorNotification"; +import { MissingLinkError } from "./errors"; +import { withContextPath } from "./urls"; +import { withRouter, RouteComponentProps } from "react-router-dom"; +import ErrorPage from "./ErrorPage"; +import { WithTranslation, withTranslation } from "react-i18next"; +import { compose } from "redux"; +import { connect } from "react-redux"; -type Props = { +type ExportedProps = { fallback?: React.ComponentType; children: ReactNode; + loginLink?: string; }; +type Props = WithTranslation & RouteComponentProps & ExportedProps; + type ErrorInfo = { componentStack: string; }; @@ -44,16 +54,44 @@ class ErrorBoundary extends React.Component { this.state = {}; } - componentDidCatch(error: Error, errorInfo: ErrorInfo) { - // Catch errors in any components below and re-render with error message - this.setState({ - error, - errorInfo - }); + componentDidUpdate(prevProps: Readonly) { + // we must reset the error if the url has changed + if (this.state.error && prevProps.location !== this.props.location) { + this.setState({ error: undefined, errorInfo: undefined }); + } } + componentDidCatch(error: Error, errorInfo: ErrorInfo) { + this.setState( + { + error, + errorInfo + }, + () => this.redirectToLogin(error) + ); + } + + redirectToLogin = (error: Error) => { + const { loginLink } = this.props; + if (error instanceof MissingLinkError) { + if (loginLink) { + window.location.assign(withContextPath("/login")); + } + } + }; + renderError = () => { + const { t } = this.props; + const { error } = this.state; + let FallbackComponent = this.props.fallback; + + if (error instanceof MissingLinkError) { + return ( + + ); + } + if (!FallbackComponent) { FallbackComponent = ErrorNotification; } @@ -69,4 +107,17 @@ class ErrorBoundary extends React.Component { return this.props.children; } } -export default ErrorBoundary; + +const mapStateToProps = (state: any) => { + const loginLink = state.indexResources?.links?.login?.href; + + return { + loginLink + }; +}; + +export default compose>( + withRouter, + withTranslation("commons"), + connect(mapStateToProps) +)(ErrorBoundary); diff --git a/scm-ui/ui-components/src/ProtectedRoute.tsx b/scm-ui/ui-components/src/ProtectedRoute.tsx index b83271fdb8..c371c49892 100644 --- a/scm-ui/ui-components/src/ProtectedRoute.tsx +++ b/scm-ui/ui-components/src/ProtectedRoute.tsx @@ -22,7 +22,7 @@ * SOFTWARE. */ import React, { Component } from "react"; -import { Route, Redirect, withRouter, RouteComponentProps, RouteProps } from "react-router-dom"; +import { Redirect, Route, RouteComponentProps, RouteProps, withRouter } from "react-router-dom"; type Props = RouteComponentProps & RouteProps & { @@ -30,7 +30,16 @@ type Props = RouteComponentProps & }; class ProtectedRoute extends Component { - renderRoute = (Component: any, authenticated?: boolean) => { + constructor(props: Props) { + super(props); + this.state = { + error: undefined + }; + } + + renderRoute = (Component: any) => { + const { authenticated } = this.props; + return (routeProps: any) => { if (authenticated) { return ; @@ -50,8 +59,8 @@ class ProtectedRoute extends Component { }; render() { - const { component, authenticated, ...routeProps } = this.props; - return ; + const { component, ...routeProps } = this.props; + return ; } } diff --git a/scm-ui/ui-components/src/errors.ts b/scm-ui/ui-components/src/errors.ts index bf1f364e67..d1f7aef226 100644 --- a/scm-ui/ui-components/src/errors.ts +++ b/scm-ui/ui-components/src/errors.ts @@ -89,6 +89,10 @@ export class ConflictError extends BackendError { } } +export class MissingLinkError extends Error { + name = "MissingLinkError"; +} + export function createBackendError(content: BackendErrorContent, statusCode: number) { switch (statusCode) { case 404: diff --git a/scm-ui/ui-webapp/src/admin/containers/Admin.tsx b/scm-ui/ui-webapp/src/admin/containers/Admin.tsx index 3d40359949..505a2760a1 100644 --- a/scm-ui/ui-webapp/src/admin/containers/Admin.tsx +++ b/scm-ui/ui-webapp/src/admin/containers/Admin.tsx @@ -29,12 +29,13 @@ import { Redirect, Route, RouteComponentProps, Switch } from "react-router-dom"; import { ExtensionPoint } from "@scm-manager/ui-extensions"; import { Links } from "@scm-manager/ui-types"; import { + CustomQueryFlexWrappedColumns, NavLink, Page, - CustomQueryFlexWrappedColumns, PrimaryContentColumn, - SecondaryNavigationColumn, SecondaryNavigation, + SecondaryNavigationColumn, + StateMenuContextProvider, SubNavigation } from "@scm-manager/ui-components"; import { getAvailablePluginsLink, getInstalledPluginsLink, getLinks } from "../../modules/indexResource"; @@ -44,7 +45,6 @@ import GlobalConfig from "./GlobalConfig"; import RepositoryRoles from "../roles/containers/RepositoryRoles"; import SingleRepositoryRole from "../roles/containers/SingleRepositoryRole"; import CreateRepositoryRole from "../roles/containers/CreateRepositoryRole"; -import { StateMenuContextProvider } from "@scm-manager/ui-components"; type Props = RouteComponentProps & WithTranslation & { diff --git a/scm-ui/ui-webapp/src/admin/containers/GlobalConfig.tsx b/scm-ui/ui-webapp/src/admin/containers/GlobalConfig.tsx index bf82635458..73ebba7c6f 100644 --- a/scm-ui/ui-webapp/src/admin/containers/GlobalConfig.tsx +++ b/scm-ui/ui-webapp/src/admin/containers/GlobalConfig.tsx @@ -26,7 +26,7 @@ import { WithTranslation, withTranslation } from "react-i18next"; import { connect } from "react-redux"; import { Config, NamespaceStrategies } from "@scm-manager/ui-types"; import { ErrorNotification, Loading, Title } from "@scm-manager/ui-components"; -import { getConfigLink } from "../../modules/indexResource"; +import { mustGetConfigLink } from "../../modules/indexResource"; import { fetchConfig, getConfig, @@ -186,7 +186,7 @@ const mapStateToProps = (state: any) => { const config = getConfig(state); const configUpdatePermission = getConfigUpdatePermission(state); - const configLink = getConfigLink(state); + const configLink = mustGetConfigLink(state); const namespaceStrategies = getNamespaceStrategies(state); return { diff --git a/scm-ui/ui-webapp/src/admin/plugins/containers/PluginsOverview.tsx b/scm-ui/ui-webapp/src/admin/plugins/containers/PluginsOverview.tsx index 7605495810..510ffcbb3d 100644 --- a/scm-ui/ui-webapp/src/admin/plugins/containers/PluginsOverview.tsx +++ b/scm-ui/ui-webapp/src/admin/plugins/containers/PluginsOverview.tsx @@ -25,7 +25,7 @@ import * as React from "react"; import { connect } from "react-redux"; import { WithTranslation, withTranslation } from "react-i18next"; import { compose } from "redux"; -import { PendingPlugins, PluginCollection } from "@scm-manager/ui-types"; +import { PendingPlugins, Plugin, PluginCollection } from "@scm-manager/ui-types"; import { Button, ButtonGroup, @@ -45,16 +45,15 @@ import { } from "../modules/plugins"; import PluginsList from "../components/PluginList"; import { - getAvailablePluginsLink, - getInstalledPluginsLink, - getPendingPluginsLink + getPendingPluginsLink, + mustGetAvailablePluginsLink, + mustGetInstalledPluginsLink } from "../../../modules/indexResource"; import PluginTopActions from "../components/PluginTopActions"; import PluginBottomActions from "../components/PluginBottomActions"; import ExecutePendingActionModal from "../components/ExecutePendingActionModal"; import CancelPendingActionModal from "../components/CancelPendingActionModal"; import UpdateAllActionModal from "../components/UpdateAllActionModal"; -import { Plugin } from "@scm-manager/ui-types"; import ShowPendingModal from "../components/ShowPendingModal"; type Props = WithTranslation & { @@ -319,8 +318,8 @@ const mapStateToProps = (state: any) => { const collection = getPluginCollection(state); const loading = isFetchPluginsPending(state); const error = getFetchPluginsFailure(state); - const availablePluginsLink = getAvailablePluginsLink(state); - const installedPluginsLink = getInstalledPluginsLink(state); + const availablePluginsLink = mustGetAvailablePluginsLink(state); + const installedPluginsLink = mustGetInstalledPluginsLink(state); const pendingPluginsLink = getPendingPluginsLink(state); const pendingPlugins = getPendingPlugins(state); diff --git a/scm-ui/ui-webapp/src/admin/roles/containers/SingleRepositoryRole.tsx b/scm-ui/ui-webapp/src/admin/roles/containers/SingleRepositoryRole.tsx index 4912dc5236..9829e91157 100644 --- a/scm-ui/ui-webapp/src/admin/roles/containers/SingleRepositoryRole.tsx +++ b/scm-ui/ui-webapp/src/admin/roles/containers/SingleRepositoryRole.tsx @@ -29,7 +29,7 @@ import { History } from "history"; import { ExtensionPoint } from "@scm-manager/ui-extensions"; import { RepositoryRole } from "@scm-manager/ui-types"; import { ErrorPage, Loading, Title } from "@scm-manager/ui-components"; -import { getRepositoryRolesLink } from "../../../modules/indexResource"; +import { mustGetRepositoryRolesLink } from "../../../modules/indexResource"; import { fetchRoleByName, getFetchRoleFailure, getRoleByName, isFetchRolePending } from "../modules/roles"; import PermissionRoleDetail from "../components/PermissionRoleDetails"; import EditRepositoryRole from "./EditRepositoryRole"; @@ -107,7 +107,7 @@ const mapStateToProps = (state: any, ownProps: Props) => { const role = getRoleByName(state, roleName); const loading = isFetchRolePending(state, roleName); const error = getFetchRoleFailure(state, roleName); - const repositoryRolesLink = getRepositoryRolesLink(state); + const repositoryRolesLink = mustGetRepositoryRolesLink(state); return { repositoryRolesLink, roleName, diff --git a/scm-ui/ui-webapp/src/containers/Main.tsx b/scm-ui/ui-webapp/src/containers/Main.tsx index 0f1a3966d5..ec72e372eb 100644 --- a/scm-ui/ui-webapp/src/containers/Main.tsx +++ b/scm-ui/ui-webapp/src/containers/Main.tsx @@ -31,7 +31,7 @@ import Users from "../users/containers/Users"; import Login from "../containers/Login"; import Logout from "../containers/Logout"; -import { ProtectedRoute } from "@scm-manager/ui-components"; +import { ProtectedRoute, ErrorBoundary } from "@scm-manager/ui-components"; import { binder, ExtensionPoint } from "@scm-manager/ui-extensions"; import CreateUser from "../users/containers/CreateUser"; @@ -68,39 +68,41 @@ class Main extends React.Component { url = "/login"; } return ( -
- - - - - - - - - - - - - - - - - - - - - - - -
+ +
+ + + + + + + + + + + + + + + + + + + + + + + +
+ ); } } diff --git a/scm-ui/ui-webapp/src/groups/containers/CreateGroup.tsx b/scm-ui/ui-webapp/src/groups/containers/CreateGroup.tsx index ceeeae20f8..2ac2de7a3f 100644 --- a/scm-ui/ui-webapp/src/groups/containers/CreateGroup.tsx +++ b/scm-ui/ui-webapp/src/groups/containers/CreateGroup.tsx @@ -27,11 +27,10 @@ import { compose } from "redux"; import { WithTranslation, withTranslation } from "react-i18next"; import { History } from "history"; import { DisplayedUser, Group } from "@scm-manager/ui-types"; -import { Page } from "@scm-manager/ui-components"; -import { getGroupsLink, getUserAutoCompleteLink } from "../../modules/indexResource"; +import { apiClient, Page } from "@scm-manager/ui-components"; +import { getUserAutoCompleteLink, mustGetGroupsLink } from "../../modules/indexResource"; import { createGroup, createGroupReset, getCreateGroupFailure, isCreateGroupPending } from "../modules/groups"; import GroupForm from "../components/GroupForm"; -import { apiClient } from "@scm-manager/ui-components"; type Props = WithTranslation & { createGroup: (link: string, group: Group, callback?: () => void) => void; @@ -97,7 +96,7 @@ const mapDispatchToProps = (dispatch: any) => { const mapStateToProps = (state: any) => { const loading = isCreateGroupPending(state); const error = getCreateGroupFailure(state); - const createLink = getGroupsLink(state); + const createLink = mustGetGroupsLink(state); const autocompleteLink = getUserAutoCompleteLink(state); return { createLink, diff --git a/scm-ui/ui-webapp/src/groups/containers/Groups.tsx b/scm-ui/ui-webapp/src/groups/containers/Groups.tsx index c8a8a66233..7fe45d0163 100644 --- a/scm-ui/ui-webapp/src/groups/containers/Groups.tsx +++ b/scm-ui/ui-webapp/src/groups/containers/Groups.tsx @@ -35,7 +35,7 @@ import { PageActions, urls } from "@scm-manager/ui-components"; -import { getGroupsLink } from "../../modules/indexResource"; +import { mustGetGroupsLink } from "../../modules/indexResource"; import { fetchGroupsByPage, getFetchGroupsFailure, @@ -128,7 +128,7 @@ const mapStateToProps = (state: any, ownProps: Props) => { const page = urls.getPageFromMatch(match); const canAddGroups = isPermittedToCreateGroups(state); const list = selectListAsCollection(state); - const groupLink = getGroupsLink(state); + const groupLink = mustGetGroupsLink(state); return { groups, diff --git a/scm-ui/ui-webapp/src/groups/containers/SingleGroup.tsx b/scm-ui/ui-webapp/src/groups/containers/SingleGroup.tsx index 5b5555b80c..b38974cc0e 100644 --- a/scm-ui/ui-webapp/src/groups/containers/SingleGroup.tsx +++ b/scm-ui/ui-webapp/src/groups/containers/SingleGroup.tsx @@ -39,7 +39,7 @@ import { SubNavigation, StateMenuContextProvider } from "@scm-manager/ui-components"; -import { getGroupsLink } from "../../modules/indexResource"; +import { getGroupsLink, mustGetGroupsLink } from "../../modules/indexResource"; import { fetchGroupByName, getFetchGroupFailure, getGroupByName, isFetchGroupPending } from "../modules/groups"; import { Details } from "./../components/table"; import { EditGroupNavLink, SetPermissionsNavLink } from "./../components/navLinks"; @@ -138,7 +138,7 @@ const mapStateToProps = (state: any, ownProps: Props) => { const group = getGroupByName(state, name); const loading = isFetchGroupPending(state, name); const error = getFetchGroupFailure(state, name); - const groupLink = getGroupsLink(state); + const groupLink = mustGetGroupsLink(state); return { name, diff --git a/scm-ui/ui-webapp/src/modules/indexResource.ts b/scm-ui/ui-webapp/src/modules/indexResource.ts index d6e9a1e0e0..4aa12f8f7e 100644 --- a/scm-ui/ui-webapp/src/modules/indexResource.ts +++ b/scm-ui/ui-webapp/src/modules/indexResource.ts @@ -24,7 +24,7 @@ import * as types from "./types"; -import { apiClient } from "@scm-manager/ui-components"; +import { apiClient, MissingLinkError } from "@scm-manager/ui-components"; import { Action, IndexResources, Link } from "@scm-manager/ui-types"; import { isPending } from "./pending"; import { getFailure } from "./failure"; @@ -123,6 +123,14 @@ export function getLink(state: object, name: string) { } } +export function mustGetLink(state: object, name: string) { + const link = getLink(state, name); + if (link) { + return link; + } + throw new MissingLinkError(`No link in state for link name: '${name}'`); +} + export function getLinkCollection(state: object, name: string): Link[] { // @ts-ignore Right types not available if (state.indexResources.links && state.indexResources.links[name]) { @@ -145,10 +153,18 @@ export function getAvailablePluginsLink(state: object) { return getLink(state, "availablePlugins"); } +export function mustGetAvailablePluginsLink(state: object) { + return mustGetLink(state, "availablePlugins"); +} + export function getInstalledPluginsLink(state: object) { return getLink(state, "installedPlugins"); } +export function mustGetInstalledPluginsLink(state: object) { + return mustGetLink(state, "installedPlugins"); +} + export function getPendingPluginsLink(state: object) { return getLink(state, "pendingPlugins"); } @@ -169,10 +185,18 @@ export function getUsersLink(state: object) { return getLink(state, "users"); } +export function mustGetUsersLink(state: object) { + return mustGetLink(state, "users"); +} + export function getRepositoryRolesLink(state: object) { return getLink(state, "repositoryRoles"); } +export function mustGetRepositoryRolesLink(state: object) { + return mustGetLink(state, "repositoryRoles"); +} + export function getRepositoryVerbsLink(state: object) { return getLink(state, "repositoryVerbs"); } @@ -181,10 +205,18 @@ export function getGroupsLink(state: object) { return getLink(state, "groups"); } +export function mustGetGroupsLink(state: object) { + return mustGetLink(state, "groups"); +} + export function getConfigLink(state: object) { return getLink(state, "config"); } +export function mustGetConfigLink(state: object) { + return mustGetLink(state, "config"); +} + export function getRepositoriesLink(state: object) { return getLink(state, "repositories"); } diff --git a/scm-ui/ui-webapp/src/users/containers/CreateUser.tsx b/scm-ui/ui-webapp/src/users/containers/CreateUser.tsx index 455a2081df..6f86f3539b 100644 --- a/scm-ui/ui-webapp/src/users/containers/CreateUser.tsx +++ b/scm-ui/ui-webapp/src/users/containers/CreateUser.tsx @@ -28,7 +28,7 @@ import { WithTranslation, withTranslation } from "react-i18next"; import { History } from "history"; import { User } from "@scm-manager/ui-types"; import { Page } from "@scm-manager/ui-components"; -import { getUsersLink } from "../../modules/indexResource"; +import { mustGetUsersLink } from "../../modules/indexResource"; import { createUser, createUserReset, getCreateUserFailure, isCreateUserPending } from "../modules/users"; import UserForm from "../components/UserForm"; @@ -84,7 +84,7 @@ const mapDispatchToProps = (dispatch: any) => { const mapStateToProps = (state: any) => { const loading = isCreateUserPending(state); const error = getCreateUserFailure(state); - const usersLink = getUsersLink(state); + const usersLink = mustGetUsersLink(state); return { usersLink, loading, diff --git a/scm-ui/ui-webapp/src/users/containers/SingleUser.tsx b/scm-ui/ui-webapp/src/users/containers/SingleUser.tsx index ceea726797..e7d0856545 100644 --- a/scm-ui/ui-webapp/src/users/containers/SingleUser.tsx +++ b/scm-ui/ui-webapp/src/users/containers/SingleUser.tsx @@ -43,10 +43,9 @@ import EditUser from "./EditUser"; import { fetchUserByName, getFetchUserFailure, getUserByName, isFetchUserPending } from "../modules/users"; import { EditUserNavLink, SetPasswordNavLink, SetPermissionsNavLink, SetPublicKeysNavLink } from "./../components/navLinks"; import { WithTranslation, withTranslation } from "react-i18next"; -import { getUsersLink } from "../../modules/indexResource"; +import { mustGetUsersLink } from "../../modules/indexResource"; import SetUserPassword from "../components/SetUserPassword"; import SetPermissions from "../../permissions/components/SetPermissions"; -import AddPublicKey from "../components/publicKeys/AddPublicKey"; import SetPublicKeys from "../components/publicKeys/SetPublicKeys"; type Props = RouteComponentProps & @@ -148,7 +147,7 @@ const mapStateToProps = (state: any, ownProps: Props) => { const user = getUserByName(state, name); const loading = isFetchUserPending(state, name); const error = getFetchUserFailure(state, name); - const usersLink = getUsersLink(state); + const usersLink = mustGetUsersLink(state); return { usersLink, name, diff --git a/scm-ui/ui-webapp/src/users/containers/Users.tsx b/scm-ui/ui-webapp/src/users/containers/Users.tsx index fe6a372c2b..d0d4adc9dc 100644 --- a/scm-ui/ui-webapp/src/users/containers/Users.tsx +++ b/scm-ui/ui-webapp/src/users/containers/Users.tsx @@ -35,7 +35,7 @@ import { PageActions, urls } from "@scm-manager/ui-components"; -import { getUsersLink } from "../../modules/indexResource"; +import { mustGetUsersLink } from "../../modules/indexResource"; import { fetchUsersByPage, getFetchUsersFailure, @@ -129,7 +129,7 @@ const mapStateToProps = (state: any, ownProps: Props) => { const page = urls.getPageFromMatch(match); const canAddUsers = isPermittedToCreateUsers(state); const list = selectListAsCollection(state); - const usersLink = getUsersLink(state); + const usersLink = mustGetUsersLink(state); return { users, From c0532282210a52fa2c973770f58c481ae81b550b Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Fri, 28 Aug 2020 13:26:19 +0200 Subject: [PATCH 04/30] Fix token expired exception being displayed in browser --- .../scm/web/filter/AuthenticationFilter.java | 7 +++++- ...otocolServletAuthenticationFilterBase.java | 12 ++++++++++ ...olServletAuthenticationFilterBaseTest.java | 23 ++++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/web/filter/AuthenticationFilter.java b/scm-core/src/main/java/sonia/scm/web/filter/AuthenticationFilter.java index a6514465fd..30e8df9fe7 100644 --- a/scm-core/src/main/java/sonia/scm/web/filter/AuthenticationFilter.java +++ b/scm-core/src/main/java/sonia/scm/web/filter/AuthenticationFilter.java @@ -166,6 +166,11 @@ public class AuthenticationFilter extends HttpFilter { HttpUtil.sendUnauthorized(request, response, configuration.getRealmDescription()); } + protected void handleTokenExpiredException(HttpServletRequest request, HttpServletResponse response, + FilterChain chain, TokenExpiredException tokenExpiredException) throws IOException, ServletException { + throw tokenExpiredException; + } + /** * Iterates all {@link WebTokenGenerator} and creates an * {@link AuthenticationToken} from the given request. @@ -211,7 +216,7 @@ public class AuthenticationFilter extends HttpFilter { processChain(request, response, chain, subject); } catch (TokenExpiredException ex) { // Rethrow to be caught by TokenExpiredFilter - throw ex; + handleTokenExpiredException(request, response, chain, ex); } catch (AuthenticationException ex) { logger.warn("authentication failed", ex); handleUnauthorized(request, response, chain); diff --git a/scm-core/src/main/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBase.java b/scm-core/src/main/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBase.java index 08f845740c..e6716f0013 100644 --- a/scm-core/src/main/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBase.java +++ b/scm-core/src/main/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBase.java @@ -25,6 +25,7 @@ package sonia.scm.web.filter; import sonia.scm.config.ScmConfiguration; +import sonia.scm.security.TokenExpiredException; import sonia.scm.util.HttpUtil; import sonia.scm.web.UserAgent; import sonia.scm.web.UserAgentParser; @@ -59,4 +60,15 @@ public class HttpProtocolServletAuthenticationFilterBase extends AuthenticationF HttpUtil.sendUnauthorized(request, response); } } + + @Override + protected void handleTokenExpiredException(HttpServletRequest request, HttpServletResponse response, FilterChain chain, TokenExpiredException tokenExpiredException) throws IOException, ServletException { + UserAgent userAgent = userAgentParser.parse(request); + if (userAgent.isBrowser()) { + // we can proceed the filter chain because the HttpProtocolServlet will render the ui if the client is a browser + chain.doFilter(request, response); + } else { + super.handleTokenExpiredException(request, response, chain, tokenExpiredException); + } + } } diff --git a/scm-core/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBaseTest.java b/scm-core/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBaseTest.java index daa2ad448b..2c49458547 100644 --- a/scm-core/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBaseTest.java +++ b/scm-core/src/test/java/sonia/scm/web/filter/HttpProtocolServletAuthenticationFilterBaseTest.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.web.filter; import org.junit.jupiter.api.BeforeEach; @@ -30,6 +30,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.config.ScmConfiguration; +import sonia.scm.security.TokenExpiredException; import sonia.scm.util.HttpUtil; import sonia.scm.web.UserAgent; import sonia.scm.web.UserAgentParser; @@ -43,6 +44,7 @@ import java.io.IOException; import java.util.Collections; import java.util.Set; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -93,4 +95,23 @@ class HttpProtocolServletAuthenticationFilterBaseTest { verify(filterChain).doFilter(request, response); } + @Test + void shouldIgnoreTokenExpiredExceptionForBrowserCall() throws IOException, ServletException { + when(userAgentParser.parse(request)).thenReturn(browser); + + authenticationFilter.handleTokenExpiredException(request, response, filterChain, new TokenExpiredException("Nothing ever expired so much")); + + verify(filterChain).doFilter(request, response); + } + + @Test + void shouldRethrowTokenExpiredExceptionForApiCall() { + when(userAgentParser.parse(request)).thenReturn(nonBrowser); + + final TokenExpiredException tokenExpiredException = new TokenExpiredException("Nothing ever expired so much"); + + assertThrows(TokenExpiredException.class, + () -> authenticationFilter.handleTokenExpiredException(request, response, filterChain, tokenExpiredException)); + } + } From daa5d459cff13e7ba116b9337166907a4935e6e4 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Fri, 28 Aug 2020 14:04:56 +0200 Subject: [PATCH 05/30] Fix token expired exception being displayed instead of login page on revisit --- .../src/modules/indexResource.test.ts | 52 +++++++++++++++++-- scm-ui/ui-webapp/src/modules/indexResource.ts | 29 ++++++++--- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/scm-ui/ui-webapp/src/modules/indexResource.test.ts b/scm-ui/ui-webapp/src/modules/indexResource.test.ts index 9f6674b509..c1ffaf77ce 100644 --- a/scm-ui/ui-webapp/src/modules/indexResource.test.ts +++ b/scm-ui/ui-webapp/src/modules/indexResource.test.ts @@ -117,7 +117,7 @@ const indexResourcesAuthenticated = { describe("index resource", () => { describe("fetch index resource", () => { - const index_url = "/api/v2/"; + const indexUrl = "/api/v2/"; const mockStore = configureMockStore([thunk]); afterEach(() => { @@ -126,7 +126,7 @@ describe("index resource", () => { }); it("should successfully fetch index resources when unauthenticated", () => { - fetchMock.getOnce(index_url, indexResourcesUnauthenticated); + fetchMock.getOnce(indexUrl, indexResourcesUnauthenticated); const expectedActions = [ { @@ -145,7 +145,7 @@ describe("index resource", () => { }); it("should successfully fetch index resources when authenticated", () => { - fetchMock.getOnce(index_url, indexResourcesAuthenticated); + fetchMock.getOnce(indexUrl, indexResourcesAuthenticated); const expectedActions = [ { @@ -164,7 +164,7 @@ describe("index resource", () => { }); it("should dispatch FETCH_INDEX_RESOURCES_FAILURE if request fails", () => { - fetchMock.getOnce(index_url, { + fetchMock.getOnce(indexUrl, { status: 500 }); @@ -176,6 +176,50 @@ describe("index resource", () => { expect(actions[1].payload).toBeDefined(); }); }); + + it("should retry to fetch index resource on unauthorized error", () => { + fetchMock.getOnce(indexUrl, { status: 401 }).getOnce(indexUrl, indexResourcesAuthenticated, { + overwriteRoutes: false + }); + + const expectedActions = [ + { + type: FETCH_INDEXRESOURCES_PENDING + }, + { + type: FETCH_INDEXRESOURCES_SUCCESS, + payload: indexResourcesAuthenticated + } + ]; + + const store = mockStore({}); + return store.dispatch(fetchIndexResources()).then(() => { + expect(store.getActions()).toEqual(expectedActions); + }); + }); + + it("should only retry to fetch index resource on unauthorized error once", () => { + fetchMock + .getOnce(indexUrl, { status: 401 }) + .getOnce( + indexUrl, + { status: 401 }, + { + overwriteRoutes: false + } + ) + .getOnce(indexUrl, indexResourcesAuthenticated, { + overwriteRoutes: false + }); + + const store = mockStore({}); + return store.dispatch(fetchIndexResources()).then(() => { + const actions = store.getActions(); + expect(actions[0].type).toEqual(FETCH_INDEXRESOURCES_PENDING); + expect(actions[1].type).toEqual(FETCH_INDEXRESOURCES_FAILURE); + expect(actions[1].payload).toBeDefined(); + }); + }); }); describe("index resources reducer", () => { diff --git a/scm-ui/ui-webapp/src/modules/indexResource.ts b/scm-ui/ui-webapp/src/modules/indexResource.ts index 4aa12f8f7e..4d642c5fb4 100644 --- a/scm-ui/ui-webapp/src/modules/indexResource.ts +++ b/scm-ui/ui-webapp/src/modules/indexResource.ts @@ -24,7 +24,7 @@ import * as types from "./types"; -import { apiClient, MissingLinkError } from "@scm-manager/ui-components"; +import { apiClient, MissingLinkError, UnauthorizedError } from "@scm-manager/ui-components"; import { Action, IndexResources, Link } from "@scm-manager/ui-types"; import { isPending } from "./pending"; import { getFailure } from "./failure"; @@ -46,14 +46,27 @@ export const callFetchIndexResources = (): Promise => { export function fetchIndexResources() { return function(dispatch: any) { + /* + * The index resource returns a 401 if the access token expired. + * This only happens once because the error response automatically invalidates the cookie. + * In this event, we have to try the request once again. + */ + let shouldRetry = true; dispatch(fetchIndexResourcesPending()); - return callFetchIndexResources() - .then(resources => { - dispatch(fetchIndexResourcesSuccess(resources)); - }) - .catch(err => { - dispatch(fetchIndexResourcesFailure(err)); - }); + const run = (): Promise => + callFetchIndexResources() + .then(resources => { + dispatch(fetchIndexResourcesSuccess(resources)); + }) + .catch(err => { + if (err instanceof UnauthorizedError && shouldRetry) { + shouldRetry = false; + return run(); + } else { + dispatch(fetchIndexResourcesFailure(err)); + } + }); + return run(); }; } From 9ee7c3ff1c863e4888a804bcca07702ac6b64d5f Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Fri, 28 Aug 2020 15:39:58 +0200 Subject: [PATCH 06/30] reset the redux state if the token is expired and the user still uses the web ui --- scm-ui/ui-components/src/apiclient.ts | 33 ++++++++++++++++++++--- scm-ui/ui-webapp/src/containers/Index.tsx | 9 +++++++ scm-ui/ui-webapp/src/createReduxStore.ts | 31 ++++++++++++++++++--- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/scm-ui/ui-components/src/apiclient.ts b/scm-ui/ui-components/src/apiclient.ts index 361038208e..2a8f53ae0a 100644 --- a/scm-ui/ui-components/src/apiclient.ts +++ b/scm-ui/ui-components/src/apiclient.ts @@ -155,9 +155,19 @@ export function createUrlWithIdentifiers(url: string): string { return createUrl(url) + "?X-SCM-Client=WUI&X-SCM-Session-ID=" + sessionId; } +type ErrorListener = (error: Error) => void; + class ApiClient { + constructor() { + this.notifyAndRethrow = this.notifyAndRethrow.bind(this); + } + + errorListeners: ErrorListener[] = []; + get(url: string): Promise { - return fetch(createUrl(url), applyFetchOptions({})).then(handleFailure); + return fetch(createUrl(url), applyFetchOptions({})) + .then(handleFailure) + .catch(this.notifyAndRethrow); } post(url: string, payload?: any, contentType = "application/json", additionalHeaders: Record = {}) { @@ -193,7 +203,9 @@ class ApiClient { method: "HEAD" }; options = applyFetchOptions(options); - return fetch(createUrl(url), options).then(handleFailure); + return fetch(createUrl(url), options) + .then(handleFailure) + .catch(this.notifyAndRethrow); } delete(url: string): Promise { @@ -201,7 +213,9 @@ class ApiClient { method: "DELETE" }; options = applyFetchOptions(options); - return fetch(createUrl(url), options).then(handleFailure); + return fetch(createUrl(url), options) + .then(handleFailure) + .catch(this.notifyAndRethrow); } httpRequestWithJSONBody( @@ -245,7 +259,9 @@ class ApiClient { options.headers["Content-Type"] = contentType; } - return fetch(createUrl(url), options).then(handleFailure); + return fetch(createUrl(url), options) + .then(handleFailure) + .catch(this.notifyAndRethrow); } subscribe(url: string, argument: SubscriptionArgument): Cancel { @@ -276,6 +292,15 @@ class ApiClient { return () => es.close(); } + + onError(errorListener: ErrorListener) { + this.errorListeners.push(errorListener); + } + + private notifyAndRethrow(error: Error): never { + this.errorListeners.forEach(errorListener => errorListener(error)); + throw error; + } } export const apiClient = new ApiClient(); diff --git a/scm-ui/ui-webapp/src/containers/Index.tsx b/scm-ui/ui-webapp/src/containers/Index.tsx index 0a3d80aa5d..a92d56d901 100644 --- a/scm-ui/ui-webapp/src/containers/Index.tsx +++ b/scm-ui/ui-webapp/src/containers/Index.tsx @@ -64,6 +64,15 @@ class Index extends Component { this.props.fetchIndexResources(); } + componentDidUpdate() { + const { indexResources, loading, error } = this.props; + const { pluginsLoaded } = this.state; + if (!indexResources && !loading && !error && pluginsLoaded) { + this.props.fetchIndexResources(); + this.setState({ pluginsLoaded: false }); + } + } + pluginLoaderCallback = () => { this.setState({ pluginsLoaded: true diff --git a/scm-ui/ui-webapp/src/createReduxStore.ts b/scm-ui/ui-webapp/src/createReduxStore.ts index 1a391d17ae..ffdbb7a859 100644 --- a/scm-ui/ui-webapp/src/createReduxStore.ts +++ b/scm-ui/ui-webapp/src/createReduxStore.ts @@ -24,14 +24,14 @@ import thunk from "redux-thunk"; import logger from "redux-logger"; -import { applyMiddleware, combineReducers, compose, createStore } from "redux"; +import { AnyAction, applyMiddleware, combineReducers, compose, createStore } from "redux"; import users from "./users/modules/users"; import repos from "./repos/modules/repos"; import repositoryTypes from "./repos/modules/repositoryTypes"; import changesets from "./repos/modules/changesets"; import sources from "./repos/sources/modules/sources"; import groups from "./groups/modules/groups"; -import auth from "./modules/auth"; +import auth, { isAuthenticated } from "./modules/auth"; import pending from "./modules/pending"; import failure from "./modules/failure"; import permissions from "./repos/permissions/modules/permissions"; @@ -40,13 +40,15 @@ import roles from "./admin/roles/modules/roles"; import namespaceStrategies from "./admin/modules/namespaceStrategies"; import indexResources from "./modules/indexResource"; import plugins from "./admin/plugins/modules/plugins"; +import { apiClient } from "@scm-manager/ui-components"; import branches from "./repos/branches/modules/branches"; +import { UnauthorizedError } from "@scm-manager/ui-components/src"; function createReduxStore() { const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose; - const reducer = combineReducers({ + const appReducer = combineReducers({ pending, failure, indexResources, @@ -65,7 +67,28 @@ function createReduxStore() { plugins }); - return createStore(reducer, composeEnhancers(applyMiddleware(thunk, logger))); + const reducer = (state: any, action: AnyAction) => { + console.log(action.type, state?.tokenExpired); + if (state?.tokenExpired && action.type.indexOf("FAILURE") === -1) { + console.log("reset state"); + return appReducer({}, action); + } + + if (action.type === "API_CLIENT_UNAUTHORIZED" && isAuthenticated(state)) { + return { ...state, tokenExpired: true }; + } + + return { ...appReducer(state, action), tokenExpired: state?.tokenExpired }; + }; + + const store = createStore(reducer, composeEnhancers(applyMiddleware(thunk, logger))); + apiClient.onError(error => { + if (error instanceof UnauthorizedError) { + store.dispatch({ type: "API_CLIENT_UNAUTHORIZED", error }); + } + }); + + return store; } export default createReduxStore; From 5a010baf2801ad592920d8a84dcd46d8819c035c Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 31 Aug 2020 15:19:40 +0200 Subject: [PATCH 07/30] Add small fixes and code comments to reset state logic --- scm-ui/ui-webapp/package.json | 1 + scm-ui/ui-webapp/src/createReduxStore.ts | 32 ++++++++++++++++-------- yarn.lock | 7 ++++++ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/scm-ui/ui-webapp/package.json b/scm-ui/ui-webapp/package.json index fb9879c015..10f35dcaa3 100644 --- a/scm-ui/ui-webapp/package.json +++ b/scm-ui/ui-webapp/package.json @@ -39,6 +39,7 @@ "@types/react-dom": "^16.9.2", "@types/react-redux": "5.0.7", "@types/react-router-dom": "^5.1.0", + "@types/redux-logger": "^3.0.8", "@types/styled-components": "^5.1.0", "@types/systemjs": "^0.20.6", "fetch-mock": "^7.5.1", diff --git a/scm-ui/ui-webapp/src/createReduxStore.ts b/scm-ui/ui-webapp/src/createReduxStore.ts index ffdbb7a859..216a720920 100644 --- a/scm-ui/ui-webapp/src/createReduxStore.ts +++ b/scm-ui/ui-webapp/src/createReduxStore.ts @@ -40,12 +40,13 @@ import roles from "./admin/roles/modules/roles"; import namespaceStrategies from "./admin/modules/namespaceStrategies"; import indexResources from "./modules/indexResource"; import plugins from "./admin/plugins/modules/plugins"; -import { apiClient } from "@scm-manager/ui-components"; - +import { apiClient, UnauthorizedError } from "@scm-manager/ui-components"; import branches from "./repos/branches/modules/branches"; -import { UnauthorizedError } from "@scm-manager/ui-components/src"; + +const EMPTY_STATE = {} as any; function createReduxStore() { + // @ts-ignore __REDUX_DEVTOOLS_EXTENSION_COMPOSE__ is defined by react dev tools const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose; const appReducer = combineReducers({ @@ -67,27 +68,38 @@ function createReduxStore() { plugins }); + // We assume that an UnauthorizedError means that the access token is expired. + // If the token is expired we want to show an error with the login link. + // This error should be displayed with the state (e.g. navigation) of the previous logged in user. + // But if the user navigates away, we want to reset the state to an anonymous one. + const reducer = (state: any, action: AnyAction) => { - console.log(action.type, state?.tokenExpired); - if (state?.tokenExpired && action.type.indexOf("FAILURE") === -1) { - console.log("reset state"); - return appReducer({}, action); + // Reset the state if the token is expired and a new action is dispatched (e.g. navigation). + // We exclude failures, because the fetch which had triggered the unauthorized error + // will likely end with an failure action. + if (state.tokenExpired && !action.type.includes("FAILURE")) { + // reset state by passing an empty state down to the app reducer + // we do not use the captured action, because the data is derived from the old state + return appReducer(EMPTY_STATE, { type: "_" }); } + // If the user is authenticated and response is an unauthorized error, + // we assume that the token is expired. if (action.type === "API_CLIENT_UNAUTHORIZED" && isAuthenticated(state)) { return { ...state, tokenExpired: true }; } - return { ...appReducer(state, action), tokenExpired: state?.tokenExpired }; + // Keep the tokenExpired after calling appReducer, + // this is required because the appReducer would remove any unknown property. + return { ...appReducer(state, action), tokenExpired: state.tokenExpired }; }; - const store = createStore(reducer, composeEnhancers(applyMiddleware(thunk, logger))); + const store = createStore(reducer, EMPTY_STATE, composeEnhancers(applyMiddleware(thunk, logger))); apiClient.onError(error => { if (error instanceof UnauthorizedError) { store.dispatch({ type: "API_CLIENT_UNAUTHORIZED", error }); } }); - return store; } diff --git a/yarn.lock b/yarn.lock index 46062a1534..9c73e4d6ce 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3397,6 +3397,13 @@ "@types/prop-types" "*" csstype "^2.2.0" +"@types/redux-logger@^3.0.8": + version "3.0.8" + resolved "https://registry.yarnpkg.com/@types/redux-logger/-/redux-logger-3.0.8.tgz#1fb6d26917bb198792bb1cf57feb31cae1532c5d" + integrity sha512-zM+cxiSw6nZtRbxpVp9SE3x/X77Z7e7YAfHD1NkxJyJbAGSXJGF0E9aqajZfPOa/sTYnuwutmlCldveExuCeLw== + dependencies: + redux "^4.0.0" + "@types/sinonjs__fake-timers@^6.0.1": version "6.0.1" resolved "https://registry.yarnpkg.com/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-6.0.1.tgz#681df970358c82836b42f989188d133e218c458e" From 6324ba37c6aef2d13f563bdd9015065330d1c94a Mon Sep 17 00:00:00 2001 From: snyk-bot Date: Tue, 1 Sep 2020 04:41:15 +0000 Subject: [PATCH 08/30] fix: upgrade mini-css-extract-plugin from 0.9.0 to 0.10.0 Snyk has created this PR to upgrade mini-css-extract-plugin from 0.9.0 to 0.10.0. See this package in npm: https://www.npmjs.com/package/mini-css-extract-plugin See this project in Snyk: https://app.snyk.io/org/scm-manager/project/e563ed8d-dedf-4b52-af75-5d89b1e70f44?utm_source=github&utm_medium=upgrade-pr --- scm-ui/ui-scripts/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-ui/ui-scripts/package.json b/scm-ui/ui-scripts/package.json index 2296e46cba..e92a744489 100644 --- a/scm-ui/ui-scripts/package.json +++ b/scm-ui/ui-scripts/package.json @@ -14,7 +14,7 @@ "babel-loader": "^8.0.6", "css-loader": "^3.2.0", "file-loader": "^4.2.0", - "mini-css-extract-plugin": "^0.9.0", + "mini-css-extract-plugin": "^0.10.0", "mustache": "^3.1.0", "optimize-css-assets-webpack-plugin": "^5.0.3", "react-refresh": "^0.8.0", From de0bcaf3e8b2422932e25c6c4d21ab4a8aa3e1b3 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 07:21:06 +0200 Subject: [PATCH 09/30] use static instance of slf4j instead of lombok annotation --- .../java/sonia/scm/web/i18n/I18nServlet.java | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java b/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java index 2748153b7d..903250c68b 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java +++ b/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.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.web.i18n; @@ -31,14 +31,15 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.github.legman.Subscribe; import com.google.common.annotations.VisibleForTesting; import com.google.inject.Singleton; -import lombok.extern.slf4j.Slf4j; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.NotFoundException; import sonia.scm.SCMContext; import sonia.scm.Stage; -import sonia.scm.lifecycle.RestartEvent; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; import sonia.scm.filter.WebElement; +import sonia.scm.lifecycle.RestartEvent; import sonia.scm.plugin.PluginLoader; import javax.inject.Inject; @@ -63,16 +64,17 @@ import static sonia.scm.NotFoundException.notFound; */ @Singleton @WebElement(value = I18nServlet.PATTERN, regex = true) -@Slf4j public class I18nServlet extends HttpServlet { + private static final Logger LOG = LoggerFactory.getLogger(I18nServlet.class); + public static final String PLUGINS_JSON = "plugins.json"; public static final String PATTERN = "/locales/[a-z\\-A-Z]*/" + PLUGINS_JSON; public static final String CACHE_NAME = "sonia.cache.plugins.translations"; private final ClassLoader classLoader; private final Cache cache; - private static ObjectMapper objectMapper = new ObjectMapper(); + private final ObjectMapper objectMapper = new ObjectMapper(); @Inject @@ -83,7 +85,7 @@ public class I18nServlet extends HttpServlet { @Subscribe(async = false) public void handleRestartEvent(RestartEvent event) { - log.debug("Clear cache on restart event with reason {}", event.getReason()); + LOG.debug("Clear cache on restart event with reason {}", event.getReason()); cache.clear(); } @@ -108,21 +110,20 @@ public class I18nServlet extends HttpServlet { try (PrintWriter out = response.getWriter()) { String path = req.getServletPath(); Function> jsonFileProvider = usedPath -> Optional.empty(); - BiConsumer createdJsonFileConsumer = (usedPath, jsonNode) -> log.debug("A json File is created from the path {}", usedPath); + BiConsumer createdJsonFileConsumer = (usedPath, jsonNode) -> LOG.debug("A json File is created from the path {}", usedPath); if (isProductionStage()) { - log.debug("In Production Stage get the plugin translations from the cache"); - jsonFileProvider = usedPath -> Optional.ofNullable( - cache.get(usedPath)); + LOG.debug("In Production Stage get the plugin translations from the cache"); + jsonFileProvider = usedPath -> Optional.ofNullable(cache.get(usedPath)); createdJsonFileConsumer = createdJsonFileConsumer - .andThen((usedPath, jsonNode) -> log.debug("Put the created json File in the cache with the key {}", usedPath)) + .andThen((usedPath, jsonNode) -> LOG.debug("Put the created json File in the cache with the key {}", usedPath)) .andThen(cache::put); } objectMapper.writeValue(out, getCollectedJson(path, jsonFileProvider, createdJsonFileConsumer)); } catch (IOException e) { - log.error("Error on getting the translation of the plugins", e); + LOG.error("Error on getting the translation of the plugins", e); response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } catch (NotFoundException e) { - log.error("Plugin translations are not found", e); + LOG.error("Plugin translations are not found", e); response.setStatus(HttpServletResponse.SC_NOT_FOUND); } } @@ -140,7 +141,7 @@ public class I18nServlet extends HttpServlet { */ @VisibleForTesting protected Optional collectJsonFile(String path) { - log.debug("Collect plugin translations from path {} for every plugin", path); + LOG.debug("Collect plugin translations from path {} for every plugin", path); JsonNode mergedJsonNode = null; try { Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); @@ -154,7 +155,7 @@ public class I18nServlet extends HttpServlet { } } } catch (IOException e) { - log.error("Error on loading sources from {}", path, e); + LOG.error("Error on loading sources from {}", path, e); return Optional.empty(); } return Optional.ofNullable(mergedJsonNode); From 8d65bf75f3fc3ce3ce32317205c8d3f7356280fe Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 07:53:21 +0200 Subject: [PATCH 10/30] refactor I18nServlet to avoid stacktrace logging on unknown language --- .../java/sonia/scm/web/i18n/I18nServlet.java | 108 +++++++++--------- .../sonia/scm/web/i18n/I18nServletTest.java | 26 ++--- 2 files changed, 65 insertions(+), 69 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java b/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java index 903250c68b..4671915a9d 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java +++ b/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java @@ -33,7 +33,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.NotFoundException; import sonia.scm.SCMContext; import sonia.scm.Stage; import sonia.scm.cache.Cache; @@ -52,11 +51,6 @@ import java.net.URL; import java.util.Enumeration; import java.util.Iterator; import java.util.Optional; -import java.util.function.BiConsumer; -import java.util.function.Function; - -import static sonia.scm.ContextEntry.ContextBuilder.entity; -import static sonia.scm.NotFoundException.notFound; /** @@ -89,45 +83,54 @@ public class I18nServlet extends HttpServlet { cache.clear(); } - private JsonNode getCollectedJson(String path, - Function> jsonFileProvider, - BiConsumer createdJsonFileConsumer) { - return Optional.ofNullable(jsonFileProvider.apply(path) - .orElseGet(() -> { - Optional createdFile = collectJsonFile(path); - createdFile.ifPresent(map -> createdJsonFileConsumer.accept(path, map)); - return createdFile.orElse(null); - } - )).orElseThrow(() -> notFound(entity("jsonprovider", path))); - } - @VisibleForTesting @Override - protected void doGet(HttpServletRequest req, HttpServletResponse response) { + protected void doGet(HttpServletRequest request, HttpServletResponse response) { + String path = request.getServletPath(); + try { + Optional json = findJson(path); + if (json.isPresent()) { + write(response, json.get()); + } else { + LOG.debug("could not find translation at {}", path); + response.setStatus(HttpServletResponse.SC_NOT_FOUND); + } + } catch (IOException ex) { + LOG.error("Error on getting the translation of the plugins", ex); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + } + + private void write(HttpServletResponse response, JsonNode jsonNode) throws IOException { response.setCharacterEncoding("UTF-8"); response.setContentType("application/json"); response.setHeader("Cache-Control", "no-cache"); - try (PrintWriter out = response.getWriter()) { - String path = req.getServletPath(); - Function> jsonFileProvider = usedPath -> Optional.empty(); - BiConsumer createdJsonFileConsumer = (usedPath, jsonNode) -> LOG.debug("A json File is created from the path {}", usedPath); - if (isProductionStage()) { - LOG.debug("In Production Stage get the plugin translations from the cache"); - jsonFileProvider = usedPath -> Optional.ofNullable(cache.get(usedPath)); - createdJsonFileConsumer = createdJsonFileConsumer - .andThen((usedPath, jsonNode) -> LOG.debug("Put the created json File in the cache with the key {}", usedPath)) - .andThen(cache::put); - } - objectMapper.writeValue(out, getCollectedJson(path, jsonFileProvider, createdJsonFileConsumer)); - } catch (IOException e) { - LOG.error("Error on getting the translation of the plugins", e); - response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - } catch (NotFoundException e) { - LOG.error("Plugin translations are not found", e); - response.setStatus(HttpServletResponse.SC_NOT_FOUND); + + try (PrintWriter writer = response.getWriter()) { + objectMapper.writeValue(writer, jsonNode); } } + public Optional findJson(String path) throws IOException { + if (isProductionStage()) { + return findJsonCached(path); + } + return collectJsonFile(path); + } + + private Optional findJsonCached(String path) throws IOException { + JsonNode jsonNode = cache.get(path); + if (jsonNode != null) { + LOG.debug("return json node from cache for path {}", path); + return Optional.of(jsonNode); + } + + LOG.debug("collect json for path {}", path); + Optional collected = collectJsonFile(path); + collected.ifPresent(node -> cache.put(path, node)); + return collected; + } + @VisibleForTesting protected boolean isProductionStage() { return SCMContext.getContext().getStage() == Stage.PRODUCTION; @@ -140,46 +143,39 @@ public class I18nServlet extends HttpServlet { * @return a collected Json File as JsonNode from the given path from all plugins in the class path */ @VisibleForTesting - protected Optional collectJsonFile(String path) { + protected Optional collectJsonFile(String path) throws IOException { LOG.debug("Collect plugin translations from path {} for every plugin", path); JsonNode mergedJsonNode = null; - try { - Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); - while (resources.hasMoreElements()) { - URL url = resources.nextElement(); - JsonNode jsonNode = objectMapper.readTree(url); - if (mergedJsonNode != null) { - merge(mergedJsonNode, jsonNode); - } else { - mergedJsonNode = jsonNode; - } + Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); + while (resources.hasMoreElements()) { + URL url = resources.nextElement(); + JsonNode jsonNode = objectMapper.readTree(url); + if (mergedJsonNode != null) { + merge(mergedJsonNode, jsonNode); + } else { + mergedJsonNode = jsonNode; } - } catch (IOException e) { - LOG.error("Error on loading sources from {}", path, e); - return Optional.empty(); } + return Optional.ofNullable(mergedJsonNode); } /** * Merge the updateNode into the mainNode and return it. - * + *

* This is not a deep merge. * - * @param mainNode the main node + * @param mainNode the main node * @param updateNode the update node * @return the merged mainNode */ @VisibleForTesting protected JsonNode merge(JsonNode mainNode, JsonNode updateNode) { Iterator fieldNames = updateNode.fieldNames(); - while (fieldNames.hasNext()) { - String fieldName = fieldNames.next(); JsonNode jsonNode = mainNode.get(fieldName); - if (jsonNode != null) { mergeNode(updateNode, fieldName, jsonNode); } else { diff --git a/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java b/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java index 4770a54d91..32c3fcc772 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java +++ b/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.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.web.i18n; import com.fasterxml.jackson.databind.JsonNode; @@ -40,7 +40,6 @@ import org.mockito.MockSettings; import org.mockito.internal.creation.MockSettingsImpl; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.MockitoJUnitRunner; -import sonia.scm.lifecycle.RestartEvent; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; import sonia.scm.event.ScmEventBus; @@ -87,7 +86,7 @@ public class I18nServletTest { "}", "}" ); - private static String SVN_PLUGIN_JSON = json( + private static final String SVN_PLUGIN_JSON = json( "{", "'scm-svn-plugin': {", "'information': {", @@ -116,11 +115,11 @@ public class I18nServletTest { private I18nServlet servlet; @Mock - private Cache cache; + private Cache cache; + private Enumeration resources; @Before - @SuppressWarnings("unchecked") public void init() throws IOException { resources = Collections.enumeration(Lists.newArrayList( createFileFromString(SVN_PLUGIN_JSON).toURI().toURL(), @@ -128,7 +127,7 @@ public class I18nServletTest { createFileFromString(HG_PLUGIN_JSON).toURI().toURL() )); when(pluginLoader.getUberClassLoader()).thenReturn(classLoader); - when(cacheManager.getCache(I18nServlet.CACHE_NAME)).thenReturn(cache); + when(cacheManager.getCache(I18nServlet.CACHE_NAME)).thenReturn(cache); MockSettings settings = new MockSettingsImpl<>(); settings.useConstructor(pluginLoader, cacheManager); settings.defaultAnswer(InvocationOnMock::callRealMethod); @@ -145,7 +144,6 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") public void shouldFailWith404OnMissingResources() throws IOException { String path = "/locales/de/plugins.json"; HttpServletRequest request = mock(HttpServletRequest.class); @@ -153,7 +151,9 @@ public class I18nServletTest { PrintWriter writer = mock(PrintWriter.class); when(response.getWriter()).thenReturn(writer); when(request.getServletPath()).thenReturn(path); - when(classLoader.getResources("locales/de/plugins.json")).thenThrow(IOException.class); + when(classLoader.getResources("locales/de/plugins.json")).thenReturn( + I18nServlet.class.getClassLoader().getResources("something/not/available") + ); servlet.doGet(request, response); @@ -161,9 +161,10 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") public void shouldFailWith500OnIOException() throws IOException { + when(servlet.isProductionStage()).thenReturn(false); HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getServletPath()).thenReturn("/locales/de/plugins.json"); HttpServletResponse response = mock(HttpServletResponse.class); doThrow(IOException.class).when(response).getWriter(); @@ -173,7 +174,7 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") + @SuppressWarnings("UnstableApiUsage") public void inDevelopmentStageShouldNotUseCache() throws IOException { String path = "/locales/de/plugins.json"; when(servlet.isProductionStage()).thenReturn(false); @@ -193,7 +194,7 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") + @SuppressWarnings("UnstableApiUsage") public void inProductionStageShouldUseCache() throws IOException { String path = "/locales/de/plugins.json"; when(servlet.isProductionStage()).thenReturn(true); @@ -216,7 +217,7 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") + @SuppressWarnings("UnstableApiUsage") public void inProductionStageShouldGetFromCache() throws IOException { String path = "/locales/de/plugins.json"; when(servlet.isProductionStage()).thenReturn(true); @@ -245,7 +246,6 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") public void shouldCollectJsonFile() throws IOException { String path = "locales/de/plugins.json"; when(classLoader.getResources(path)).thenReturn(resources); From dcfd0d0d163bced6a90ed74a56d4840c6b3acff0 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 08:34:01 +0200 Subject: [PATCH 11/30] refactor I18nServletTest Use JUnit5 and only api methods --- .../java/sonia/scm/web/i18n/I18nServlet.java | 29 +-- .../sonia/scm/web/i18n/I18nServletTest.java | 203 ++++++++---------- 2 files changed, 95 insertions(+), 137 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java b/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java index 4671915a9d..11ec8b5978 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java +++ b/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java @@ -33,7 +33,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.SCMContext; +import sonia.scm.SCMContextProvider; import sonia.scm.Stage; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; @@ -66,13 +66,15 @@ public class I18nServlet extends HttpServlet { public static final String PATTERN = "/locales/[a-z\\-A-Z]*/" + PLUGINS_JSON; public static final String CACHE_NAME = "sonia.cache.plugins.translations"; + private final SCMContextProvider context; private final ClassLoader classLoader; private final Cache cache; private final ObjectMapper objectMapper = new ObjectMapper(); @Inject - public I18nServlet(PluginLoader pluginLoader, CacheManager cacheManager) { + public I18nServlet(SCMContextProvider context, PluginLoader pluginLoader, CacheManager cacheManager) { + this.context = context; this.classLoader = pluginLoader.getUberClassLoader(); this.cache = cacheManager.getCache(CACHE_NAME); } @@ -133,17 +135,11 @@ public class I18nServlet extends HttpServlet { @VisibleForTesting protected boolean isProductionStage() { - return SCMContext.getContext().getStage() == Stage.PRODUCTION; + return context.getStage() == Stage.PRODUCTION; } - /** - * Return a collected Json File as JsonNode from the given path from all plugins in the class path - * - * @param path the searched resource path - * @return a collected Json File as JsonNode from the given path from all plugins in the class path - */ @VisibleForTesting - protected Optional collectJsonFile(String path) throws IOException { + private Optional collectJsonFile(String path) throws IOException { LOG.debug("Collect plugin translations from path {} for every plugin", path); JsonNode mergedJsonNode = null; Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); @@ -160,18 +156,7 @@ public class I18nServlet extends HttpServlet { return Optional.ofNullable(mergedJsonNode); } - - /** - * Merge the updateNode into the mainNode and return it. - *

- * This is not a deep merge. - * - * @param mainNode the main node - * @param updateNode the update node - * @return the merged mainNode - */ - @VisibleForTesting - protected JsonNode merge(JsonNode mainNode, JsonNode updateNode) { + private JsonNode merge(JsonNode mainNode, JsonNode updateNode) { Iterator fieldNames = updateNode.fieldNames(); while (fieldNames.hasNext()) { String fieldName = fieldNames.next(); diff --git a/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java b/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java index 32c3fcc772..495c233906 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java +++ b/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java @@ -26,43 +26,38 @@ package sonia.scm.web.i18n; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Charsets; -import com.google.common.io.Files; +import com.github.legman.EventBus; import org.apache.commons.lang3.StringUtils; -import org.assertj.core.util.Lists; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; -import org.mockito.MockSettings; -import org.mockito.internal.creation.MockSettingsImpl; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.SCMContextProvider; +import sonia.scm.Stage; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; -import sonia.scm.event.ScmEventBus; import sonia.scm.lifecycle.RestartEventFactory; import sonia.scm.plugin.PluginLoader; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.io.File; -import java.io.FileOutputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintWriter; import java.net.URL; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; -import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; -@RunWith(MockitoJUnitRunner.Silent.class) -public class I18nServletTest { +@ExtendWith(MockitoExtension.class) +class I18nServletTest { private static final String GIT_PLUGIN_JSON = json( "{", @@ -75,6 +70,7 @@ public class I18nServletTest { "}", "}" ); + private static final String HG_PLUGIN_JSON = json( "{", "'scm-hg-plugin': {", @@ -86,6 +82,7 @@ public class I18nServletTest { "}", "}" ); + private static final String SVN_PLUGIN_JSON = json( "{", "'scm-svn-plugin': {", @@ -100,56 +97,45 @@ public class I18nServletTest { return String.join("\n", parts ).replaceAll("'", "\""); } - @Rule - public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Mock + private SCMContextProvider context; @Mock private PluginLoader pluginLoader; - @Mock - private CacheManager cacheManager; - @Mock private ClassLoader classLoader; - private I18nServlet servlet; + @Mock + private CacheManager cacheManager; @Mock private Cache cache; - private Enumeration resources; + private I18nServlet servlet; - @Before - public void init() throws IOException { - resources = Collections.enumeration(Lists.newArrayList( - createFileFromString(SVN_PLUGIN_JSON).toURI().toURL(), - createFileFromString(GIT_PLUGIN_JSON).toURI().toURL(), - createFileFromString(HG_PLUGIN_JSON).toURI().toURL() - )); + + @BeforeEach + void init() { when(pluginLoader.getUberClassLoader()).thenReturn(classLoader); when(cacheManager.getCache(I18nServlet.CACHE_NAME)).thenReturn(cache); - MockSettings settings = new MockSettingsImpl<>(); - settings.useConstructor(pluginLoader, cacheManager); - settings.defaultAnswer(InvocationOnMock::callRealMethod); - servlet = mock(I18nServlet.class, settings); + servlet = new I18nServlet(context, pluginLoader, cacheManager); } @Test - public void shouldCleanCacheOnRestartEvent() { - ScmEventBus.getInstance().register(servlet); - - ScmEventBus.getInstance().post(RestartEventFactory.create(I18nServlet.class, "Restart to reload the plugin resources")); + void shouldCleanCacheOnRestartEvent() { + EventBus eventBus = new EventBus("forTestingOnly"); + eventBus.register(servlet); + eventBus.post(RestartEventFactory.create(I18nServlet.class, "Restart to reload the plugin resources")); verify(cache).clear(); } @Test - public void shouldFailWith404OnMissingResources() throws IOException { + void shouldFailWith404OnMissingResources() throws IOException { String path = "/locales/de/plugins.json"; HttpServletRequest request = mock(HttpServletRequest.class); HttpServletResponse response = mock(HttpServletResponse.class); - PrintWriter writer = mock(PrintWriter.class); - when(response.getWriter()).thenReturn(writer); when(request.getServletPath()).thenReturn(path); when(classLoader.getResources("locales/de/plugins.json")).thenReturn( I18nServlet.class.getClassLoader().getResources("something/not/available") @@ -161,98 +147,96 @@ public class I18nServletTest { } @Test - public void shouldFailWith500OnIOException() throws IOException { - when(servlet.isProductionStage()).thenReturn(false); + void shouldFailWith500OnIOException() throws IOException { + stage(Stage.DEVELOPMENT); HttpServletRequest request = mock(HttpServletRequest.class); when(request.getServletPath()).thenReturn("/locales/de/plugins.json"); HttpServletResponse response = mock(HttpServletResponse.class); - doThrow(IOException.class).when(response).getWriter(); + when(classLoader.getResources("locales/de/plugins.json")).thenThrow(new IOException("failed")); servlet.doGet(request, response); verify(response).setStatus(500); } + private void stage(Stage stage) { + when(context.getStage()).thenReturn(stage); + } + @Test - @SuppressWarnings("UnstableApiUsage") - public void inDevelopmentStageShouldNotUseCache() throws IOException { - String path = "/locales/de/plugins.json"; - when(servlet.isProductionStage()).thenReturn(false); + void inDevelopmentStageShouldNotUseCache(@TempDir Path temp) throws IOException { + stage(Stage.DEVELOPMENT); + mockResources(temp, "locales/de/plugins.json"); HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getServletPath()).thenReturn("/locales/de/plugins.json"); + HttpServletResponse response = mock(HttpServletResponse.class); - File file = temporaryFolder.newFile(); - PrintWriter writer = new PrintWriter(new FileOutputStream(file)); - when(response.getWriter()).thenReturn(writer); - when(request.getServletPath()).thenReturn(path); - when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); + String json = doGetString(request, response); - servlet.doGet(request, response); - - String json = Files.readLines(file, Charset.defaultCharset()).get(0); assertJson(json); verify(cache, never()).get(any()); } - @Test - @SuppressWarnings("UnstableApiUsage") - public void inProductionStageShouldUseCache() throws IOException { - String path = "/locales/de/plugins.json"; - when(servlet.isProductionStage()).thenReturn(true); - HttpServletRequest request = mock(HttpServletRequest.class); - HttpServletResponse response = mock(HttpServletResponse.class); - File file = temporaryFolder.newFile(); - PrintWriter writer = new PrintWriter(new FileOutputStream(file)); + private String doGetString(HttpServletRequest request, HttpServletResponse response) throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintWriter writer = new PrintWriter(baos); when(response.getWriter()).thenReturn(writer); - when(request.getServletPath()).thenReturn(path); - when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); servlet.doGet(request, response); - String json = Files.readLines(file, Charset.defaultCharset()).get(0); - assertJson(json); - verify(cache).get(path); - verify(cache).put(eq(path), any()); + writer.flush(); + return baos.toString(StandardCharsets.UTF_8.name()); + } + private void mockResources(Path directory, String resourcePath) throws IOException { + Enumeration resources = Collections.enumeration( + Arrays.asList( + toURL(directory, "git.json", GIT_PLUGIN_JSON), + toURL(directory, "hg.json", HG_PLUGIN_JSON), + toURL(directory, "svn.json", SVN_PLUGIN_JSON) + ) + ); + when(classLoader.getResources(resourcePath)).thenReturn(resources); + } + + private URL toURL(Path directory, String name, String content) throws IOException { + Path file = directory.resolve(name); + java.nio.file.Files.write(file, content.getBytes(StandardCharsets.UTF_8)); + return file.toUri().toURL(); + } + + @Test + void shouldGetFromCacheInProductionStage() throws IOException { + String path = "/locales/de/plugins.json"; + stage(Stage.PRODUCTION); + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getServletPath()).thenReturn(path); + HttpServletResponse response = mock(HttpServletResponse.class); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode jsonNode = mapper.readTree(GIT_PLUGIN_JSON); + when(cache.get(path)).thenReturn(jsonNode); + + String json = doGetString(request, response); + assertThat(json).contains("scm-git-plugin").doesNotContain("scm-hg-plugin"); verifyHeaders(response); } @Test - @SuppressWarnings("UnstableApiUsage") - public void inProductionStageShouldGetFromCache() throws IOException { + void shouldStoreToCacheInProductionStage(@TempDir Path temp) throws IOException { String path = "/locales/de/plugins.json"; - when(servlet.isProductionStage()).thenReturn(true); + mockResources(temp, "locales/de/plugins.json"); + stage(Stage.PRODUCTION); HttpServletRequest request = mock(HttpServletRequest.class); - HttpServletResponse response = mock(HttpServletResponse.class); - File file = temporaryFolder.newFile(); - PrintWriter writer = new PrintWriter(new FileOutputStream(file)); - when(response.getWriter()).thenReturn(writer); when(request.getServletPath()).thenReturn(path); - when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); - ObjectMapper objectMapper = new ObjectMapper(); - JsonNode node = objectMapper.readTree(GIT_PLUGIN_JSON); - node = servlet.merge(node, objectMapper.readTree(HG_PLUGIN_JSON)); - node = servlet.merge(node, objectMapper.readTree(SVN_PLUGIN_JSON)); - when(cache.get(path)).thenReturn(node); + HttpServletResponse response = mock(HttpServletResponse.class); - servlet.doGet(request, response); + String json = doGetString(request, response); - String json = Files.readLines(file, Charset.defaultCharset()).get(0); - verify(servlet, never()).collectJsonFile(path); - verify(cache, never()).put(eq(path), any()); - verify(cache).get(path); - assertJson(json); + verify(cache).put(any(String.class), any(JsonNode.class)); verifyHeaders(response); - } - - @Test - public void shouldCollectJsonFile() throws IOException { - String path = "locales/de/plugins.json"; - when(classLoader.getResources(path)).thenReturn(resources); - - Optional jsonNodeOptional = servlet.collectJsonFile("/" + path); - - assertJson(jsonNodeOptional.orElse(null)); + assertJson(json); } private void verifyHeaders(HttpServletResponse response) { @@ -261,22 +245,11 @@ public class I18nServletTest { verify(response).setHeader("Cache-Control", "no-cache"); } - public void assertJson(JsonNode actual) throws IOException { - assertJson(actual.toString()); - } - - private void assertJson(String actual) throws IOException { + private void assertJson(String actual) { assertThat(actual) .isNotEmpty() .contains(StringUtils.deleteWhitespace(GIT_PLUGIN_JSON.substring(1, GIT_PLUGIN_JSON.length() - 1))) .contains(StringUtils.deleteWhitespace(HG_PLUGIN_JSON.substring(1, HG_PLUGIN_JSON.length() - 1))) .contains(StringUtils.deleteWhitespace(SVN_PLUGIN_JSON.substring(1, SVN_PLUGIN_JSON.length() - 1))); } - - private File createFileFromString(String json) throws IOException { - File file = temporaryFolder.newFile(); - Files.write(json.getBytes(Charsets.UTF_8), file); - return file; - } - } From 397f524aab2fe48538b8103d175119c0acda9dd9 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 08:42:48 +0200 Subject: [PATCH 12/30] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5dec347ce..8d96e623d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - JWT token timeout is now handled properly ([#1297](https://github.com/scm-manager/scm-manager/pull/1297)) - Fix text-overflow in danger zone ([#1298](https://github.com/scm-manager/scm-manager/pull/1298)) - Fix plugin installation error if previously a plugin was installed with the same dependency which is still pending. ([#1300](https://github.com/scm-manager/scm-manager/pull/1300)) +- Fix logging of large stacktrace for unknown language ([#1313](https://github.com/scm-manager/scm-manager/pull/1313)) ## [2.4.0] - 2020-08-14 ### Added From 97f0ebe6d754b72a84086b83ed3aad03fd3621a4 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Mon, 31 Aug 2020 16:27:10 +0200 Subject: [PATCH 13/30] add from query parameter to login link --- .../src/navigation/PrimaryNavigation.tsx | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/scm-ui/ui-components/src/navigation/PrimaryNavigation.tsx b/scm-ui/ui-components/src/navigation/PrimaryNavigation.tsx index c5e23d0c45..981af6ed79 100644 --- a/scm-ui/ui-components/src/navigation/PrimaryNavigation.tsx +++ b/scm-ui/ui-components/src/navigation/PrimaryNavigation.tsx @@ -26,8 +26,10 @@ import { WithTranslation, withTranslation } from "react-i18next"; import PrimaryNavigationLink from "./PrimaryNavigationLink"; import { Links } from "@scm-manager/ui-types"; import { binder, ExtensionPoint } from "@scm-manager/ui-extensions"; +import {withContextPath} from "../urls"; +import { withRouter, RouteComponentProps } from "react-router-dom"; -type Props = WithTranslation & { +type Props = RouteComponentProps & WithTranslation & { links: Links; }; @@ -64,11 +66,17 @@ class PrimaryNavigation extends React.Component { }; appendLogin = (navigationItems: ReactNode[], append: Appender) => { - const { t, links } = this.props; + const { t, links, location } = this.props; + + const from = location.pathname; + const loginPath = "/login"; + const to = `${loginPath}?from=${encodeURIComponent(from)}`; const props = { links, - label: t("primary-navigation.login") + label: t("primary-navigation.login"), + loginUrl: withContextPath(loginPath), + from }; if (binder.hasExtension("primary-navigation.login", props)) { @@ -76,7 +84,7 @@ class PrimaryNavigation extends React.Component { ); } else { - append("/login", "/login", "primary-navigation.login", "login"); + append(to, "/login", "primary-navigation.login", "login"); } }; @@ -128,4 +136,4 @@ class PrimaryNavigation extends React.Component { } } -export default withTranslation("commons")(PrimaryNavigation); +export default withTranslation("commons")(withRouter(PrimaryNavigation)); From a54d5c6c5dfd6a7f4b9e97fe0a0990eb83637f3b Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 09:43:39 +0200 Subject: [PATCH 14/30] Fix layout overflow on changesets with multiple tags --- CHANGELOG.md | 1 + scm-ui/ui-components/src/Tooltip.tsx | 14 +++++-- .../changesets/ChangesetTagsCollapsed.tsx | 2 +- .../changesets/ChangesetDetails.tsx | 41 ++++--------------- 4 files changed, 21 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5dec347ce..5235d386f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - JWT token timeout is now handled properly ([#1297](https://github.com/scm-manager/scm-manager/pull/1297)) - Fix text-overflow in danger zone ([#1298](https://github.com/scm-manager/scm-manager/pull/1298)) - Fix plugin installation error if previously a plugin was installed with the same dependency which is still pending. ([#1300](https://github.com/scm-manager/scm-manager/pull/1300)) +- Fix layout overflow on changesets with multiple tags ([#1314](https://github.com/scm-manager/scm-manager/pull/1314)) ## [2.4.0] - 2020-08-14 ### Added diff --git a/scm-ui/ui-components/src/Tooltip.tsx b/scm-ui/ui-components/src/Tooltip.tsx index 98dce960d2..9277ad0d01 100644 --- a/scm-ui/ui-components/src/Tooltip.tsx +++ b/scm-ui/ui-components/src/Tooltip.tsx @@ -22,12 +22,12 @@ * SOFTWARE. */ import React, { ReactNode } from "react"; -import classNames from "classnames"; type Props = { message: string; className?: string; location: string; + multiline?: boolean; children: ReactNode; }; @@ -37,9 +37,17 @@ class Tooltip extends React.Component { }; render() { - const { className, message, location, children } = this.props; + const { className, message, location, multiline, children } = this.props; + let classes = `tooltip has-tooltip-${location}`; + if (multiline) { + classes += " has-tooltip-multiline"; + } + if (className) { + classes += " " + className; + } + return ( - + {children} ); diff --git a/scm-ui/ui-components/src/repos/changesets/ChangesetTagsCollapsed.tsx b/scm-ui/ui-components/src/repos/changesets/ChangesetTagsCollapsed.tsx index 4fba6ec192..e2658618ac 100644 --- a/scm-ui/ui-components/src/repos/changesets/ChangesetTagsCollapsed.tsx +++ b/scm-ui/ui-components/src/repos/changesets/ChangesetTagsCollapsed.tsx @@ -36,7 +36,7 @@ class ChangesetTagsCollapsed extends React.Component { const { tags, t } = this.props; const message = tags.map(tag => tag.name).join(", "); return ( - + ); diff --git a/scm-ui/ui-webapp/src/repos/components/changesets/ChangesetDetails.tsx b/scm-ui/ui-webapp/src/repos/components/changesets/ChangesetDetails.tsx index 8e01cf67e7..ab691be69c 100644 --- a/scm-ui/ui-webapp/src/repos/components/changesets/ChangesetDetails.tsx +++ b/scm-ui/ui-webapp/src/repos/components/changesets/ChangesetDetails.tsx @@ -26,7 +26,7 @@ import { Trans, useTranslation, WithTranslation, withTranslation } from "react-i import classNames from "classnames"; import styled from "styled-components"; import { ExtensionPoint } from "@scm-manager/ui-extensions"; -import { Changeset, ParentChangeset, Repository, Tag } from "@scm-manager/ui-types"; +import { Changeset, ParentChangeset, Repository } from "@scm-manager/ui-types"; import { AvatarImage, AvatarWrapper, @@ -36,14 +36,15 @@ import { ChangesetDiff, ChangesetId, changesets, - ChangesetTag, + ChangesetTags, DateFromNow, + FileControlFactory, Icon, - Level + Level, + SignatureIcon } from "@scm-manager/ui-components"; import ContributorTable from "./ContributorTable"; import { Link as ReactLink } from "react-router-dom"; -import { FileControlFactory, SignatureIcon } from "@scm-manager/ui-components"; type Props = WithTranslation & { changeset: Changeset; @@ -59,16 +60,6 @@ const RightMarginP = styled.p` margin-right: 1em; `; -const TagsWrapper = styled.div` - & .tag { - margin-left: 0.25rem; - } -`; - -const SignedIcon = styled(SignatureIcon)` - padding-left: 1rem; -`; - const BottomMarginLevel = styled(Level)` margin-bottom: 1rem !important; `; @@ -224,7 +215,9 @@ class ChangesetDetails extends React.Component { )} -

{this.renderTags()}
+
+ +

{description.message.split("\n").map((item, key) => { @@ -264,24 +257,6 @@ class ChangesetDetails extends React.Component { ); } - getTags = () => { - return this.props.changeset._embedded.tags || []; - }; - - renderTags = () => { - const tags = this.getTags(); - if (tags.length > 0) { - return ( - - {tags.map((tag: Tag) => { - return ; - })} - - ); - } - return null; - }; - collapseDiffs = () => { this.setState(state => ({ collapsed: !state.collapsed From 4d8d39c4106158cd46d1f5200673fd8d75babde6 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Tue, 1 Sep 2020 10:01:00 +0200 Subject: [PATCH 15/30] remove obsolete revision decoding to utf 8 --- CHANGELOG.md | 1 + .../java/sonia/scm/api/v2/resources/SourceRootResource.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5dec347ce..794efe3b8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - JWT token timeout is now handled properly ([#1297](https://github.com/scm-manager/scm-manager/pull/1297)) - Fix text-overflow in danger zone ([#1298](https://github.com/scm-manager/scm-manager/pull/1298)) - Fix plugin installation error if previously a plugin was installed with the same dependency which is still pending. ([#1300](https://github.com/scm-manager/scm-manager/pull/1300)) +- Remove obsolete revision encoding on sources ([#1315](https://github.com/scm-manager/scm-manager/pull/1315)) ## [2.4.0] - 2020-08-14 ### Added diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java index ddb88527a1..e8ef3c1bd9 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java @@ -88,7 +88,7 @@ public class SourceRootResource { browseCommand.setPath(path); browseCommand.setOffset(offset); if (revision != null && !revision.isEmpty()) { - browseCommand.setRevision(URLDecoder.decode(revision, "UTF-8")); + browseCommand.setRevision(revision); } BrowserResult browserResult = browseCommand.getBrowserResult(); From 8d2a98832be99e4223930c5644f7710bebafc707 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 10:13:06 +0200 Subject: [PATCH 16/30] Use '@babel/plugin-proposal-class-properties' to avoid problems with undefined 'this' --- scm-ui/ui-components/src/apiclient.ts | 61 ++++++++++++++------------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/scm-ui/ui-components/src/apiclient.ts b/scm-ui/ui-components/src/apiclient.ts index 2a8f53ae0a..5378bbc944 100644 --- a/scm-ui/ui-components/src/apiclient.ts +++ b/scm-ui/ui-components/src/apiclient.ts @@ -158,31 +158,32 @@ export function createUrlWithIdentifiers(url: string): string { type ErrorListener = (error: Error) => void; class ApiClient { - constructor() { - this.notifyAndRethrow = this.notifyAndRethrow.bind(this); - } - errorListeners: ErrorListener[] = []; - get(url: string): Promise { + get = (url: string): Promise => { return fetch(createUrl(url), applyFetchOptions({})) .then(handleFailure) .catch(this.notifyAndRethrow); - } + }; - post(url: string, payload?: any, contentType = "application/json", additionalHeaders: Record = {}) { + post = ( + url: string, + payload?: any, + contentType = "application/json", + additionalHeaders: Record = {} + ) => { return this.httpRequestWithJSONBody("POST", url, contentType, additionalHeaders, payload); - } + }; - postText(url: string, payload: string, additionalHeaders: Record = {}) { + postText = (url: string, payload: string, additionalHeaders: Record = {}) => { return this.httpRequestWithTextBody("POST", url, additionalHeaders, payload); - } + }; - putText(url: string, payload: string, additionalHeaders: Record = {}) { + putText = (url: string, payload: string, additionalHeaders: Record = {}) => { return this.httpRequestWithTextBody("PUT", url, additionalHeaders, payload); - } + }; - postBinary(url: string, fileAppender: (p: FormData) => void, additionalHeaders: Record = {}) { + postBinary = (url: string, fileAppender: (p: FormData) => void, additionalHeaders: Record = {}) => { const formData = new FormData(); fileAppender(formData); @@ -192,13 +193,13 @@ class ApiClient { headers: additionalHeaders }; return this.httpRequestWithBinaryBody(options, url); - } + }; put(url: string, payload: any, contentType = "application/json", additionalHeaders: Record = {}) { return this.httpRequestWithJSONBody("PUT", url, contentType, additionalHeaders, payload); } - head(url: string) { + head = (url: string) => { let options: RequestInit = { method: "HEAD" }; @@ -206,9 +207,9 @@ class ApiClient { return fetch(createUrl(url), options) .then(handleFailure) .catch(this.notifyAndRethrow); - } + }; - delete(url: string): Promise { + delete = (url: string): Promise => { let options: RequestInit = { method: "DELETE" }; @@ -216,15 +217,15 @@ class ApiClient { return fetch(createUrl(url), options) .then(handleFailure) .catch(this.notifyAndRethrow); - } + }; - httpRequestWithJSONBody( + httpRequestWithJSONBody = ( method: string, url: string, contentType: string, additionalHeaders: Record, payload?: any - ): Promise { + ): Promise => { const options: RequestInit = { method: method, headers: additionalHeaders @@ -233,23 +234,23 @@ class ApiClient { options.body = JSON.stringify(payload); } return this.httpRequestWithBinaryBody(options, url, contentType); - } + }; - httpRequestWithTextBody( + httpRequestWithTextBody = ( method: string, url: string, additionalHeaders: Record = {}, payload: string - ) { + ) => { const options: RequestInit = { method: method, headers: additionalHeaders }; options.body = payload; return this.httpRequestWithBinaryBody(options, url, "text/plain"); - } + }; - httpRequestWithBinaryBody(options: RequestInit, url: string, contentType?: string) { + httpRequestWithBinaryBody = (options: RequestInit, url: string, contentType?: string) => { options = applyFetchOptions(options); if (contentType) { if (!options.headers) { @@ -262,7 +263,7 @@ class ApiClient { return fetch(createUrl(url), options) .then(handleFailure) .catch(this.notifyAndRethrow); - } + }; subscribe(url: string, argument: SubscriptionArgument): Cancel { const es = new EventSource(createUrlWithIdentifiers(url), { @@ -293,14 +294,14 @@ class ApiClient { return () => es.close(); } - onError(errorListener: ErrorListener) { + onError = (errorListener: ErrorListener) => { this.errorListeners.push(errorListener); - } + }; - private notifyAndRethrow(error: Error): never { + private notifyAndRethrow = (error: Error): never => { this.errorListeners.forEach(errorListener => errorListener(error)); throw error; - } + }; } export const apiClient = new ApiClient(); From 543f6da382b5d9d1454b7933aaca34eeaac6336a Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 10:46:08 +0200 Subject: [PATCH 17/30] Remove unused import --- .../java/sonia/scm/api/v2/resources/SourceRootResource.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java index e8ef3c1bd9..1256742804 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SourceRootResource.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 io.swagger.v3.oas.annotations.Operation; @@ -40,7 +40,6 @@ import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; import java.io.IOException; -import java.net.URLDecoder; import static sonia.scm.ContextEntry.ContextBuilder.entity; import static sonia.scm.NotFoundException.notFound; From b9aa8ac5a46b8b0aa4fe8a36510e2b1876fc5d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 1 Sep 2020 12:04:34 +0200 Subject: [PATCH 18/30] Map WebApplicationExceptions from JaxRS to appropriate response --- CHANGELOG.md | 1 + .../api/WebApplicationExceptionMapper.java | 63 +++++++++++++++++++ .../main/resources/locales/de/plugins.json | 4 ++ .../main/resources/locales/en/plugins.json | 4 ++ 4 files changed, 72 insertions(+) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/WebApplicationExceptionMapper.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 794efe3b8d..a64adee8f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix text-overflow in danger zone ([#1298](https://github.com/scm-manager/scm-manager/pull/1298)) - Fix plugin installation error if previously a plugin was installed with the same dependency which is still pending. ([#1300](https://github.com/scm-manager/scm-manager/pull/1300)) - Remove obsolete revision encoding on sources ([#1315](https://github.com/scm-manager/scm-manager/pull/1315)) +- Map generic JaxRS 'web application exceptions' to appropriate response instead of "internal server error" ([#1318](https://github.com/scm-manager/scm-manager/pull/1312)) ## [2.4.0] - 2020-08-14 ### Added diff --git a/scm-webapp/src/main/java/sonia/scm/api/WebApplicationExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/WebApplicationExceptionMapper.java new file mode 100644 index 0000000000..41ec6787ef --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/WebApplicationExceptionMapper.java @@ -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. + */ + +package sonia.scm.api; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; +import sonia.scm.api.v2.resources.ErrorDto; +import sonia.scm.web.VndMediaType; + +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; +import java.util.Collections; + +@Provider +public class WebApplicationExceptionMapper implements ExceptionMapper { + + private static final Logger LOG = LoggerFactory.getLogger(WebApplicationExceptionMapper.class); + + private static final String ERROR_CODE = "FVS9JY1T21"; + + @Override + public Response toResponse(WebApplicationException exception) { + LOG.trace("caught web application exception", exception); + + ErrorDto errorDto = new ErrorDto(); + errorDto.setMessage(exception.getMessage()); + errorDto.setContext(Collections.emptyList()); + errorDto.setErrorCode(ERROR_CODE); + errorDto.setTransactionId(MDC.get("transaction_id")); + + Response originalResponse = exception.getResponse(); + + return Response.fromResponse(originalResponse) + .entity(errorDto) + .type(VndMediaType.ERROR_TYPE) + .build(); + } +} diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 169c38cd85..c5ab04d950 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -268,6 +268,10 @@ "BxS5wX2v71": { "displayName": "Inkorrekter Schlüssel", "description": "Der bereitgestellte Schlüssel ist kein korrekt formartierter öffentlicher Schlüssel." + }, + "FVS9JY1T21": { + "displayName": "Fehler bei der Anfrage", + "description": "Bei der Anfrage trat ein Fehler auf. Prüfen Sie bitte den Status der HTTP Antwort und die konkrete Meldung." } }, "namespaceStrategies": { diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index c65c506386..7338d53267 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -268,6 +268,10 @@ "BxS5wX2v71": { "displayName": "Invalid key", "description": "The provided key is not a valid public key." + }, + "FVS9JY1T21": { + "displayName": "Error in the request", + "description": "While processing the request there was an error. Please check the http return status and the concrete error message." } }, "namespaceStrategies": { From 9fba2f301e209ec7ef52139d5ddee077698254d1 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Tue, 1 Sep 2020 10:01:52 +0200 Subject: [PATCH 19/30] fix default word break behaviour --- scm-ui/ui-styles/src/scm.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-ui/ui-styles/src/scm.scss b/scm-ui/ui-styles/src/scm.scss index 2b34ddf295..f4bad17eb0 100644 --- a/scm-ui/ui-styles/src/scm.scss +++ b/scm-ui/ui-styles/src/scm.scss @@ -19,7 +19,7 @@ $family-monospace: "Courier New", Monaco, Menlo, "Ubuntu Mono", "source-code-pro -moz-hyphens: auto; -ms-hyphens: auto; hyphens: auto; - word-break: break-all; + word-break: break-word; } .has-rounded-border { From f6bb7e7b57edb685d4f57bf71b5a9a446c633b20 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Tue, 1 Sep 2020 10:14:04 +0200 Subject: [PATCH 20/30] Always break on whitespace --- scm-ui/ui-styles/src/scm.scss | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scm-ui/ui-styles/src/scm.scss b/scm-ui/ui-styles/src/scm.scss index f4bad17eb0..ace55efd5f 100644 --- a/scm-ui/ui-styles/src/scm.scss +++ b/scm-ui/ui-styles/src/scm.scss @@ -15,10 +15,10 @@ $family-monospace: "Courier New", Monaco, Menlo, "Ubuntu Mono", "source-code-pro } .is-word-break { - -webkit-hyphens: auto; - -moz-hyphens: auto; - -ms-hyphens: auto; - hyphens: auto; + -webkit-hyphens: none; + -moz-hyphens: none; + -ms-hyphens: none; + hyphens: none; word-break: break-word; } From 23404364ab49bc61512d2899343af83d85db252e Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Tue, 1 Sep 2020 10:24:53 +0200 Subject: [PATCH 21/30] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a916745207..72f11a28a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix text-overflow in danger zone ([#1298](https://github.com/scm-manager/scm-manager/pull/1298)) - Fix plugin installation error if previously a plugin was installed with the same dependency which is still pending. ([#1300](https://github.com/scm-manager/scm-manager/pull/1300)) - Fix logging of large stacktrace for unknown language ([#1313](https://github.com/scm-manager/scm-manager/pull/1313)) +- Fix incorrect word breaking behaviour in markdown ([1317](https://github.com/scm-manager/scm-manager/pull/1317)) - Remove obsolete revision encoding on sources ([#1315](https://github.com/scm-manager/scm-manager/pull/1315)) - Map generic JaxRS 'web application exceptions' to appropriate response instead of "internal server error" ([#1318](https://github.com/scm-manager/scm-manager/pull/1312)) From 6ba090b82e82c6527e99e7ed9ddb6c93f5b61467 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 31 Aug 2020 15:45:31 +0200 Subject: [PATCH 22/30] Make checkbox accessible from keyboard --- CHANGELOG.md | 1 + .../src/__snapshots__/storyshots.test.ts.snap | 91 +++++++++++++------ .../src/forms/TriStateCheckbox.tsx | 7 +- 3 files changed, 69 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5f6a7e87e..ae84089240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix text-overflow in danger zone ([#1298](https://github.com/scm-manager/scm-manager/pull/1298)) - Fix plugin installation error if previously a plugin was installed with the same dependency which is still pending. ([#1300](https://github.com/scm-manager/scm-manager/pull/1300)) - Fix layout overflow on changesets with multiple tags ([#1314](https://github.com/scm-manager/scm-manager/pull/1314)) +- Make checkbox accessible from keyboard ([#1309](https://github.com/scm-manager/scm-manager/pull/1309)) - Fix logging of large stacktrace for unknown language ([#1313](https://github.com/scm-manager/scm-manager/pull/1313)) - Fix incorrect word breaking behaviour in markdown ([1317](https://github.com/scm-manager/scm-manager/pull/1317)) - Remove obsolete revision encoding on sources ([#1315](https://github.com/scm-manager/scm-manager/pull/1315)) diff --git a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap index c725cfbb6d..ccec322bf8 100644 --- a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap +++ b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap @@ -44049,10 +44049,15 @@ exports[`Storyshots Forms|Checkbox Default 1`] = ` @@ -44067,10 +44072,15 @@ exports[`Storyshots Forms|Checkbox Default 1`] = ` @@ -44085,10 +44095,15 @@ exports[`Storyshots Forms|Checkbox Default 1`] = ` @@ -44111,10 +44126,15 @@ exports[`Storyshots Forms|Checkbox Disabled 1`] = ` className="checkbox" disabled={true} > - - + + + + Checked but disabled @@ -44136,10 +44156,15 @@ exports[`Storyshots Forms|Checkbox With HelpText 1`] = `