From 27b9d2c78a3cf1a564e6f1a4225bf088a9635b1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 20 Oct 2021 13:29:47 +0200 Subject: [PATCH] Resolved branch revision in Source Extension Point (#1803) This adds the "resolved revision" of the HEAD (of the current branch, if branches are supported) to the extension point repos.sources.extensions. This "resolved revision" holds the current HEAD revision of the repository (or the selected branch, if branches are supported). This means you can check, whether the release has changed since an extension has been rendered for the first time. Co-authored-by: Konstantin Schaper --- .../changelog/content_type_resolver_api.yaml | 2 + .../repository/spi/AbstractGitCommand.java | 4 +- .../scm/repository/spi/GitModifyCommand.java | 2 +- .../scm/repository/spi/SvnModifyCommand.java | 16 +++ .../repository/spi/SvnModifyCommandTest.java | 33 +++++ scm-ui/ui-api/src/annotations.ts | 11 +- scm-ui/ui-api/src/base.ts | 5 + scm-ui/ui-api/src/branches.ts | 4 +- scm-ui/ui-api/src/changesets.ts | 4 +- scm-ui/ui-api/src/contentType.ts | 9 +- scm-ui/ui-api/src/repositories.ts | 7 +- scm-ui/ui-api/src/search.ts | 15 ++- scm-ui/ui-extensions/src/extensionPoints.ts | 12 +- .../sources/containers/SourceExtensions.tsx | 125 ++++++++++++++++-- 14 files changed, 217 insertions(+), 32 deletions(-) create mode 100644 gradle/changelog/content_type_resolver_api.yaml diff --git a/gradle/changelog/content_type_resolver_api.yaml b/gradle/changelog/content_type_resolver_api.yaml new file mode 100644 index 0000000000..7510ef1525 --- /dev/null +++ b/gradle/changelog/content_type_resolver_api.yaml @@ -0,0 +1,2 @@ +- type: Changed + description: Resolved branch revision in source extension point ([#1803](https://github.com/scm-manager/scm-manager/pull/1803)) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java index ff63829ffa..ccb1105e3c 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java @@ -278,8 +278,8 @@ class AbstractGitCommand { logger.debug("pushed changes"); } - Ref getCurrentRevision() throws IOException { - return getClone().getRepository().getRefDatabase().findRef("HEAD"); + ObjectId getCurrentObjectId() throws IOException { + return getClone().getRepository().getRefDatabase().findRef("HEAD").getObjectId(); } private Person determineAuthor(Person author) { diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index 014dc0c06e..94815c08b4 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -92,7 +92,7 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman boolean initialCommit = getClone().getRepository().getRefDatabase().getRefs().isEmpty(); if (!StringUtils.isEmpty(request.getExpectedRevision()) - && !request.getExpectedRevision().equals(getCurrentRevision().getName())) { + && !request.getExpectedRevision().equals(getCurrentObjectId().getName())) { throw new ConcurrentModificationException(ContextEntry.ContextBuilder.entity("Branch", request.getBranch() == null ? "default" : request.getBranch()).in(repository).build()); } for (ModifyCommandRequest.PartialRequest r : request.getRequests()) { diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java index 326b710874..8b30766525 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java @@ -24,6 +24,7 @@ package sonia.scm.repository.spi; +import org.apache.commons.lang.StringUtils; import org.apache.shiro.SecurityUtils; import org.tmatesoft.svn.core.SVNCommitInfo; import org.tmatesoft.svn.core.SVNDepth; @@ -32,6 +33,8 @@ import org.tmatesoft.svn.core.SVNException; import org.tmatesoft.svn.core.wc.SVNClientManager; import org.tmatesoft.svn.core.wc.SVNWCClient; import org.tmatesoft.svn.core.wc.SVNWCUtil; +import sonia.scm.ConcurrentModificationException; +import sonia.scm.ContextEntry; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Repository; import sonia.scm.repository.SvnWorkingCopyFactory; @@ -43,6 +46,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.regex.Pattern; +import static sonia.scm.ContextEntry.ContextBuilder.entity; import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.withPattern; public class SvnModifyCommand implements ModifyCommand { @@ -64,6 +68,10 @@ public class SvnModifyCommand implements ModifyCommand { SVNClientManager clientManager = SVNClientManager.newInstance(); try (WorkingCopy workingCopy = workingCopyFactory.createWorkingCopy(context, null)) { File workingDirectory = workingCopy.getDirectory(); + if (!StringUtils.isEmpty(request.getExpectedRevision()) + && !request.getExpectedRevision().equals(getCurrentRevision(clientManager, workingCopy))) { + throw new ConcurrentModificationException(entity(repository).build()); + } if (request.isDefaultPath()) { workingDirectory = Paths.get(workingDirectory.toString() + "/trunk").toFile(); } @@ -72,6 +80,14 @@ public class SvnModifyCommand implements ModifyCommand { } } + private String getCurrentRevision(SVNClientManager clientManager, WorkingCopy workingCopy) { + try { + return Integer.toString(clientManager.getStatusClient().doStatus(workingCopy.getWorkingRepository(), false).getRevision().getID()); + } catch (SVNException e) { + throw new InternalRepositoryException(entity(repository), "Could not read status of working repository", e); + } + } + private String commitChanges(SVNClientManager clientManager, File workingDirectory, String commitMessage) { try { clientManager.setAuthenticationManager(SVNWCUtil.createDefaultAuthenticationManager(getCurrentUserName(), new char[0])); diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java index eebbc5e711..caf492a5b5 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java @@ -33,6 +33,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import sonia.scm.AlreadyExistsException; +import sonia.scm.ConcurrentModificationException; import sonia.scm.repository.Person; import sonia.scm.repository.work.NoneCachingWorkingCopyPool; import sonia.scm.repository.work.WorkdirProvider; @@ -158,4 +159,36 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { assertThat(new File(workingCopy.getWorkingRepository(), "a.txt")).exists(); assertThat(new File(workingCopy.getWorkingRepository(), "a.txt")).hasContent(""); } + + @Test + public void shouldThrowExceptionIfExpectedRevisionDoesNotMatch() throws IOException { + File testfile = temporaryFolder.newFile("Test123"); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("Test123", testfile, false)); + request.setCommitMessage("this should not pass"); + request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + request.setExpectedRevision("42"); + + assertThrows(ConcurrentModificationException.class, () -> svnModifyCommand.execute(request)); + + WorkingCopy workingCopy = workingCopyFactory.createWorkingCopy(context, null); + assertThat(new File(workingCopy.getWorkingRepository(), "Test123")).doesNotExist(); + } + + @Test + @SuppressWarnings("java:S2699") // we just want to ensure that no exception is thrown + public void shouldPassIfExpectedRevisionMatchesCurrentRevision() throws IOException { + File testfile = temporaryFolder.newFile("Test123"); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("Test123", testfile, false)); + request.setCommitMessage("this should not pass"); + request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + request.setExpectedRevision("10"); + + svnModifyCommand.execute(request); + + // nothing to check here; we just want to ensure that no exception is thrown + } } diff --git a/scm-ui/ui-api/src/annotations.ts b/scm-ui/ui-api/src/annotations.ts index c745857f73..9a1f598865 100644 --- a/scm-ui/ui-api/src/annotations.ts +++ b/scm-ui/ui-api/src/annotations.ts @@ -24,16 +24,21 @@ import { AnnotatedSource, File, Link, Repository } from "@scm-manager/ui-types"; import { useQuery } from "react-query"; import { apiClient } from "./apiclient"; -import { ApiResult } from "./base"; +import { ApiResultWithFetching } from "./base"; import { repoQueryKey } from "./keys"; -export const useAnnotations = (repository: Repository, revision: string, file: File): ApiResult => { - const { isLoading, error, data } = useQuery( +export const useAnnotations = ( + repository: Repository, + revision: string, + file: File +): ApiResultWithFetching => { + const { isLoading, isFetching, error, data } = useQuery( repoQueryKey(repository, "annotations", revision, file.path), () => apiClient.get((file._links.annotate as Link).href).then((response) => response.json()) ); return { isLoading, + isFetching, error, data, }; diff --git a/scm-ui/ui-api/src/base.ts b/scm-ui/ui-api/src/base.ts index c08320d82c..d80e668052 100644 --- a/scm-ui/ui-api/src/base.ts +++ b/scm-ui/ui-api/src/base.ts @@ -34,6 +34,11 @@ export type ApiResult = { error: Error | null; data?: T; }; + +export type ApiResultWithFetching = ApiResult & { + isFetching: boolean; +}; + export type DeleteFunction = (entity: T) => void; export const useIndex = (): ApiResult => { diff --git a/scm-ui/ui-api/src/branches.ts b/scm-ui/ui-api/src/branches.ts index 5e1cff3541..b752e6786f 100644 --- a/scm-ui/ui-api/src/branches.ts +++ b/scm-ui/ui-api/src/branches.ts @@ -24,7 +24,7 @@ import { Branch, BranchCollection, BranchCreation, Link, Repository } from "@scm-manager/ui-types"; import { requiredLink } from "./links"; import { useMutation, useQuery, useQueryClient } from "react-query"; -import { ApiResult } from "./base"; +import { ApiResult, ApiResultWithFetching } from "./base"; import { branchQueryKey, repoQueryKey } from "./keys"; import { apiClient } from "./apiclient"; import { concat } from "./urls"; @@ -40,7 +40,7 @@ export const useBranches = (repository: Repository): ApiResult ); }; -export const useBranch = (repository: Repository, name: string): ApiResult => { +export const useBranch = (repository: Repository, name: string): ApiResultWithFetching => { const link = requiredLink(repository, "branches"); return useQuery(branchQueryKey(repository, name), () => apiClient.get(concat(link, encodeURIComponent(name))).then((response) => response.json()) diff --git a/scm-ui/ui-api/src/changesets.ts b/scm-ui/ui-api/src/changesets.ts index c9b27517b2..3a336390f7 100644 --- a/scm-ui/ui-api/src/changesets.ts +++ b/scm-ui/ui-api/src/changesets.ts @@ -25,7 +25,7 @@ import { Branch, Changeset, ChangesetCollection, NamespaceAndName, Repository } import { useQuery, useQueryClient } from "react-query"; import { requiredLink } from "./links"; import { apiClient } from "./apiclient"; -import { ApiResult } from "./base"; +import { ApiResult, ApiResultWithFetching } from "./base"; import { branchQueryKey, repoQueryKey } from "./keys"; import { concat } from "./urls"; @@ -42,7 +42,7 @@ export const changesetQueryKey = (repository: NamespaceAndName, id: string) => { export const useChangesets = ( repository: Repository, request?: UseChangesetsRequest -): ApiResult => { +): ApiResultWithFetching => { const queryClient = useQueryClient(); let link: string; diff --git a/scm-ui/ui-api/src/contentType.ts b/scm-ui/ui-api/src/contentType.ts index 156fa96ec8..96d75b9582 100644 --- a/scm-ui/ui-api/src/contentType.ts +++ b/scm-ui/ui-api/src/contentType.ts @@ -23,7 +23,7 @@ */ import { apiClient } from "./apiclient"; import { useQuery } from "react-query"; -import { ApiResult } from "./base"; +import { ApiResultWithFetching } from "./base"; export type ContentType = { type: string; @@ -39,10 +39,13 @@ function getContentType(url: string): Promise { }); } -export const useContentType = (url: string): ApiResult => { - const { isLoading, error, data } = useQuery(["contentType", url], () => getContentType(url)); +export const useContentType = (url: string): ApiResultWithFetching => { + const { isLoading, isFetching, error, data } = useQuery(["contentType", url], () => + getContentType(url) + ); return { isLoading, + isFetching, error, data, }; diff --git a/scm-ui/ui-api/src/repositories.ts b/scm-ui/ui-api/src/repositories.ts index c28fb74e98..077ab5656f 100644 --- a/scm-ui/ui-api/src/repositories.ts +++ b/scm-ui/ui-api/src/repositories.ts @@ -34,7 +34,7 @@ import { } from "@scm-manager/ui-types"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { apiClient } from "./apiclient"; -import { ApiResult, useIndexJsonResource, useRequiredIndexLink } from "./base"; +import { ApiResult, ApiResultWithFetching, useIndexJsonResource, useRequiredIndexLink } from "./base"; import { createQueryString } from "./utils"; import { objectLink, requiredLink } from "./links"; import { repoQueryKey } from "./keys"; @@ -253,10 +253,10 @@ export const useRunHealthCheck = () => { }; }; -export const useExportInfo = (repository: Repository): ApiResult => { +export const useExportInfo = (repository: Repository): ApiResultWithFetching => { const link = requiredLink(repository, "exportInfo"); //TODO Refetch while exporting to update the page - const { isLoading, error, data } = useQuery( + const { isLoading, isFetching, error, data } = useQuery( ["repository", repository.namespace, repository.name, "exportInfo"], () => apiClient.get(link).then((response) => response.json()), {} @@ -264,6 +264,7 @@ export const useExportInfo = (repository: Repository): ApiResult => return { isLoading, + isFetching, error: error instanceof NotFoundError ? null : error, data, }; diff --git a/scm-ui/ui-api/src/search.ts b/scm-ui/ui-api/src/search.ts index a28a6f2e7f..e32d2d8dc7 100644 --- a/scm-ui/ui-api/src/search.ts +++ b/scm-ui/ui-api/src/search.ts @@ -22,7 +22,7 @@ * SOFTWARE. */ -import { ApiResult, useIndexJsonResource, useIndexLinks } from "./base"; +import { ApiResult, ApiResultWithFetching, useIndexJsonResource, useIndexLinks } from "./base"; import { Link, QueryResult, SearchableType } from "@scm-manager/ui-types"; import { apiClient } from "./apiclient"; import { createQueryString } from "./utils"; @@ -58,10 +58,11 @@ export const useSearchCounts = (types: string[], query: string) => { apiClient.get(`${findLink(searchLinks, type)}?q=${query}&countOnly=true`).then((response) => response.json()), })) ); - const result: { [type: string]: ApiResult } = {}; + const result: { [type: string]: ApiResultWithFetching } = {}; queries.forEach((q, i) => { result[types[i]] = { isLoading: q.isLoading, + isFetching: q.isFetching, error: q.error as Error, data: (q.data as QueryResult)?.totalHits, }; @@ -141,6 +142,12 @@ const pickLang = (language: string) => { }; export const useSearchHelpContent = (language: string) => - useObserveAsync((lang) => import(`./help/search/modal.${pickLang(lang)}`).then((module) => module.default), [language]); + useObserveAsync( + (lang) => import(`./help/search/modal.${pickLang(lang)}`).then((module) => module.default), + [language] + ); export const useSearchSyntaxContent = (language: string) => - useObserveAsync((lang) => import(`./help/search/syntax.${pickLang(lang)}`).then((module) => module.default), [language]); + useObserveAsync( + (lang) => import(`./help/search/syntax.${pickLang(lang)}`).then((module) => module.default), + [language] + ); diff --git a/scm-ui/ui-extensions/src/extensionPoints.ts b/scm-ui/ui-extensions/src/extensionPoints.ts index 837f5d139f..971eeeeec2 100644 --- a/scm-ui/ui-extensions/src/extensionPoints.ts +++ b/scm-ui/ui-extensions/src/extensionPoints.ts @@ -24,8 +24,8 @@ import React from "react"; import { - File, Branch, + File, IndexResources, Links, NamespaceStrategies, @@ -136,6 +136,16 @@ export type PrimaryNavigationLogoutButtonExtension = ExtensionPointDefinition< PrimaryNavigationLogoutButtonProps >; +export type SourceExtensionProps = { + repository: Repository; + baseUrl: string; + revision: string; + extension: string; + sources: File | undefined; + path: string; +}; +export type SourceExtension = ExtensionPointDefinition<"repos.sources.extensions", SourceExtensionProps>; + export type RepositoryOverviewTopExtensionProps = { page: number; search: string; diff --git a/scm-ui/ui-webapp/src/repos/sources/containers/SourceExtensions.tsx b/scm-ui/ui-webapp/src/repos/sources/containers/SourceExtensions.tsx index 571b05732b..1488d0dac0 100644 --- a/scm-ui/ui-webapp/src/repos/sources/containers/SourceExtensions.tsx +++ b/scm-ui/ui-webapp/src/repos/sources/containers/SourceExtensions.tsx @@ -21,14 +21,14 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ -import React, { FC } from "react"; -import { Repository } from "@scm-manager/ui-types"; +import React, { FC, useEffect, useState } from "react"; +import { File, Repository } from "@scm-manager/ui-types"; import { useParams } from "react-router-dom"; import { binder, ExtensionPoint } from "@scm-manager/ui-extensions"; import { ErrorNotification, Loading, Notification } from "@scm-manager/ui-components"; import { useTranslation } from "react-i18next"; -import { useSources } from "@scm-manager/ui-api"; +import { useBranch, useChangesets, useSources } from "@scm-manager/ui-api"; const extensionPointName = "repos.sources.extensions"; @@ -48,29 +48,60 @@ const useUrlParams = () => { return { revision: revision ? decodeURIComponent(revision) : undefined, path: path, - extension + extension, }; }; -const SourceExtensions: FC = ({ repository, baseUrl }) => { - const { revision, path, extension } = useUrlParams(); - const { error, isLoading, data: sources } = useSources(repository, { revision, path }); +type PropsWithoutBranches = Props & { + revision?: string; + extension: string; + path: string; + sources?: File; +}; + +type PropsWithBranches = PropsWithoutBranches & { + revision: string; +}; + +const useWaitForInitialLoad = (isFetching: boolean) => { + const [renderedOnce, setRenderedOnce] = useState(false); + + useEffect(() => { + if (!isFetching) { + setRenderedOnce(true); + } + }, [isFetching]); + + return !renderedOnce && isFetching; +}; + +const SourceExtensionsWithBranches: FC = ({ + repository, + baseUrl, + revision, + extension, + sources, + path, +}) => { + const { isFetching, data: branch } = useBranch(repository, revision); const [t] = useTranslation("repos"); - if (error) { - return ; - } + const isLoading = useWaitForInitialLoad(isFetching); + if (isLoading) { return ; } + const resolvedRevision = branch?.revision; + const extprops = { extension, repository, revision: revision ? encodeURIComponent(revision) : "", + resolvedRevision, path, sources, - baseUrl + baseUrl, }; if (!binder.hasExtension(extensionPointName, extprops)) { @@ -80,4 +111,76 @@ const SourceExtensions: FC = ({ repository, baseUrl }) => { return ; }; +const SourceExtensionsWithoutBranches: FC = ({ + repository, + baseUrl, + revision, + extension, + sources, + path, +}) => { + const [t] = useTranslation("repos"); + + const { isFetching, data: headChangeset } = useChangesets(repository, { limit: 1 }); + const isLoading = useWaitForInitialLoad(isFetching); + + if (isLoading) { + return ; + } + + const resolvedRevision = headChangeset?._embedded?.changesets[0]?.id; + + const extprops = { + extension, + repository, + revision: revision ? encodeURIComponent(revision) : "", + resolvedRevision, + path, + sources, + baseUrl, + }; + + if (!binder.hasExtension(extensionPointName, extprops)) { + return {t("sources.extension.notBound")}; + } + + return ; +}; + +const SourceExtensions: FC = ({ repository, baseUrl }) => { + const { revision, path, extension } = useUrlParams(); + const { error, isLoading, data: sources } = useSources(repository, { revision, path }); + + if (error) { + return ; + } + if (isLoading) { + return ; + } + + if (revision && repository._links.branches) { + return ( + + ); + } else { + return ( + + ); + } +}; + export default SourceExtensions;