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 <konstantin.schaper@cloudogu.com>

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))

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 {
+    ObjectId getCurrentObjectId() throws IOException {
-      return getClone().getRepository().getRefDatabase().findRef("HEAD");
+      return getClone().getRepository().getRefDatabase().findRef("HEAD").getObjectId();
     }
 
     private Person determineAuthor(Person author) {

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()) {

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<File, File> 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<File, File> 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]));

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<File, File> 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
+  }
 }

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<AnnotatedSource> => {
+export const useAnnotations = (
-  const { isLoading, error, data } = useQuery<AnnotatedSource, Error>(
+  repository: Repository,
+  revision: string,
+  file: File
+): ApiResultWithFetching<AnnotatedSource> => {
+  const { isLoading, isFetching, error, data } = useQuery<AnnotatedSource, Error>(
     repoQueryKey(repository, "annotations", revision, file.path),
     () => apiClient.get((file._links.annotate as Link).href).then((response) => response.json())
   );
   return {
     isLoading,
+    isFetching,
     error,
     data,
   };

scm-ui/ui-api/src/base.ts

@@ -34,6 +34,11 @@ export type ApiResult<T> = {
   error: Error | null;
   data?: T;
 };
+
+export type ApiResultWithFetching<T> = ApiResult<T> & {
+  isFetching: boolean;
+};
+
 export type DeleteFunction<T> = (entity: T) => void;
 
 export const useIndex = (): ApiResult<IndexResources> => {

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<BranchCollection>
   );
 };
 
-export const useBranch = (repository: Repository, name: string): ApiResult<Branch> => {
+export const useBranch = (repository: Repository, name: string): ApiResultWithFetching<Branch> => {
   const link = requiredLink(repository, "branches");
   return useQuery<Branch, Error>(branchQueryKey(repository, name), () =>
     apiClient.get(concat(link, encodeURIComponent(name))).then((response) => response.json())

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<ChangesetCollection> => {
+): ApiResultWithFetching<ChangesetCollection> => {
   const queryClient = useQueryClient();
 
   let link: string;

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<ContentType> {
   });
 }
 
-export const useContentType = (url: string): ApiResult<ContentType> => {
+export const useContentType = (url: string): ApiResultWithFetching<ContentType> => {
-  const { isLoading, error, data } = useQuery<ContentType, Error>(["contentType", url], () => getContentType(url));
+  const { isLoading, isFetching, error, data } = useQuery<ContentType, Error>(["contentType", url], () =>
+    getContentType(url)
+  );
   return {
     isLoading,
+    isFetching,
     error,
     data,
   };

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<ExportInfo> => {
+export const useExportInfo = (repository: Repository): ApiResultWithFetching<ExportInfo> => {
   const link = requiredLink(repository, "exportInfo");
   //TODO Refetch while exporting to update the page
-  const { isLoading, error, data } = useQuery<ExportInfo, Error>(
+  const { isLoading, isFetching, error, data } = useQuery<ExportInfo, Error>(
     ["repository", repository.namespace, repository.name, "exportInfo"],
     () => apiClient.get(link).then((response) => response.json()),
     {}
@@ -264,6 +264,7 @@ export const useExportInfo = (repository: Repository): ApiResult<ExportInfo> =>
 
   return {
     isLoading,
+    isFetching,
     error: error instanceof NotFoundError ? null : error,
     data,
   };

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<number> } = {};
+  const result: { [type: string]: ApiResultWithFetching<number> } = {};
   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]
+  );

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;

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 React, { FC, useEffect, useState } from "react";
-import { Repository } from "@scm-manager/ui-types";
+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<Props> = ({ repository, baseUrl }) => {
+type PropsWithoutBranches = Props & {
-  const { revision, path, extension } = useUrlParams();
+  revision?: string;
-  const { error, isLoading, data: sources } = useSources(repository, { revision, path });
+  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<PropsWithBranches> = ({
+  repository,
+  baseUrl,
+  revision,
+  extension,
+  sources,
+  path,
+}) => {
+  const { isFetching, data: branch } = useBranch(repository, revision);
   const [t] = useTranslation("repos");
 
-  if (error) {
+  const isLoading = useWaitForInitialLoad(isFetching);
-    return <ErrorNotification error={error} />;
+
-  }
   if (isLoading) {
     return <Loading />;
   }
 
+  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<Props> = ({ repository, baseUrl }) => {
   return <ExtensionPoint name={extensionPointName} props={extprops} />;
 };
 
+const SourceExtensionsWithoutBranches: FC<PropsWithoutBranches> = ({
+  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 <Loading />;
+  }
+
+  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 <Notification type="warning">{t("sources.extension.notBound")}</Notification>;
+  }
+
+  return <ExtensionPoint name={extensionPointName} props={extprops} />;
+};
+
+const SourceExtensions: FC<Props> = ({ repository, baseUrl }) => {
+  const { revision, path, extension } = useUrlParams();
+  const { error, isLoading, data: sources } = useSources(repository, { revision, path });
+
+  if (error) {
+    return <ErrorNotification error={error} />;
+  }
+  if (isLoading) {
+    return <Loading />;
+  }
+
+  if (revision && repository._links.branches) {
+    return (
+      <SourceExtensionsWithBranches
+        repository={repository}
+        baseUrl={baseUrl}
+        revision={revision}
+        extension={extension}
+        sources={sources}
+        path={path}
+      />
+    );
+  } else {
+    return (
+      <SourceExtensionsWithoutBranches
+        repository={repository}
+        baseUrl={baseUrl}
+        revision={revision}
+        extension={extension}
+        sources={sources}
+        path={path}
+      />
+    );
+  }
+};
+
 export default SourceExtensions;