Heed peer review remarks

This commit is contained in:
René Pfeuffer
2020-09-18 16:02:20 +02:00
parent 70dcb9ca81
commit b1c0ec15a7
11 changed files with 49 additions and 19 deletions

View File

@@ -27,6 +27,13 @@ package sonia.scm.repository;
import java.util.Collection; import java.util.Collection;
import java.util.Optional; import java.util.Optional;
/**
* Manages namespaces. Mind that namespaces do not have a lifecycle on their own, but only do exist through
* repositories. Therefore you cannot create or delete namespaces, but just change related settings like permissions
* associated with them.
*
* @since 2.6.0
*/
public interface NamespaceManager { public interface NamespaceManager {
/** /**

View File

@@ -119,11 +119,16 @@ public class RepositoryPermission implements PermissionObject, Serializable
final RepositoryPermission other = (RepositoryPermission) obj; final RepositoryPermission other = (RepositoryPermission) obj;
return Objects.equal(name, other.name) return Objects.equal(name, other.name)
&& (verbs == null && other.verbs == null || verbs != null && other.verbs != null && CollectionUtils.isEqualCollection(verbs, other.verbs)) && equalVerbs(other)
&& Objects.equal(role, other.role) && Objects.equal(role, other.role)
&& Objects.equal(groupPermission, other.groupPermission); && Objects.equal(groupPermission, other.groupPermission);
} }
public boolean equalVerbs(RepositoryPermission other) {
return verbs == null && other.verbs == null
|| verbs != null && other.verbs != null && CollectionUtils.isEqualCollection(verbs, other.verbs);
}
/** /**
* Returns the hash code value for the {@link RepositoryPermission}. * Returns the hash code value for the {@link RepositoryPermission}.
* *

View File

@@ -58,6 +58,6 @@ export type NamespaceCollection = {
export type RepositoryGroup = { export type RepositoryGroup = {
name: string; name: string;
namespace: Namespace; namespace?: Namespace;
repositories: Repository[]; repositories: Repository[];
}; };

View File

@@ -5,5 +5,10 @@
"settingsNavLink": "Einstellungen", "settingsNavLink": "Einstellungen",
"permissionsNavLink": "Berechtigungen" "permissionsNavLink": "Berechtigungen"
} }
},
"repositoryOverview": {
"settings": {
"tooltip": "Einstellungen für den Namespace"
}
} }
} }

View File

@@ -5,5 +5,10 @@
"settingsNavLink": "Settings", "settingsNavLink": "Settings",
"permissionsNavLink": "Permissions" "permissionsNavLink": "Permissions"
} }
},
"repositoryOverview": {
"settings": {
"tooltip": "Namespace related settings"
}
} }
} }

View File

@@ -26,17 +26,18 @@ import { Link } from "react-router-dom";
import { CardColumnGroup, RepositoryEntry } from "@scm-manager/ui-components"; import { CardColumnGroup, RepositoryEntry } from "@scm-manager/ui-components";
import { RepositoryGroup } from "@scm-manager/ui-types"; import { RepositoryGroup } from "@scm-manager/ui-types";
import { Icon } from "@scm-manager/ui-components"; import { Icon } from "@scm-manager/ui-components";
import { WithTranslation, withTranslation } from "react-i18next";
type Props = { type Props = WithTranslation & {
group: RepositoryGroup; group: RepositoryGroup;
}; };
class RepositoryGroupEntry extends React.Component<Props> { class RepositoryGroupEntry extends React.Component<Props> {
render() { render() {
const { group } = this.props; const { group, t } = this.props;
const settingsLink = group.namespace?._links?.permissions && ( const settingsLink = group.namespace?._links?.permissions && (
<Link to={`/namespace/${group.name}/settings`}> <Link to={`/namespace/${group.name}/settings`}>
<Icon color={"is-link"} name={"cog"} /> <Icon color={"is-link"} name={"cog"} title={t("repositoryOverview.settings.tooltip")} />
</Link> </Link>
); );
const namespaceHeader = ( const namespaceHeader = (
@@ -54,4 +55,4 @@ class RepositoryGroupEntry extends React.Component<Props> {
} }
} }
export default RepositoryGroupEntry; export default withTranslation("namespaces")(RepositoryGroupEntry);

View File

@@ -65,5 +65,5 @@ function sortByName(a, b) {
} }
function findNamespace(namespaces: NamespaceCollection, namespaceToFind: string) { function findNamespace(namespaces: NamespaceCollection, namespaceToFind: string) {
return namespaces._embedded.namespaces.filter(namespace => namespace.namespace === namespaceToFind)[0]; return namespaces._embedded.namespaces.find(namespace => namespace.namespace === namespaceToFind);
} }

View File

@@ -33,7 +33,8 @@ import {
CustomQueryFlexWrappedColumns, CustomQueryFlexWrappedColumns,
ErrorPage, ErrorPage,
Loading, Loading,
Page, PrimaryContentColumn, Page,
PrimaryContentColumn,
SecondaryNavigation, SecondaryNavigation,
SecondaryNavigationColumn, SecondaryNavigationColumn,
StateMenuContextProvider, StateMenuContextProvider,
@@ -49,6 +50,7 @@ type Props = RouteComponentProps &
namespaceName: string; namespaceName: string;
namespacesLink: string; namespacesLink: string;
namespace: Namespace; namespace: Namespace;
error: Error;
// dispatch functions // dispatch functions
fetchNamespace: (link: string, namespace: string) => void; fetchNamespace: (link: string, namespace: string) => void;

View File

@@ -60,10 +60,10 @@ import static sonia.scm.api.v2.resources.RepositoryPermissionDto.GROUP_PREFIX;
@Slf4j @Slf4j
public class NamespacePermissionResource { public class NamespacePermissionResource {
private RepositoryPermissionDtoToRepositoryPermissionMapper dtoToModelMapper; private final RepositoryPermissionDtoToRepositoryPermissionMapper dtoToModelMapper;
private RepositoryPermissionToRepositoryPermissionDtoMapper modelToDtoMapper; private final RepositoryPermissionToRepositoryPermissionDtoMapper modelToDtoMapper;
private RepositoryPermissionCollectionToDtoMapper repositoryPermissionCollectionToDtoMapper; private final RepositoryPermissionCollectionToDtoMapper repositoryPermissionCollectionToDtoMapper;
private ResourceLinks resourceLinks; private final ResourceLinks resourceLinks;
private final NamespaceManager manager; private final NamespaceManager manager;
@Inject @Inject
@@ -234,7 +234,6 @@ public class NamespacePermissionResource {
public void update(@PathParam("namespace") String namespaceName, public void update(@PathParam("namespace") String namespaceName,
@PathParam("permission-name") String permissionName, @PathParam("permission-name") String permissionName,
@Valid RepositoryPermissionDto permission) { @Valid RepositoryPermissionDto permission) {
log.info("try to update the permission with name: {}. the modified permission is: {}", permissionName, permission);
Namespace namespace = load(namespaceName); Namespace namespace = load(namespaceName);
String extractedPermissionName = getPermissionName(permissionName); String extractedPermissionName = getPermissionName(permissionName);
if (!isPermissionExist(new RepositoryPermissionDto(extractedPermissionName, isGroupPermission(permissionName)), namespace)) { if (!isPermissionExist(new RepositoryPermissionDto(extractedPermissionName, isGroupPermission(permissionName)), namespace)) {
@@ -256,7 +255,7 @@ public class NamespacePermissionResource {
} }
namespace.addPermission(newPermission); namespace.addPermission(newPermission);
manager.modify(namespace); manager.modify(namespace);
log.info("the permission with name: {} is updated.", permissionName); log.info("the permission with name: {} is updated to {}.", permissionName, permission);
} }
/** /**
@@ -296,7 +295,8 @@ public class NamespacePermissionResource {
private Predicate<RepositoryPermission> filterPermission(String name) { private Predicate<RepositoryPermission> filterPermission(String name) {
return permission -> return permission ->
getPermissionName(name).equals(permission.getName()) && permission.isGroupPermission() == isGroupPermission(name); getPermissionName(name).equals(permission.getName())
&& permission.isGroupPermission() == isGroupPermission(name);
} }
private String getPermissionName(String permissionName) { private String getPermissionName(String permissionName) {

View File

@@ -80,8 +80,7 @@ public class DefaultNamespaceManager implements NamespaceManager {
@Subscribe @Subscribe
public void cleanupDeletedNamespaces(RepositoryEvent repositoryEvent) { public void cleanupDeletedNamespaces(RepositoryEvent repositoryEvent) {
HandlerEventType eventType = repositoryEvent.getEventType(); if (namespaceRelevantChange(repositoryEvent)) {
if (eventType == HandlerEventType.DELETE || eventType == HandlerEventType.MODIFY && !repositoryEvent.getItem().getNamespace().equals(repositoryEvent.getOldItem().getNamespace())) {
Collection<String> allNamespaces = repositoryManager.getAllNamespaces(); Collection<String> allNamespaces = repositoryManager.getAllNamespaces();
String oldNamespace = getOldNamespace(repositoryEvent); String oldNamespace = getOldNamespace(repositoryEvent);
if (!allNamespaces.contains(oldNamespace)) { if (!allNamespaces.contains(oldNamespace)) {
@@ -90,6 +89,12 @@ public class DefaultNamespaceManager implements NamespaceManager {
} }
} }
public boolean namespaceRelevantChange(RepositoryEvent repositoryEvent) {
HandlerEventType eventType = repositoryEvent.getEventType();
return eventType == HandlerEventType.DELETE
|| eventType == HandlerEventType.MODIFY && !repositoryEvent.getItem().getNamespace().equals(repositoryEvent.getOldItem().getNamespace());
}
public String getOldNamespace(RepositoryEvent repositoryEvent) { public String getOldNamespace(RepositoryEvent repositoryEvent) {
if (repositoryEvent.getEventType() == HandlerEventType.DELETE) { if (repositoryEvent.getEventType() == HandlerEventType.DELETE) {
return repositoryEvent.getItem().getNamespace(); return repositoryEvent.getItem().getNamespace();

View File

@@ -95,11 +95,11 @@
}, },
"namespace": { "namespace": {
"permissionRead": { "permissionRead": {
"displayName": "read permissions on namespaces", "displayName": "Read permissions on namespaces",
"description": "May see the permissions set for namespaces" "description": "May see the permissions set for namespaces"
}, },
"permissionWrite": { "permissionWrite": {
"displayName": "modify permissions on namespaces", "displayName": "Modify permissions on namespaces",
"description": "May modify the permissions set for namespaces" "description": "May modify the permissions set for namespaces"
} }
}, },