From 49a3f547cbf6a0d0b8ee6587b0986272a3344308 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 12 Feb 2024 04:20:38 +0000 Subject: [PATCH 01/13] Bump org.codehaus.mojo:flatten-maven-plugin from 1.4.0 to 1.6.0 Bumps [org.codehaus.mojo:flatten-maven-plugin](https://github.com/mojohaus/flatten-maven-plugin) from 1.4.0 to 1.6.0. - [Release notes](https://github.com/mojohaus/flatten-maven-plugin/releases) - [Commits](https://github.com/mojohaus/flatten-maven-plugin/compare/1.4.0...1.6.0) --- updated-dependencies: - dependency-name: org.codehaus.mojo:flatten-maven-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index ba27ace8cf..27ffa71948 100644 --- a/pom.xml +++ b/pom.xml @@ -94,7 +94,7 @@ org.codehaus.mojo flatten-maven-plugin - 1.4.0 + 1.6.0 true resolveCiFriendliesOnly From 10499bdd42a7cfb7fa0b48f56744f503882ab364 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 23 May 2024 04:44:19 +0000 Subject: [PATCH 02/13] Bump com.zaxxer:HikariCP from 5.0.1 to 5.1.0 Bumps [com.zaxxer:HikariCP](https://github.com/brettwooldridge/HikariCP) from 5.0.1 to 5.1.0. - [Changelog](https://github.com/brettwooldridge/HikariCP/blob/dev/CHANGES) - [Commits](https://github.com/brettwooldridge/HikariCP/compare/HikariCP-5.0.1...HikariCP-5.1.0) --- updated-dependencies: - dependency-name: com.zaxxer:HikariCP dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- backend/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/pom.xml b/backend/pom.xml index 7354901368..9efeb08451 100644 --- a/backend/pom.xml +++ b/backend/pom.xml @@ -364,7 +364,7 @@ com.zaxxer HikariCP - 5.0.1 + 5.1.0 org.testcontainers From 583b99089a9613133427a401457c7ce690fa42ca Mon Sep 17 00:00:00 2001 From: Kai Rollmann Date: Fri, 6 Sep 2024 19:06:41 +0200 Subject: [PATCH 03/13] Don't render folder if no children match --- frontend/src/js/concept-trees/ConceptTreeFolder.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frontend/src/js/concept-trees/ConceptTreeFolder.tsx b/frontend/src/js/concept-trees/ConceptTreeFolder.tsx index 0b3eda0126..6462d09643 100644 --- a/frontend/src/js/concept-trees/ConceptTreeFolder.tsx +++ b/frontend/src/js/concept-trees/ConceptTreeFolder.tsx @@ -8,6 +8,7 @@ import ConceptTree from "./ConceptTree"; import ConceptTreeNodeTextContainer from "./ConceptTreeNodeTextContainer"; import { getConceptById } from "./globalTreeStoreHelper"; import type { SearchT, TreesT } from "./reducer"; +import { isNodeInSearchResult } from "./selectors"; const Root = styled("div")` font-size: ${({ theme }) => theme.font.sm}; @@ -57,6 +58,12 @@ const ConceptTreeFolder: FC = ({ openInitially, }); + if (!search.showMismatches) { + const shouldRender = isNodeInSearchResult(conceptId, search, tree.children); + + if (!shouldRender) return null; + } + const matchingEntries = !tree.children || !tree.matchingEntries ? null From 5d0faf92ced5a597294e05b5ffe08ebaaab8bd9b Mon Sep 17 00:00:00 2001 From: Kai Rollmann Date: Fri, 6 Sep 2024 19:09:40 +0200 Subject: [PATCH 04/13] refactor concept search and concept tree list - update the way we define props - update styles to use tw - remove some unused console.logs - search: simplify - use Object.fromEntries --- .../src/js/concept-trees/ConceptTreeList.tsx | 35 ++++++++++--------- .../js/concept-trees/ConceptTreeListItem.tsx | 16 ++++----- .../js/concept-trees/globalTreeStoreHelper.ts | 9 ++--- .../FilterListMultiSelect.tsx | 3 -- .../InputMultiSelect.stories.tsx | 2 -- 5 files changed, 27 insertions(+), 38 deletions(-) diff --git a/frontend/src/js/concept-trees/ConceptTreeList.tsx b/frontend/src/js/concept-trees/ConceptTreeList.tsx index 92a5cf258d..d72f326898 100644 --- a/frontend/src/js/concept-trees/ConceptTreeList.tsx +++ b/frontend/src/js/concept-trees/ConceptTreeList.tsx @@ -1,10 +1,10 @@ -import styled from "@emotion/styled"; -import { FC, useMemo } from "react"; +import { useMemo } from "react"; import { useSelector } from "react-redux"; import type { DatasetT } from "../api/types"; import type { StateT } from "../app/reducers"; +import tw from "tailwind-styled-components"; import ConceptTreeListItem from "./ConceptTreeListItem"; import ConceptTreesLoading from "./ConceptTreesLoading"; import ConceptsProgressBar from "./ConceptsProgressBar"; @@ -18,26 +18,27 @@ import { useRootConceptIds } from "./useRootConceptIds"; @param show For historic reasons, it was necessary to only hide / show the concept tree list, instead of mounting / unmounting it. Maybe we can remove this in the future. */ -const Root = styled("div")<{ show?: boolean }>` - flex-grow: 1; - flex-shrink: 0; - flex-basis: 0; - overflow-y: auto; - -webkit-overflow-scrolling: touch; - padding: 0 10px 0; - white-space: nowrap; - margin-bottom: 10px; - display: ${({ show }) => (show ? "initial" : "none")}; +const Root = tw("div")<{ $show?: boolean }>` + flex-grow + shrink-0 + basis-0 + px-[10px] + overflow-y-auto + whitespace-nowrap + mb-[10px] + + ${({ $show }) => ($show ? "block" : "hidden")} `; -interface PropsT { +const ConceptTreeList = ({ + datasetId, +}: { datasetId: DatasetT["id"] | null; -} - -const ConceptTreeList: FC = ({ datasetId }) => { +}) => { const trees = useSelector( (state) => state.conceptTrees.trees, ); + const loading = useSelector( (state) => state.conceptTrees.loading, ); @@ -69,7 +70,7 @@ const ConceptTreeList: FC = ({ datasetId }) => { if (search.loading) return null; return ( - + {loading && } {!loading && !areTreesAvailable && !areDatasetsPristineOrLoading && ( diff --git a/frontend/src/js/concept-trees/ConceptTreeListItem.tsx b/frontend/src/js/concept-trees/ConceptTreeListItem.tsx index 47c4373c5a..b5f4bda0cb 100644 --- a/frontend/src/js/concept-trees/ConceptTreeListItem.tsx +++ b/frontend/src/js/concept-trees/ConceptTreeListItem.tsx @@ -1,5 +1,3 @@ -import { FC } from "react"; - import type { ConceptIdT } from "../api/types"; import ConceptTree from "./ConceptTree"; @@ -8,18 +6,16 @@ import { getConceptById } from "./globalTreeStoreHelper"; import type { SearchT, TreesT } from "./reducer"; import { isNodeInSearchResult } from "./selectors"; -interface PropsT { - trees: TreesT; - conceptId: ConceptIdT; - search: SearchT; - onLoadTree: (id: string) => void; -} - -const ConceptTreeListItem: FC = ({ +const ConceptTreeListItem = ({ trees, conceptId, search, onLoadTree, +}: { + trees: TreesT; + conceptId: ConceptIdT; + search: SearchT; + onLoadTree: (id: string) => void; }) => { const tree = trees[conceptId]; diff --git a/frontend/src/js/concept-trees/globalTreeStoreHelper.ts b/frontend/src/js/concept-trees/globalTreeStoreHelper.ts index dd33b35db0..d16409b7dd 100644 --- a/frontend/src/js/concept-trees/globalTreeStoreHelper.ts +++ b/frontend/src/js/concept-trees/globalTreeStoreHelper.ts @@ -180,13 +180,10 @@ export const globalSearch = async (trees: TreesT, query: string) => { // // TODO: Refactor the state and keep both root trees as well as concept trees in a single format // Then simply use that here - const formattedTrees = Object.keys(trees).reduce< - Record> - >((all, key) => { - all[key] = { [key]: trees[key] }; + const formattedTrees = Object.fromEntries( + Object.keys(trees).map((key) => [key, { [key]: trees[key] }]), + ); - return all; - }, {}); const combinedTrees = Object.assign({}, formattedTrees, window.conceptTrees); const result = Object.keys(combinedTrees) diff --git a/frontend/src/js/query-node-editor/FilterListMultiSelect.tsx b/frontend/src/js/query-node-editor/FilterListMultiSelect.tsx index 21a1d1a8ba..e665ef9968 100644 --- a/frontend/src/js/query-node-editor/FilterListMultiSelect.tsx +++ b/frontend/src/js/query-node-editor/FilterListMultiSelect.tsx @@ -157,9 +157,6 @@ const FilterListMultiSelect: FC = ({ (resolvedResponse.resolvedFilter?.value?.length || 0) > 0 || resolvedResponse.unknownCodes.length > 0; - console.log("hasSomethingToInsert", hasSomethingToInsert); - console.log("resolvedResponse", resolvedResponse); - if (!hasSomethingToInsert) return; const value = includeUnresolved diff --git a/frontend/src/js/ui-components/InputMultiSelect/InputMultiSelect.stories.tsx b/frontend/src/js/ui-components/InputMultiSelect/InputMultiSelect.stories.tsx index a85ac07233..c46acdfafd 100644 --- a/frontend/src/js/ui-components/InputMultiSelect/InputMultiSelect.stories.tsx +++ b/frontend/src/js/ui-components/InputMultiSelect/InputMultiSelect.stories.tsx @@ -31,7 +31,6 @@ const Template: Story< }, ]); const onLoad = (str: string) => { - console.log("ONLOAD MORE WITH ", str); setLoading(true); setTimeout(() => { setOptions((opts) => { @@ -52,7 +51,6 @@ const Template: Story< }; const onResolve = (csvLines: string[]) => { - console.log(csvLines); setValue(csvLines.map((line) => ({ value: line, label: line }))); }; From 758f6bbb6ae53dc906eb25abb3ffbc42637520fe Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 9 Sep 2024 04:56:57 +0000 Subject: [PATCH 05/13] Bump org.apache.logging.log4j:log4j-to-slf4j from 2.19.0 to 2.24.0 Bumps org.apache.logging.log4j:log4j-to-slf4j from 2.19.0 to 2.24.0. --- updated-dependencies: - dependency-name: org.apache.logging.log4j:log4j-to-slf4j dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- backend/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/pom.xml b/backend/pom.xml index f7ee0f053c..994601b1c9 100644 --- a/backend/pom.xml +++ b/backend/pom.xml @@ -82,7 +82,7 @@ org.apache.logging.log4j log4j-to-slf4j - 2.19.0 + 2.24.0 com.fasterxml.jackson.module From 43a3616eef0ba333b0d906266c821df3e56baa50 Mon Sep 17 00:00:00 2001 From: Max Thonagel <12283268+thoniTUB@users.noreply.github.com> Date: Mon, 9 Sep 2024 08:42:13 +0200 Subject: [PATCH 06/13] fixes skipping of structure node when not roots are set and sets parents for nested concepts --- .../datasets/concepts/FrontEndConceptBuilder.java | 13 ++++++++++--- .../models/datasets/concepts/StructureNode.java | 9 +++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/FrontEndConceptBuilder.java b/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/FrontEndConceptBuilder.java index 721fc2c920..c8daab29f2 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/FrontEndConceptBuilder.java +++ b/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/FrontEndConceptBuilder.java @@ -67,7 +67,8 @@ public FrontendRoot createRoot(NamespaceStorage storage, Subject subject) { continue; } - roots.put(allConcepts.get(i).getId(), createConceptRoot(allConcepts.get(i), storage.getStructure())); + Concept concept = allConcepts.get(i); + roots.put(concept.getId(), createConceptRoot(concept, storage.getStructure())); } if (roots.isEmpty()) { log.warn("No concepts could be collected for {} on dataset {}. The subject is possibly lacking the permission to use them.", subject.getId(), storage.getDataset() @@ -95,9 +96,15 @@ private FrontendNode createConceptRoot(Concept concept, StructureNode[] struc final MatchingStats matchingStats = concept.getMatchingStats(); + final StructureNodeId structureParent = - Arrays.stream(structureNodes).filter(sn -> sn.getContainedRoots().contains(concept.getId())).findAny().map(StructureNode::getId).orElse(null); + Arrays.stream(structureNodes) + .flatMap(StructureNode::stream) + .filter(sn -> sn.getContainedRoots().contains(concept.getId())) + .findAny() + .map(StructureNode::getId) + .orElse(null); final FrontendNode node = FrontendNode.builder() @@ -145,7 +152,7 @@ private void insertStructureNode(StructureNode structureNode, Map, Fronten contained.add(id); } - if (contained.isEmpty()) { + if (contained.isEmpty() && structureNode.getChildren().isEmpty()) { log.trace("Did not create a structure node entry for {}. Contained no concepts.", structureNode.getId()); return; } diff --git a/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/StructureNode.java b/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/StructureNode.java index bfbcbfcee0..3470b06c38 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/StructureNode.java +++ b/backend/src/main/java/com/bakdata/conquery/models/datasets/concepts/StructureNode.java @@ -3,6 +3,9 @@ import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; +import java.util.stream.Stream; +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotNull; import com.bakdata.conquery.apiv1.KeyValue; import com.bakdata.conquery.io.jackson.serializer.NsIdRef; @@ -12,8 +15,6 @@ import com.bakdata.conquery.models.identifiable.ids.specific.StructureNodeId; import com.fasterxml.jackson.annotation.JsonBackReference; import com.fasterxml.jackson.annotation.JsonManagedReference; -import jakarta.validation.Valid; -import jakarta.validation.constraints.NotNull; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Setter; @@ -41,4 +42,8 @@ public class StructureNode extends Labeled { public StructureNodeId createId() { return new StructureNodeId(dataset.getId(), parent!=null?parent.getId():null, getName()); } + + public Stream stream() { + return Stream.concat(Stream.of(this), children.stream()); + } } From 3235ec0af600050f9df82541258871e8822b9359 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 10 Sep 2024 04:26:01 +0000 Subject: [PATCH 07/13] Bump org.testcontainers:postgresql from 1.17.6 to 1.20.1 Bumps [org.testcontainers:postgresql](https://github.com/testcontainers/testcontainers-java) from 1.17.6 to 1.20.1. - [Release notes](https://github.com/testcontainers/testcontainers-java/releases) - [Changelog](https://github.com/testcontainers/testcontainers-java/blob/main/CHANGELOG.md) - [Commits](https://github.com/testcontainers/testcontainers-java/compare/1.17.6...1.20.1) --- updated-dependencies: - dependency-name: org.testcontainers:postgresql dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- backend/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/pom.xml b/backend/pom.xml index 994601b1c9..3f08d7a446 100644 --- a/backend/pom.xml +++ b/backend/pom.xml @@ -367,7 +367,7 @@ org.testcontainers postgresql - 1.17.6 + 1.20.1 test From d760e5836a03ef0e630713c74009275ebc3977a9 Mon Sep 17 00:00:00 2001 From: awildturtok <1553491+awildturtok@users.noreply.github.com> Date: Thu, 19 Sep 2024 14:29:34 +0200 Subject: [PATCH 08/13] fix casting date to integer not long --- .../conquery/sql/execution/DefaultResultSetProcessor.java | 5 +++-- .../bakdata/conquery/sql/execution/ResultSetProcessor.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/com/bakdata/conquery/sql/execution/DefaultResultSetProcessor.java b/backend/src/main/java/com/bakdata/conquery/sql/execution/DefaultResultSetProcessor.java index f14fc852b9..b17d3db261 100644 --- a/backend/src/main/java/com/bakdata/conquery/sql/execution/DefaultResultSetProcessor.java +++ b/backend/src/main/java/com/bakdata/conquery/sql/execution/DefaultResultSetProcessor.java @@ -49,13 +49,13 @@ public Boolean getBoolean(ResultSet resultSet, int columnIndex) throws SQLExcept } @Override - public Number getDate(ResultSet resultSet, int columnIndex) throws SQLException { + public Integer getDate(ResultSet resultSet, int columnIndex) throws SQLException { String dateString = resultSet.getString(columnIndex); if (dateString == null) { return null; } DateReader dateReader = config.getLocale().getDateReader(); - return dateReader.parseToLocalDate(dateString).toEpochDay(); + return (int) dateReader.parseToLocalDate(dateString).toEpochDay(); } @Override @@ -129,6 +129,7 @@ private interface Getter { * For example, calling a primitives' ResultSet getter like getDouble, getInt etc. straightaway will never return null. */ private static T checkForNullElseGet(ResultSet resultSet, int columnIndex, Getter getter, Class resultType) throws SQLException { + if (resultSet.getObject(columnIndex) == null) { return null; } diff --git a/backend/src/main/java/com/bakdata/conquery/sql/execution/ResultSetProcessor.java b/backend/src/main/java/com/bakdata/conquery/sql/execution/ResultSetProcessor.java index 6581ec8b21..074716073d 100644 --- a/backend/src/main/java/com/bakdata/conquery/sql/execution/ResultSetProcessor.java +++ b/backend/src/main/java/com/bakdata/conquery/sql/execution/ResultSetProcessor.java @@ -19,7 +19,7 @@ public interface ResultSetProcessor { Boolean getBoolean(ResultSet resultSet, int columnIndex) throws SQLException; - Number getDate(ResultSet resultSet, int columnIndex) throws SQLException; + Integer getDate(ResultSet resultSet, int columnIndex) throws SQLException; List getDateRange(ResultSet resultSet, int columnIndex) throws SQLException; From 3a1dd5305bc6a2f46428a367d65764ee7799e9ee Mon Sep 17 00:00:00 2001 From: awildturtok <1553491+awildturtok@users.noreply.github.com> Date: Thu, 19 Sep 2024 16:13:05 +0200 Subject: [PATCH 09/13] fixes multithreaded access of NumberFormat causing issues --- .../conquery/models/query/PrintSettings.java | 20 ++++++++ .../resultinfo/printers/ResultPrinters.java | 50 ++++++++++--------- .../NumberColumnStatsCollector.java | 17 ++++--- 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/backend/src/main/java/com/bakdata/conquery/models/query/PrintSettings.java b/backend/src/main/java/com/bakdata/conquery/models/query/PrintSettings.java index 6379049aee..70c9ebe266 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/query/PrintSettings.java +++ b/backend/src/main/java/com/bakdata/conquery/models/query/PrintSettings.java @@ -85,4 +85,24 @@ public PrintSettings(boolean prettyPrint, Locale locale, Namespace namespace, Co } + /** + * @implNote We are cloning, because {@link NumberFormat} is NOT thread safe. + */ + public NumberFormat getIntegerFormat() { + return (NumberFormat) integerFormat.clone(); + } + + /** + * @implNote We are cloning, because {@link NumberFormat} is NOT thread safe. + */ + public DecimalFormat getCurrencyFormat() { + return (DecimalFormat) currencyFormat.clone(); + } + + /** + * @implNote We are cloning, because {@link NumberFormat} is NOT thread safe. + */ + public NumberFormat getDecimalFormat() { + return (NumberFormat) decimalFormat.clone(); + } } diff --git a/backend/src/main/java/com/bakdata/conquery/models/query/resultinfo/printers/ResultPrinters.java b/backend/src/main/java/com/bakdata/conquery/models/query/resultinfo/printers/ResultPrinters.java index 6f1a4b6d97..8cc0a3208b 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/query/resultinfo/printers/ResultPrinters.java +++ b/backend/src/main/java/com/bakdata/conquery/models/query/resultinfo/printers/ResultPrinters.java @@ -1,6 +1,7 @@ package com.bakdata.conquery.models.query.resultinfo.printers; import java.math.BigDecimal; +import java.text.NumberFormat; import java.util.List; import java.util.Objects; import java.util.StringJoiner; @@ -32,12 +33,12 @@ public Printer printerFor(ResultType type, PrintSettings printSettings) { return switch (((ResultType.Primitive) type)) { case BOOLEAN -> new BooleanPrinter(printSettings); - case INTEGER -> new IntegerPrinter(printSettings); - case NUMERIC -> new NumericPrinter(printSettings); + case INTEGER -> NumberFormatPrinter.integerPrinter(printSettings); + case NUMERIC -> NumberFormatPrinter.decimalPrinter(printSettings); case DATE -> new DatePrinter(printSettings); case DATE_RANGE -> new DateRangePrinter(printSettings); case STRING -> new StringPrinter(); - case MONEY -> new MoneyPrinter(printSettings); + case MONEY -> MoneyPrinter.create(printSettings); }; } @@ -56,41 +57,44 @@ public String print(Object f) { } } - public record IntegerPrinter(PrintSettings cfg) implements Printer { + public record NumberFormatPrinter(NumberFormat format) implements Printer { - @Override - public String print(Object f) { - if (cfg.isPrettyPrint()) { - return cfg.getIntegerFormat().format(((Number) f).longValue()); + public static Printer integerPrinter(PrintSettings cfg){ + if (!cfg.isPrettyPrint()) { + return new StringPrinter(); } - return f.toString(); + return new NumberFormatPrinter(cfg.getIntegerFormat()); } - } - - public record NumericPrinter(PrintSettings cfg) implements Printer { - @Override - public String print(Object f) { - if (cfg.isPrettyPrint()) { - return cfg.getDecimalFormat().format(f); + public static Printer decimalPrinter(PrintSettings cfg){ + if (!cfg.isPrettyPrint()) { + return new StringPrinter(); } - return f.toString(); + return new NumberFormatPrinter(cfg.getDecimalFormat()); } - } - - public record MoneyPrinter(PrintSettings cfg) implements Printer { @Override public String print(Object f) { + return format.format(f); + } + } - if (cfg.isPrettyPrint()) { + public record MoneyPrinter(PrintSettings cfg, NumberFormat format) implements Printer { - return cfg.getDecimalFormat().format(readMoney(cfg, (Number) f)); + public static Printer create(PrintSettings cfg) { + if (!cfg.isPrettyPrint()) { + return new StringPrinter(); } - return f.toString(); + return new MoneyPrinter(cfg, (NumberFormat) cfg.getCurrencyFormat().clone()); + } + + @Override + public String print(Object f) { + final BigDecimal asMoney = readMoney(cfg, (Number) f); + return format.format(asMoney); } } diff --git a/backend/src/main/java/com/bakdata/conquery/models/query/statistics/NumberColumnStatsCollector.java b/backend/src/main/java/com/bakdata/conquery/models/query/statistics/NumberColumnStatsCollector.java index 5cf76a8cf3..e2f83d13be 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/query/statistics/NumberColumnStatsCollector.java +++ b/backend/src/main/java/com/bakdata/conquery/models/query/statistics/NumberColumnStatsCollector.java @@ -1,6 +1,5 @@ package com.bakdata.conquery.models.query.statistics; -import java.text.DecimalFormat; import java.text.NumberFormat; import java.util.Arrays; import java.util.Collections; @@ -25,7 +24,8 @@ public class NumberColumnStatsCollector> private final ResultType type; private final DescriptiveStatistics statistics = new DescriptiveStatistics(); - private int nulls = 0; + private final NumberFormat decimalFormat; + private int nulls; private final Comparator comparator; @@ -48,13 +48,16 @@ public NumberColumnStatsCollector(String name, String label, String description, this.expectedBins = expectedBins; this.upperPercentile = upperPercentile; this.lowerPercentile = lowerPercentile; + + // Clone to ensure Thread-safe access + decimalFormat = printSettings.getDecimalFormat(); } private static NumberFormat selectFormatter(ResultType type, PrintSettings printSettings) { return switch (((ResultType.Primitive) type)) { - case INTEGER -> ((NumberFormat) printSettings.getIntegerFormat().clone()); - case MONEY -> ((DecimalFormat) printSettings.getCurrencyFormat().clone()); - default -> ((NumberFormat) printSettings.getDecimalFormat().clone()); + case INTEGER -> printSettings.getIntegerFormat(); + case MONEY -> printSettings.getCurrencyFormat(); + default -> printSettings.getDecimalFormat(); }; } @@ -156,7 +159,7 @@ private Map getExtras() { // mean is always a decimal number, therefore integer needs special handling if(ResultType.Primitive.INTEGER.equals(getType())){ - out.put(labels.mean(), getPrintSettings().getDecimalFormat().format(getStatistics().getMean())); + out.put(labels.mean(), decimalFormat.format(getStatistics().getMean())); } else { out.put(labels.mean(), printValue(getStatistics().getMean())); @@ -166,7 +169,7 @@ private Map getExtras() { out.put(labels.median(), printValue(getStatistics().getPercentile(50))); out.put(labels.p75(), printValue(getStatistics().getPercentile(75))); - out.put(labels.std(), getPrintSettings().getDecimalFormat().format(getStatistics().getStandardDeviation())); + out.put(labels.std(), decimalFormat.format(getStatistics().getStandardDeviation())); out.put(labels.sum(), printValue(getStatistics().getSum())); out.put(labels.count(), getPrintSettings().getIntegerFormat().format(getStatistics().getN())); From 6953feaf5d9fce62b9bb65274c48e2ce6af2251b Mon Sep 17 00:00:00 2001 From: awildturtok <1553491+awildturtok@users.noreply.github.com> Date: Thu, 19 Sep 2024 17:02:19 +0200 Subject: [PATCH 10/13] fixes copy paste error for money printing --- .../com/bakdata/conquery/models/types/ResultTypeTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/test/java/com/bakdata/conquery/models/types/ResultTypeTest.java b/backend/src/test/java/com/bakdata/conquery/models/types/ResultTypeTest.java index 283255fe98..45888ecd27 100644 --- a/backend/src/test/java/com/bakdata/conquery/models/types/ResultTypeTest.java +++ b/backend/src/test/java/com/bakdata/conquery/models/types/ResultTypeTest.java @@ -63,8 +63,8 @@ public static List testData() { .intValue()), "12.07.2013 - 12.07.2014"), Arguments.of(PRETTY, ResultType.Primitive.INTEGER, 51839274, "51,839,274"), Arguments.of(PRETTY_DE, ResultType.Primitive.INTEGER, 51839274, "51.839.274"), - Arguments.of(PRETTY, ResultType.Primitive.MONEY, 51839274L, "518,392.74"), - Arguments.of(PRETTY_DE, ResultType.Primitive.MONEY, 51839274L, "518.392,74"), + Arguments.of(PRETTY, ResultType.Primitive.MONEY, 51839274L, "€518,392.74"), + Arguments.of(PRETTY_DE, ResultType.Primitive.MONEY, 51839274L, "518.392,74 €"), Arguments.of(PRETTY, ResultType.Primitive.NUMERIC, 0.2, "0.2"), Arguments.of(PRETTY_DE, ResultType.Primitive.NUMERIC, 0.2, "0,2"), Arguments.of(PRETTY, ResultType.Primitive.NUMERIC, new BigDecimal("716283712389817246892743124.12312"), "716,283,712,389,817,246,892,743,124.12312"), From e76a4c641b4828d94b5153fb2d4364b49ca39849 Mon Sep 17 00:00:00 2001 From: awildturtok <1553491+awildturtok@users.noreply.github.com> Date: Mon, 23 Sep 2024 10:47:06 +0200 Subject: [PATCH 11/13] fixes non breaking space comparison with Excel --- .../conquery/io/result/excel/ExcelResultRenderTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/src/test/java/com/bakdata/conquery/io/result/excel/ExcelResultRenderTest.java b/backend/src/test/java/com/bakdata/conquery/io/result/excel/ExcelResultRenderTest.java index 9cda4e839d..2ffb17008b 100644 --- a/backend/src/test/java/com/bakdata/conquery/io/result/excel/ExcelResultRenderTest.java +++ b/backend/src/test/java/com/bakdata/conquery/io/result/excel/ExcelResultRenderTest.java @@ -171,9 +171,10 @@ private void joinValue(StringJoiner valueJoiner, Object val, ResultInfo info) { printVal = (Boolean) val ? "TRUE" : "FALSE"; } - if (info.getType().equals(ResultType.Primitive.MONEY)) { - printVal = printVal + " €"; + if(info.getType().equals(ResultType.Primitive.MONEY)){ + printVal = printVal.replace(" ", " "); } + valueJoiner.add(printVal); } From a195f813a12f389a0421579c56c4372579473bd0 Mon Sep 17 00:00:00 2001 From: Max Thonagel <12283268+thoniTUB@users.noreply.github.com> Date: Mon, 7 Oct 2024 18:02:06 +0200 Subject: [PATCH 12/13] show warning in ui if user or role could not be resolved --- .../conquery/models/auth/entities/Group.java | 26 ++-- .../models/auth/entities/RoleOwner.java | 6 +- .../conquery/models/auth/entities/User.java | 112 +++++++------- .../oidc/IntrospectionDelegatingRealm.java | 136 ++++++++-------- .../resources/admin/rest/AdminProcessor.java | 18 ++- .../resources/admin/rest/GroupResource.java | 10 +- .../resources/admin/rest/RoleResource.java | 9 +- .../resources/admin/rest/UIProcessor.java | 146 +++++++++++------- .../resources/admin/rest/UserResource.java | 12 +- .../admin/ui/model/FrontendGroupContent.java | 4 +- .../model/FrontendPermissionOwnerContent.java | 5 +- .../admin/ui/model/FrontendRoleOwner.java | 2 +- .../admin/ui/model/FrontendUserContent.java | 2 +- .../com/bakdata/conquery/util/AuthUtil.java | 2 +- .../resources/admin/ui/group.html.ftl | 21 ++- .../conquery/resources/admin/ui/role.html.ftl | 8 +- .../admin/ui/templates/groupHandler.html.ftl | 4 +- .../admin/ui/templates/roleHandler.html.ftl | 21 ++- .../conquery/resources/admin/ui/user.html.ftl | 8 +- .../integration/tests/GroupHandlingTest.java | 2 +- .../tests/PermissionGroupHandlingTest.java | 2 +- .../tests/PermissionRoleHandlingTest.java | 2 +- .../integration/tests/RestartTest.java | 4 +- .../tests/RoleHandlingOnGroupTest.java | 2 +- .../integration/tests/RoleHandlingTest.java | 4 +- .../backend-admin-ui/test_3_smoketest.cy.js | 92 +++++++++-- cypress/integration-helpers/visitAdminUI.js | 8 +- 27 files changed, 385 insertions(+), 283 deletions(-) diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/Group.java b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/Group.java index 2d34ddadc8..7287547901 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/Group.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/Group.java @@ -45,6 +45,13 @@ public Set getEffectivePermissions() { return permissions; } + public synchronized void addMember(User user) { + if (members.add(user.getId())) { + log.trace("Added user {} to group {}", user.getId(), getId()); + updateStorage(); + } + } + @Override public void updateStorage() { storage.updateGroup(this); @@ -55,16 +62,9 @@ public GroupId createId() { return new GroupId(name); } - public synchronized void addMember(User user) { - if (members.add(user.getId())) { - log.trace("Added user {} to group {}", user.getId(), getId()); - updateStorage(); - } - } - - public synchronized void removeMember(User user) { - if (members.remove(user.getId())) { - log.trace("Removed user {} from group {}", user.getId(), getId()); + public synchronized void removeMember(UserId user) { + if (members.remove(user)) { + log.trace("Removed user {} from group {}", user, getId()); updateStorage(); } } @@ -84,9 +84,9 @@ public synchronized void addRole(Role role) { } } - public synchronized void removeRole(Role role) { - if (roles.remove(role.getId())) { - log.trace("Removed role {} from group {}", role.getId(), getId()); + public synchronized void removeRole(RoleId role) { + if (roles.remove(role)) { + log.trace("Removed role {} from group {}", role, getId()); updateStorage(); } } diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/RoleOwner.java b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/RoleOwner.java index b2be368dd5..b352829e80 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/RoleOwner.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/RoleOwner.java @@ -2,18 +2,14 @@ import java.util.Set; -import com.bakdata.conquery.io.storage.MetaStorage; -import com.bakdata.conquery.models.auth.permissions.ConqueryPermission; import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; import com.fasterxml.jackson.annotation.JsonIgnore; -import com.google.common.collect.Sets; -import lombok.extern.slf4j.Slf4j; public interface RoleOwner { void addRole(Role role); - void removeRole(Role role); + void removeRole(RoleId role); /** * Return a copy of the roles hold by the owner. diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/User.java b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/User.java index bc3b1fc8d1..142ea20e8a 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/User.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/User.java @@ -72,11 +72,6 @@ public Set getEffectivePermissions() { return permissions; } - @Override - public UserId createId() { - return new UserId(name); - } - public synchronized void addRole(Role role) { if (roles.add(role.getId())) { log.trace("Added role {} to user {}", role.getId(), getId()); @@ -85,9 +80,9 @@ public synchronized void addRole(Role role) { } @Override - public synchronized void removeRole(Role role) { - if (roles.remove(role.getId())) { - log.trace("Removed role {} from user {}", role.getId(), getId()); + public synchronized void removeRole(RoleId role) { + if (roles.remove(role)) { + log.trace("Removed role {} from user {}", role, getId()); updateStorage(); } } @@ -101,7 +96,59 @@ public void updateStorage() { storage.updateUser(this); } - public void authorize(@NonNull Authorized object, @NonNull Ability ability) { + @Override + public UserId createId() { + return new UserId(name); + } + + /** + * This class is non-static so it's a fixed part of the enclosing User object. + * It's protected for testing purposes only. + */ + public class ShiroUserAdapter extends FilteredUser { + + @Getter + private final ThreadLocal authenticationInfo = + ThreadLocal.withInitial(() -> new ConqueryAuthenticationInfo(User.this, null, null, false, null)); + + @Override + public Object getPrincipal() { + return getId(); + } @Override + public void checkPermission(Permission permission) throws AuthorizationException { + SecurityUtils.getSecurityManager().checkPermission(getPrincipals(), permission); + } + + @Override + public void checkPermissions(Collection permissions) throws AuthorizationException { + SecurityUtils.getSecurityManager().checkPermissions(getPrincipals(), permissions); + } + + @Override + public PrincipalCollection getPrincipals() { + return authenticationInfo.get().getPrincipals(); + } + + @Override + public boolean isPermitted(Permission permission) { + return SecurityUtils.getSecurityManager().isPermitted(getPrincipals(), permission); + } + + @Override + public boolean[] isPermitted(List permissions) { + return SecurityUtils.getSecurityManager().isPermitted(getPrincipals(), permissions); + } + + @Override + public boolean isPermittedAll(Collection permissions) { + return SecurityUtils.getSecurityManager().isPermittedAll(getPrincipals(), permissions); + } + + + + + + } public void authorize(@NonNull Authorized object, @NonNull Ability ability) { if (isOwner(object)) { return; } @@ -168,52 +215,5 @@ public User getUser() { } - /** - * This class is non-static so it's a fixed part of the enclosing User object. - * It's protected for testing purposes only. - */ - public class ShiroUserAdapter extends FilteredUser { - - @Getter - private final ThreadLocal authenticationInfo = - ThreadLocal.withInitial(() -> new ConqueryAuthenticationInfo(User.this, null, null, false, null)); - - @Override - public void checkPermission(Permission permission) throws AuthorizationException { - SecurityUtils.getSecurityManager().checkPermission(getPrincipals(), permission); - } - - @Override - public void checkPermissions(Collection permissions) throws AuthorizationException { - SecurityUtils.getSecurityManager().checkPermissions(getPrincipals(), permissions); - } - @Override - public PrincipalCollection getPrincipals() { - return authenticationInfo.get().getPrincipals(); - } - - @Override - public boolean isPermitted(Permission permission) { - return SecurityUtils.getSecurityManager().isPermitted(getPrincipals(), permission); - } - - @Override - public boolean[] isPermitted(List permissions) { - return SecurityUtils.getSecurityManager().isPermitted(getPrincipals(), permissions); - } - - @Override - public boolean isPermittedAll(Collection permissions) { - return SecurityUtils.getSecurityManager().isPermittedAll(getPrincipals(), permissions); - } - - - @Override - public Object getPrincipal() { - return getId(); - } - - - } } diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/oidc/IntrospectionDelegatingRealm.java b/backend/src/main/java/com/bakdata/conquery/models/auth/oidc/IntrospectionDelegatingRealm.java index 4913cf3c6a..f3f668a5b6 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/oidc/IntrospectionDelegatingRealm.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/oidc/IntrospectionDelegatingRealm.java @@ -83,6 +83,20 @@ public IntrospectionDelegatingRealm(MetaStorage storage, IntrospectionDelegating this.keycloakApi = keycloakApi; } + private static String extractDisplayName(JWTClaimsSet claims) { + try { + final String name = claims.getStringClaim("name"); + + if (StringUtils.isBlank(name)) { + throw new UnsupportedTokenException("Claim 'name' was empty"); + } + return name; + } + catch (java.text.ParseException e) { + throw new IncorrectCredentialsException("Unable to extract username from token", e); + } + } + @Override protected void onInit() { super.onInit(); @@ -126,7 +140,6 @@ public ConqueryAuthenticationInfo doGetAuthenticationInfo(AuthenticationToken to return new ConqueryAuthenticationInfo(user, token, this, true, URI.create(logoutEndpoint)); } - /** * Is called on every request to ensure that the token is still valid. */ @@ -163,6 +176,12 @@ else if (!(response instanceof TokenIntrospectionSuccessResponse)) { } + private static UserId getUserId(JWTClaimsSet claims) { + final String subject = claims.getSubject(); + UserId userId = new UserId(subject); + log.trace("Extracted UserId {}", userId); + return userId; + } private void validateAudiences(JWTClaimsSet claims) { // Check if the token is intended for our client/resource @@ -182,28 +201,6 @@ private void validateAudiences(JWTClaimsSet claims) { } } - private static UserId getUserId(JWTClaimsSet claims) { - final String subject = claims.getSubject(); - UserId userId = new UserId(subject); - log.trace("Extracted UserId {}", userId); - return userId; - } - - private static String extractDisplayName(JWTClaimsSet claims) { - try { - final String name = claims.getStringClaim("name"); - - if (StringUtils.isBlank(name)) { - throw new UnsupportedTokenException("Claim 'name' was empty"); - } - return name; - } - catch (java.text.ParseException e) { - throw new IncorrectCredentialsException("Unable to extract username from token", e); - } - } - - /** * Validates token and synchronizes user and its group memberships with keycloak. */ @@ -232,6 +229,32 @@ public JWTClaimsSet load(String token) throws Exception { return claimsSet; } + private synchronized User getOrCreateUser(JWTClaimsSet claims) { + UserId userId = getUserId(claims); + final String displayName = extractDisplayName(claims); + + User user = storage.getUser(userId); + + if (user != null) { + log.trace("Found existing user: {}", user); + // Update display name if necessary + if (!user.getLabel().equals(displayName)) { + log.info("Updating display name of user [{}]: '{}' -> '{}'", user.getName(), user.getLabel(), displayName); + user.setLabel(displayName); + user.updateStorage(); + } + + return user; + } + + // Construct a new User if none could be found in the storage + user = new User(userId.getName(), displayName, storage); + storage.addUser(user); + log.info("Created new user: {}", user); + + return user; + } + @Nullable private Set getUserGroups(JWTClaimsSet claims, String groupIdAttribute) { final Set userGroups = keycloakApi.getUserGroups(claims.getSubject()); @@ -247,6 +270,25 @@ private Set getUserGroups(JWTClaimsSet claims, String groupIdAttribute) { .collect(Collectors.toSet()); } + private void syncGroupMappings(User user, Set mappedGroupsToDo) { + // TODO mark mappings as managed by keycloak + for (Group group : storage.getAllGroups()) { + if (group.containsMember(user)) { + if (mappedGroupsToDo.contains(group)) { + // Mapping is still valid, remove from todo-list + mappedGroupsToDo.remove(group); + } + else { + // Mapping is not valid any more remove user from group + group.removeMember(user.getId()); + } + } + } + + for (Group group : mappedGroupsToDo) { + group.addMember(user); + } + } private Optional tryGetGroup(KeycloakGroup keycloakGroup) { @@ -306,54 +348,6 @@ private synchronized Group createGroup(String name, String label) { return group; } - - private void syncGroupMappings(User user, Set mappedGroupsToDo) { - // TODO mark mappings as managed by keycloak - for (Group group : storage.getAllGroups()) { - if (group.containsMember(user)) { - if (mappedGroupsToDo.contains(group)) { - // Mapping is still valid, remove from todo-list - mappedGroupsToDo.remove(group); - } - else { - // Mapping is not valid any more remove user from group - group.removeMember(user); - } - } - } - - for (Group group : mappedGroupsToDo) { - group.addMember(user); - } - } - - - private synchronized User getOrCreateUser(JWTClaimsSet claims) { - UserId userId = getUserId(claims); - final String displayName = extractDisplayName(claims); - - User user = storage.getUser(userId); - - if (user != null) { - log.trace("Found existing user: {}", user); - // Update display name if necessary - if (!user.getLabel().equals(displayName)) { - log.info("Updating display name of user [{}]: '{}' -> '{}'", user.getName(), user.getLabel(), displayName); - user.setLabel(displayName); - user.updateStorage(); - } - - return user; - } - - // Construct a new User if none could be found in the storage - user = new User(userId.getName(), displayName, storage); - storage.addUser(user); - log.info("Created new user: {}", user); - - return user; - } - } } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/AdminProcessor.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/AdminProcessor.java index b02fa9bd33..aab83fa3a7 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/AdminProcessor.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/AdminProcessor.java @@ -11,6 +11,7 @@ import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; +import jakarta.validation.Validator; import com.bakdata.conquery.commands.ManagerNode; import com.bakdata.conquery.io.jackson.Jackson; @@ -25,6 +26,8 @@ import com.bakdata.conquery.models.config.ConqueryConfig; import com.bakdata.conquery.models.exceptions.JSONException; import com.bakdata.conquery.models.exceptions.ValidatorHelper; +import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; +import com.bakdata.conquery.models.identifiable.ids.specific.UserId; import com.bakdata.conquery.models.index.IndexKey; import com.bakdata.conquery.models.jobs.JobManager; import com.bakdata.conquery.models.jobs.JobManagerStatus; @@ -37,7 +40,6 @@ import com.google.common.collect.Multimap; import com.univocity.parsers.csv.CsvWriter; import groovy.lang.GroovyShell; -import jakarta.validation.Validator; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -87,7 +89,7 @@ public synchronized void addRole(Role role) throws JSONException { * * @param role the role to delete */ - public void deleteRole(Role role) { + public void deleteRole(RoleId role) { log.info("Deleting {}", role); for (User user : storage.getAllUsers()) { @@ -98,7 +100,7 @@ public void deleteRole(Role role) { group.removeRole(role); } - storage.removeRole(role.getId()); + storage.removeRole(role); } public SortedSet getAllRoles() { @@ -132,12 +134,12 @@ public TreeSet getAllUsers() { return new TreeSet<>(storage.getAllUsers()); } - public synchronized void deleteUser(User user) { + public synchronized void deleteUser(UserId user) { for (Group group : storage.getAllGroups()) { group.removeMember(user); } - storage.removeUser(user.getId()); - log.trace("Removed user {} from the storage.", user.getId()); + storage.removeUser(user); + log.trace("Removed user {} from the storage.", user); } public void addUsers(List users) { @@ -185,7 +187,7 @@ public void addUserToGroup(Group group, User user) { log.trace("Added user {} to group {}", user, group); } - public void deleteUserFromGroup(Group group, User user) { + public void deleteUserFromGroup(Group group, UserId user) { group.removeMember(user); log.trace("Removed user {} from group {}", user, group); } @@ -195,7 +197,7 @@ public void deleteGroup(Group group) { log.trace("Removed group {}", group); } - public void deleteRoleFrom(RoleOwner owner, Role role) { + public void deleteRoleFrom(RoleOwner owner, RoleId role) { owner.removeRole(role); log.trace("Removed role {} from {}", role, owner); } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/GroupResource.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/GroupResource.java index d9aca561ac..cb7e529e10 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/GroupResource.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/GroupResource.java @@ -4,7 +4,6 @@ import java.util.Collection; import java.util.List; - import jakarta.inject.Inject; import jakarta.validation.constraints.NotEmpty; import jakarta.ws.rs.Consumes; @@ -20,6 +19,8 @@ import com.bakdata.conquery.models.auth.entities.Group; import com.bakdata.conquery.models.auth.entities.Role; import com.bakdata.conquery.models.auth.entities.User; +import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; +import com.bakdata.conquery.models.identifiable.ids.specific.UserId; import lombok.RequiredArgsConstructor; @Produces(MediaType.APPLICATION_JSON) @@ -50,9 +51,8 @@ public Response deleteGroup(@PathParam(GROUP_ID) Group group) { } @POST - public Response postGroups(@NotEmpty List groups) { + public void postGroups(@NotEmpty List groups) { processor.addGroups(groups); - return Response.ok().build(); } @Path("{" + GROUP_ID + "}/" + USERS_PATH_ELEMENT + "/{" + USER_ID + "}") @@ -64,14 +64,14 @@ public Response addUserToGroup(@PathParam(GROUP_ID) Group group, @PathParam(USER @Path("{" + GROUP_ID + "}/" + USERS_PATH_ELEMENT + "/{" + USER_ID + "}") @DELETE - public Response deleteUserFromGroup(@PathParam(GROUP_ID) Group group, @PathParam(USER_ID) User user) { + public Response deleteUserFromGroup(@PathParam(GROUP_ID) Group group, @PathParam(USER_ID) UserId user) { processor.deleteUserFromGroup(group, user); return Response.ok().build(); } @Path("{" + GROUP_ID + "}/" + ROLES_PATH_ELEMENT + "/{" + ROLE_ID + "}") @DELETE - public Response deleteRoleFromUser(@PathParam(GROUP_ID) Group group, @PathParam(ROLE_ID) Role role) { + public Response deleteRoleFromUser(@PathParam(GROUP_ID) Group group, @PathParam(ROLE_ID) RoleId role) { processor.deleteRoleFrom(group, role); return Response.ok().build(); } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/RoleResource.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/RoleResource.java index b4cfdf3ca8..5997d07245 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/RoleResource.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/RoleResource.java @@ -4,9 +4,6 @@ import static com.bakdata.conquery.resources.ResourceConstants.ROLE_ID; import java.util.Collection; - -import com.bakdata.conquery.models.auth.entities.Role; -import com.bakdata.conquery.models.exceptions.JSONException; import jakarta.inject.Inject; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.DELETE; @@ -17,6 +14,10 @@ import jakarta.ws.rs.Produces; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; + +import com.bakdata.conquery.models.auth.entities.Role; +import com.bakdata.conquery.models.exceptions.JSONException; +import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; import lombok.RequiredArgsConstructor; @Consumes(MediaType.APPLICATION_JSON) @@ -46,7 +47,7 @@ public Response getRole(@PathParam(ROLE_ID) Role role) throws JSONException { @Path("{" + ROLE_ID + "}") @DELETE - public Response deleteRole(@PathParam(ROLE_ID) Role role) throws JSONException { + public Response deleteRole(@PathParam(ROLE_ID) RoleId role) throws JSONException { processor.deleteRole(role); return Response.ok().build(); } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/UIProcessor.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/UIProcessor.java index 17c89ccdbf..edac579d1f 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/UIProcessor.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/UIProcessor.java @@ -12,6 +12,7 @@ import java.util.TreeSet; import java.util.function.Predicate; import java.util.stream.Collectors; +import jakarta.inject.Inject; import com.bakdata.conquery.io.cps.CPSTypeIdResolver; import com.bakdata.conquery.io.storage.MetaStorage; @@ -30,6 +31,7 @@ import com.bakdata.conquery.models.datasets.concepts.tree.ConceptTreeNode; import com.bakdata.conquery.models.datasets.concepts.tree.TreeConcept; import com.bakdata.conquery.models.events.CBlock; +import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; import com.bakdata.conquery.models.identifiable.ids.specific.UserId; import com.bakdata.conquery.models.index.IndexKey; import com.bakdata.conquery.models.worker.DatasetRegistry; @@ -43,7 +45,6 @@ import com.bakdata.conquery.resources.admin.ui.model.TableStatistics; import com.bakdata.conquery.resources.admin.ui.model.UIContext; import com.google.common.cache.CacheStats; -import jakarta.inject.Inject; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -60,14 +61,6 @@ public class UIProcessor { @Getter private final AdminProcessor adminProcessor; - public DatasetRegistry getDatasetRegistry() { - return adminProcessor.getDatasetRegistry(); - } - - public MetaStorage getStorage() { - return adminProcessor.getStorage(); - } - public UIContext getUIContext(String csrfToken) { return new UIContext(adminProcessor.getNodeProvider(), csrfToken); } @@ -99,17 +92,91 @@ public FrontendAuthOverview getAuthOverview() { return FrontendAuthOverview.builder().overview(overview).build(); } + public MetaStorage getStorage() { + return adminProcessor.getStorage(); + } + + public FrontendGroupContent getGroupContent(Group group) { + + Set membersIds = group.getMembers(); + ArrayList availableMembers = new ArrayList<>(getStorage().getAllUsers()); + availableMembers.removeIf(u -> membersIds.contains(u.getId())); + + List members = membersIds.stream() + .map(id -> { + User user = getStorage().getUser(id); + if (user != null) { + return getUserContent(user); + } + return FrontendUserContent.builder().id(id).build(); + }) + .toList(); + + List roles = group.getRoles().stream() + .map(this::getFrontendRoleContent) + .toList(); + + return FrontendGroupContent + .builder() + .resolvable(true) + .label(group.getLabel()) + .id(group.getId()) + .members(members) + .availableMembers(availableMembers) + .roles(roles) + .availableRoles(getStorage().getAllRoles()) + .permissions(wrapInFEPermission(group.getPermissions())) + .permissionTemplateMap(preparePermissionTemplate()) + .build(); + } + + private FrontendRoleContent getFrontendRoleContent(RoleId id) { + Role role = getStorage().getRole(id); + if (role != null) { + return getRoleContent(role); + } + return FrontendRoleContent.builder().id(id).build(); + } + + public FrontendUserContent getUserContent(User user) { + final Collection availableGroups = new ArrayList<>(getStorage().getAllGroups()); + availableGroups.removeIf(g -> g.containsMember(user)); + + return FrontendUserContent + .builder() + .resolvable(true) + .label(user.getLabel()) + .id(user.getId()) + .groups(AuthorizationHelper.getGroupsOf(user, getStorage())) + .availableGroups(availableGroups) + .roles(user.getRoles().stream().map(this::getFrontendRoleContent).collect(Collectors.toList())) + .availableRoles(getStorage().getAllRoles()) + .permissions(wrapInFEPermission(user.getPermissions())) + .permissionTemplateMap(preparePermissionTemplate()) + .build(); + } public FrontendRoleContent getRoleContent(Role role) { return FrontendRoleContent.builder() + .resolvable(true) .permissions(wrapInFEPermission(role.getPermissions())) .permissionTemplateMap(preparePermissionTemplate()) .users(getUsers(role)) .groups(getGroups(role)) - .owner(role) + .id(role.getId()) + .label(role.getLabel()) .build(); } + private SortedSet wrapInFEPermission(Collection permissions) { + TreeSet fePermissions = new TreeSet<>(); + + for (ConqueryPermission permission : permissions) { + fePermissions.add(FrontendPermission.from(permission)); + } + return fePermissions; + } + private Map, List>> preparePermissionTemplate() { Map, List>> permissionTemplateMap = new HashMap<>(); @@ -144,49 +211,6 @@ private List getGroups(Role role) { .collect(Collectors.toList()); } - private SortedSet wrapInFEPermission(Collection permissions) { - TreeSet fePermissions = new TreeSet<>(); - - for (ConqueryPermission permission : permissions) { - fePermissions.add(FrontendPermission.from(permission)); - } - return fePermissions; - } - - public FrontendUserContent getUserContent(User user) { - final Collection availableGroups = new ArrayList<>(getStorage().getAllGroups()); - availableGroups.removeIf(g -> g.containsMember(user)); - - return FrontendUserContent - .builder() - .owner(user) - .groups(AuthorizationHelper.getGroupsOf(user, getStorage())) - .availableGroups(availableGroups) - .roles(user.getRoles().stream().map(getStorage()::getRole).collect(Collectors.toList())) - .availableRoles(getStorage().getAllRoles()) - .permissions(wrapInFEPermission(user.getPermissions())) - .permissionTemplateMap(preparePermissionTemplate()) - .build(); - } - - - public FrontendGroupContent getGroupContent(Group group) { - - Set membersIds = group.getMembers(); - ArrayList availableMembers = new ArrayList<>(getStorage().getAllUsers()); - availableMembers.removeIf(u -> membersIds.contains(u.getId())); - return FrontendGroupContent - .builder() - .owner(group) - .members(membersIds.stream().map(getStorage()::getUser).collect(Collectors.toList())) - .availableMembers(availableMembers) - .roles(group.getRoles().stream().map(getStorage()::getRole).collect(Collectors.toList())) - .availableRoles(getStorage().getAllRoles()) - .permissions(wrapInFEPermission(group.getPermissions())) - .permissionTemplateMap(preparePermissionTemplate()) - .build(); - } - public TableStatistics getTableStatistics(Table table) { final NamespaceStorage storage = getDatasetRegistry().get(table.getDataset().getId()).getStorage(); List imports = table.findImports(storage).collect(Collectors.toList()); @@ -214,12 +238,8 @@ public TableStatistics getTableStatistics(Table table) { ); } - public ImportStatistics getImportStatistics(Import imp) { - final NamespaceStorage storage = getDatasetRegistry().get(imp.getDataset().getId()).getStorage(); - - final long cBlockSize = calculateCBlocksSizeBytes(imp, storage.getAllConcepts()); - - return new ImportStatistics(imp, cBlockSize); + public DatasetRegistry getDatasetRegistry() { + return adminProcessor.getDatasetRegistry(); } public static long calculateCBlocksSizeBytes(Import imp, Collection> concepts) { @@ -242,4 +262,12 @@ public static long calculateCBlocksSizeBytes(Import imp, Collection implements FrontendRoleOwner { - public final Collection members; + public final Collection members; public final Collection availableMembers; - public final Collection roles; + public final Collection roles; public final Collection availableRoles; } \ No newline at end of file diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendPermissionOwnerContent.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendPermissionOwnerContent.java index 17caeb3242..04365b840e 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendPermissionOwnerContent.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendPermissionOwnerContent.java @@ -6,6 +6,7 @@ import com.bakdata.conquery.models.auth.entities.PermissionOwner; import com.bakdata.conquery.models.auth.permissions.Ability; +import com.bakdata.conquery.models.identifiable.ids.Id; import lombok.Getter; import lombok.experimental.SuperBuilder; import org.apache.commons.lang3.tuple.Pair; @@ -16,7 +17,9 @@ @Getter @SuperBuilder public class FrontendPermissionOwnerContent> { - private OWNER owner; + private String label; + private Id id; + private boolean resolvable; /** * Holds the owned permission objects and its JSON representation. diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendRoleOwner.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendRoleOwner.java index fc63e72483..4fa91f9308 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendRoleOwner.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendRoleOwner.java @@ -10,7 +10,7 @@ */ public interface FrontendRoleOwner { - Collection getRoles(); + Collection getRoles(); Collection getAvailableRoles(); } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendUserContent.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendUserContent.java index 6a3626412d..739550e6f8 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendUserContent.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendUserContent.java @@ -15,7 +15,7 @@ @SuperBuilder public class FrontendUserContent extends FrontendPermissionOwnerContent implements FrontendRoleOwner { - public final Collection roles; + public final Collection roles; public final Collection availableRoles; public final Collection groups; public final Collection availableGroups; diff --git a/backend/src/main/java/com/bakdata/conquery/util/AuthUtil.java b/backend/src/main/java/com/bakdata/conquery/util/AuthUtil.java index a76e0040c2..3a84ffb4f7 100644 --- a/backend/src/main/java/com/bakdata/conquery/util/AuthUtil.java +++ b/backend/src/main/java/com/bakdata/conquery/util/AuthUtil.java @@ -45,7 +45,7 @@ public synchronized void cleanUpUserAndBelongings(User user, MetaStorage storage for (Group group : storage.getAllGroups()) { if (group.containsMember(user)) { - group.removeMember(user); + group.removeMember(user.getId()); group.updateStorage(); log.debug("Removed user '{}' from group '{}'", user.getId(), group.getId()); } diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/group.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/group.html.ftl index 611ea50a5c..ac58161b2c 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/group.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/group.html.ftl @@ -6,8 +6,8 @@
-

Group ${c.owner.label}

- ${c.owner.id} +

Group ${c.label}

+ ${c.id}
@@ -29,10 +29,10 @@
- <@permissionTable.permissionTable ownerId=c.owner.getId() permissions=c.permissions /> + <@permissionTable.permissionTable ownerId=c.id permissions=c.permissions />
- <@permissionCreator.permissionCreator ownerId=c.owner.getId() permissionTemplateMap=c.permissionTemplateMap /> + <@permissionCreator.permissionCreator ownerId=c.id permissionTemplateMap=c.permissionTemplateMap />
@@ -65,9 +65,14 @@ <#list c.members as member> - ${member.label} - ${member.id} - Remove from ${c.owner.label} + <#if member.resolvable> + ${member.label} + ${member.id} + <#else> + Unresolvable Member: ${member.id} + + + Remove from ${c.label} @@ -85,7 +90,7 @@ function addMember() { event.preventDefault(); rest( - '/admin/${ctx.staticUriElem.GROUPS_PATH_ELEMENT}/${c.owner.id}/${ctx.staticUriElem.USERS_PATH_ELEMENT}/'+document.getElementById('member_id').value, + '/admin/${ctx.staticUriElem.GROUPS_PATH_ELEMENT}/${c.id}/${ctx.staticUriElem.USERS_PATH_ELEMENT}/'+document.getElementById('member_id').value, { method: 'post', }) diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/role.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/role.html.ftl index b4cd405414..518ca50ed8 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/role.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/role.html.ftl @@ -6,8 +6,8 @@
-

Role ${c.owner.label}

- ${c.owner.id} +

Role ${c.label}

+ ${c.id}
@@ -32,11 +32,11 @@
- <@permissionTable.permissionTable ownerId=c.owner.getId() permissions=c.permissions /> + <@permissionTable.permissionTable ownerId=c.id permissions=c.permissions />
- <@permissionCreator.permissionCreator ownerId=c.owner.getId() + <@permissionCreator.permissionCreator ownerId=c.id permissionTemplateMap=c.permissionTemplateMap />
diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/groupHandler.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/groupHandler.html.ftl index dbef9a7327..4250fd8b74 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/groupHandler.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/groupHandler.html.ftl @@ -30,7 +30,7 @@ ${group.label} ${group.id} Remove + onclick="removeGroup('${adminPathBase}/${group.id}/${ctx.staticUriElem.USERS_PATH_ELEMENT}/${c.id}')">Remove from Entity @@ -41,7 +41,7 @@ function addGroup() { event.preventDefault(); rest( - '${adminPathBase}/' + document.getElementById('group_id').value + '/${ctx.staticUriElem.USERS_PATH_ELEMENT}/${c.owner.id}', + '${adminPathBase}/' + document.getElementById('group_id').value + '/${ctx.staticUriElem.USERS_PATH_ELEMENT}/${c.id}', { method: 'post', }).then(function () { location.reload() }); diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/roleHandler.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/roleHandler.html.ftl index 640fc0e0c6..6cda1644c3 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/roleHandler.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/roleHandler.html.ftl @@ -26,13 +26,18 @@ <#list c.roles as role> - - ${role.label} - ${role.id} - Remove - from Entity - + + <#if role.resolvable > + ${role.label} + ${role.id} + <#else> + Unresolvable Role: ${role.id} + + + Remove + from Entity + @@ -41,7 +46,7 @@ function addRole() { event.preventDefault(); rest( - '${adminPathBase}/${c.owner.id}/${ctx.staticUriElem.ROLES_PATH_ELEMENT}/' + document.getElementById('role_id').value, + '${adminPathBase}/${c.id}/${ctx.staticUriElem.ROLES_PATH_ELEMENT}/' + document.getElementById('role_id').value, { method: 'post', }).then(function () { location.reload() }); diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/user.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/user.html.ftl index 081c45cfcc..867996e0c7 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/user.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/user.html.ftl @@ -7,8 +7,8 @@
-

User ${c.owner.label}

- ${c.owner.id} +

User ${c.label}

+ ${c.id}
@@ -30,10 +30,10 @@
- <@permissionTable.permissionTable ownerId=c.owner.getId() permissions=c.permissions /> + <@permissionTable.permissionTable ownerId=c.id permissions=c.permissions />
- <@permissionCreator.permissionCreator ownerId=c.owner.getId() permissionTemplateMap=c.permissionTemplateMap /> + <@permissionCreator.permissionCreator ownerId=c.id permissionTemplateMap=c.permissionTemplateMap />
<@groupHandler.groupHandler c=c adminPathBase="/admin/${ctx.staticUriElem.GROUPS_PATH_ELEMENT}" /> diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/GroupHandlingTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/GroupHandlingTest.java index 8c156caa29..5053400c29 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/GroupHandlingTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/GroupHandlingTest.java @@ -37,7 +37,7 @@ public void execute(StandaloneSupport conquery) throws Exception { group1.addMember(user2); assertThat(group1.getMembers()).containsExactlyInAnyOrder(user1.getId(), user2.getId()); - group1.removeMember(user2); + group1.removeMember(user2.getId()); assertThat(group1.getMembers()).containsExactlyInAnyOrder(user1.getId()); diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionGroupHandlingTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionGroupHandlingTest.java index a6d45ee831..3dda1b4fd6 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionGroupHandlingTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionGroupHandlingTest.java @@ -52,7 +52,7 @@ public void execute(StandaloneSupport conquery) throws Exception { assertThat(user1.isPermitted(ExecutionPermission.onInstance(Ability.SHARE, query1))).isTrue(); // remove user from group - group1.removeMember(user1); + group1.removeMember(user1.getId()); assertThat(user1.isPermitted(ExecutionPermission.onInstance(Ability.READ, query1))).isTrue(); assertThat(user1.isPermitted(ExecutionPermission.onInstance(Ability.DELETE, query1))).isTrue(); diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionRoleHandlingTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionRoleHandlingTest.java index 105abea3f7..ea9afb556b 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionRoleHandlingTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionRoleHandlingTest.java @@ -58,7 +58,7 @@ public void execute(StandaloneSupport conquery) throws Exception { // Add permission to mandator, remove mandator from user mandator1.addPermission(dataset.createPermission(Ability.DOWNLOAD.asSet())); - user1.removeRole(mandator1); + user1.removeRole(mandator1.getId()); assertThat(user1.isPermitted(dataset.createPermission(Ability.READ.asSet()))).isTrue(); assertThat(user1.isPermitted(dataset.createPermission(Ability.DOWNLOAD.asSet()))).isFalse(); diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/RestartTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/RestartTest.java index 9960d8cc94..ba3de818a8 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/RestartTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/RestartTest.java @@ -130,8 +130,8 @@ public void execute(String name, TestConquery testConquery) throws Exception { // Delete entities //TODO use API - adminProcessor.deleteUser(userToDelete); - adminProcessor.deleteRole(roleToDelete); + adminProcessor.deleteUser(userToDelete.getId()); + adminProcessor.deleteRole(roleToDelete.getId()); adminProcessor.deleteGroup(groupToDelete); } diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingOnGroupTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingOnGroupTest.java index 205e39d5de..787468bcaf 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingOnGroupTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingOnGroupTest.java @@ -48,7 +48,7 @@ public void execute(StandaloneSupport conquery) throws Exception { //// Remove role from group - group1.removeRole(role); + group1.removeRole(role.getId()); assertThat(group1.getRoles()).isEmpty(); assertThat(user1.isPermitted(new DatasetPermission().instancePermission(Ability.READ, new DatasetId("testDataset")))).isFalse(); diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingTest.java index ba797e04be..40dd3470f6 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingTest.java @@ -46,10 +46,10 @@ public void execute(StandaloneSupport conquery) throws Exception { //// REMOVING - user1.removeRole(mandator2); + user1.removeRole(mandator2.getId()); assertThat(user1.getRoles()).containsExactlyInAnyOrder(mandator1.getId()); - user1.removeRole(mandator1); + user1.removeRole(mandator1.getId()); assertThat(user1.getRoles()).isEmpty(); } diff --git a/cypress/e2e/backend-admin-ui/test_3_smoketest.cy.js b/cypress/e2e/backend-admin-ui/test_3_smoketest.cy.js index a44dd12331..1d548877e4 100644 --- a/cypress/e2e/backend-admin-ui/test_3_smoketest.cy.js +++ b/cypress/e2e/backend-admin-ui/test_3_smoketest.cy.js @@ -1,25 +1,85 @@ /// -import { visitAdminUI } from "../../integration-helpers/visitAdminUI"; +import { adminBaseUrl, checkFreemarkerError, visitAdminUI } from "../../integration-helpers/visitAdminUI"; -context("Simplest Smoke Tests", () => { +context("Simple UI Render Smoke Tests", () => { - describe("UI renders", () => { - - it("Query Overview", () => { - - visitAdminUI("queries"); + describe("Query Overview renders", () => { - // Disable updates so the test ends - cy.get('#updateCheckBox').click() - cy.root().should('not.contain.text', 'FreeMarker template error') - }); + it("Visit Query Overview", () => { + + visitAdminUI("queries"); + + // Disable updates so the test ends eventually + cy.get('#updateCheckBox').click() + checkFreemarkerError() + }); + }); + + describe("Query Jobs renders", () => { + + it("Visit Jobs Overview", () => { + + visitAdminUI("jobs"); - - it("Jobs Overview", () => { - - visitAdminUI("jobs"); + checkFreemarkerError() + }); + }); + + describe("Groups renders", () => { - cy.root().should('not.contain.text', 'FreeMarker template error') + it("Post faulty group (member id not resolvable)", () => { + + cy.request({ + method: 'POST', + url: adminBaseUrl + "/admin/groups/", + body: [{ + name: "faulty_group", + label: "Faulty Group", + members: [ + "user.unresolvable" + ], + roles: [ + "role.unresolvable" + ] + }] }); }); + + + it("Visit Groups", () => { + + visitAdminUI("groups"); + + checkFreemarkerError(); + }); + + it("Visit Faulty Group", () => { + visitAdminUI("groups/group.faulty_group"); + + checkFreemarkerError(); + + cy.get('#member > .table-responsive').contains('Unresolvable Member') + + cy.get('#roles > .table-responsive').contains('Unresolvable Role') + }) + + + + it("Delete faulty member and role", () => { + visitAdminUI("groups/group.faulty_group"); + + cy.get('#members-tab').click() + cy.get('#member > .table-responsive > .table > tbody > tr > :nth-child(2) > a').click() + + cy.get('#roles-tab').click() + cy.get('#roles > .table-responsive > .table > tbody > tr > :nth-child(2) > a').click() + + checkFreemarkerError(); + + cy.get('#member > .table-responsive').should('not.contain.text', 'Unresolvable Member') + + cy.get('#roles > .table-responsive').should('not.contain.text', 'Unresolvable Role') + }) + }) + }) \ No newline at end of file diff --git a/cypress/integration-helpers/visitAdminUI.js b/cypress/integration-helpers/visitAdminUI.js index bf9911cf9c..ba45b81af5 100644 --- a/cypress/integration-helpers/visitAdminUI.js +++ b/cypress/integration-helpers/visitAdminUI.js @@ -1,4 +1,10 @@ +export const adminBaseUrl = 'http://localhost:8081' + export const visitAdminUI = (page = '') => { - cy.visit('http://localhost:8081/admin-ui/' + page); + cy.visit(adminBaseUrl + '/admin-ui/' + page); }; + +export const checkFreemarkerError = () => { + cy.root().should('not.contain.text', 'FreeMarker template error') +} \ No newline at end of file From c636dd8946fcb8256a0dbf060ce81006ab666ef2 Mon Sep 17 00:00:00 2001 From: Max Thonagel <12283268+thoniTUB@users.noreply.github.com> Date: Mon, 28 Oct 2024 13:44:45 +0100 Subject: [PATCH 13/13] does not issue messages on mina process thread, adds logging --- .../conquery/io/mina/NetworkSession.java | 12 +- .../mode/cluster/ClusterConnectionShard.java | 169 +++++++++--------- .../command/DistributedCommandsTest.java | 39 ++-- 3 files changed, 112 insertions(+), 108 deletions(-) diff --git a/backend/src/main/java/com/bakdata/conquery/io/mina/NetworkSession.java b/backend/src/main/java/com/bakdata/conquery/io/mina/NetworkSession.java index c82b8f5dfa..52c61a22fa 100644 --- a/backend/src/main/java/com/bakdata/conquery/io/mina/NetworkSession.java +++ b/backend/src/main/java/com/bakdata/conquery/io/mina/NetworkSession.java @@ -11,6 +11,7 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.apache.mina.core.future.DefaultWriteFuture; import org.apache.mina.core.future.WriteFuture; import org.apache.mina.core.session.IoSession; import org.jetbrains.annotations.NotNull; @@ -40,11 +41,16 @@ public WriteFuture send(final NetworkMessage message) { } } catch (InterruptedException e) { - log.error("Unexpected interruption", e); - return send(message); + log.error("Unexpected interruption, while trying to queue: {}", message, e); + return DefaultWriteFuture.newNotWrittenFuture(session, e); } WriteFuture future = session.write(message); - future.addListener(f -> queuedMessages.remove(message)); + future.addListener(f -> { + if(f instanceof WriteFuture writeFuture && ! writeFuture.isWritten()) { + log.error("Could not write message: {}", message, writeFuture.getException()); + } + queuedMessages.remove(message); + }); return future; } diff --git a/backend/src/main/java/com/bakdata/conquery/mode/cluster/ClusterConnectionShard.java b/backend/src/main/java/com/bakdata/conquery/mode/cluster/ClusterConnectionShard.java index 54cfe44459..0511b7048b 100644 --- a/backend/src/main/java/com/bakdata/conquery/mode/cluster/ClusterConnectionShard.java +++ b/backend/src/main/java/com/bakdata/conquery/mode/cluster/ClusterConnectionShard.java @@ -68,22 +68,39 @@ public void sessionCreated(IoSession session) { public void sessionOpened(IoSession session) { NetworkSession networkSession = new NetworkSession(session); - context = new NetworkMessageContext.ShardNodeNetworkContext(networkSession, workers, config, environment.getValidator()); - log.info("Connected to ManagerNode @ `{}`", session.getRemoteAddress()); + // Schedule ShardNode and Worker registration, so we don't block this thread which does the actual sending + scheduler.schedule(() -> { + context = new NetworkMessageContext.ShardNodeNetworkContext(networkSession, workers, config, environment.getValidator()); + log.info("Connected to ManagerNode @ `{}`", session.getRemoteAddress()); + + // Authenticate with ManagerNode + context.send(new AddShardNode()); + + for (Worker w : workers.getWorkers().values()) { + w.setSession(new NetworkSession(session)); + WorkerInformation info = w.getInfo(); + log.info("Sending worker identity '{}'", info.getName()); + networkSession.send(new RegisterWorker(info)); + } + }, 0, TimeUnit.SECONDS); - // Authenticate with ManagerNode - context.send(new AddShardNode()); - for (Worker w : workers.getWorkers().values()) { - w.setSession(new NetworkSession(session)); - WorkerInformation info = w.getInfo(); - log.info("Sending worker identity '{}'", info.getName()); - networkSession.send(new RegisterWorker(info)); - } scheduleIdleLogger(scheduler, session, config.getCluster().getIdleTimeOut()); } + private static void scheduleIdleLogger(ScheduledExecutorService scheduler, IoSession session, Duration timeout) { + scheduler.scheduleAtFixedRate( + () -> { + final Duration elapsed = Duration.milliseconds(System.currentTimeMillis() - session.getLastIoTime()); + if (elapsed.compareTo(timeout) > 0) { + log.trace("No message sent or received since {}", elapsed); + } + }, + timeout.toSeconds(), timeout.toSeconds() / 2, TimeUnit.SECONDS + ); + } + @Override public void sessionClosed(IoSession session) { log.info("Disconnected from ManagerNode."); @@ -96,56 +113,6 @@ public void sessionClosed(IoSession session) { } } - @Override - public void sessionIdle(IoSession session, IdleStatus status) { - log.trace("Session idle {}.", status); - } - - @Override - public void exceptionCaught(IoSession session, Throwable cause) { - log.error("Exception caught", cause); - } - - - @Override - public void messageReceived(IoSession session, Object message) { - if (!(message instanceof MessageToShardNode)) { - log.error("Unknown message type {} in {}", message.getClass(), message); - return; - } - - log.trace("{} received {} from {}", environment.getName(), message.getClass().getSimpleName(), session.getRemoteAddress()); - ReactingJob job = new ReactingJob<>((MessageToShardNode) message, context); - - if (message instanceof SlowMessage slowMessage) { - slowMessage.setProgressReporter(job.getProgressReporter()); - jobManager.addSlowJob(job); - } - else { - jobManager.addFastJob(job); - } - } - - - @Override - public void messageSent(IoSession session, Object message) { - log.trace("Message sent: {}", message); - } - - - @Override - public void inputClosed(IoSession session) { - log.info("Input closed."); - session.closeNow(); - scheduler.schedule(this::disconnectFromCluster, 0, TimeUnit.SECONDS); - } - - - @Override - public void event(IoSession session, FilterEvent event) throws Exception { - log.trace("Event handled: {}", event); - } - private void connectToCluster() { InetSocketAddress address = new InetSocketAddress( config.getCluster().getManagerURL().getHostAddress(), @@ -184,7 +151,6 @@ private void connectToCluster() { } } - private void disconnectFromCluster() { if (future != null) { future.cancel(); @@ -201,7 +167,6 @@ private void disconnectFromCluster() { } } - @NotNull private NioSocketConnector getClusterConnector(IdResolveContext workers) { ObjectMapper om = internalMapperFactory.createShardCommunicationMapper(); @@ -216,19 +181,65 @@ private NioSocketConnector getClusterConnector(IdResolveContext workers) { return connector; } + @Override + public void sessionIdle(IoSession session, IdleStatus status) { + log.trace("Session idle {}.", status); + } - private static void scheduleIdleLogger(ScheduledExecutorService scheduler, IoSession session, Duration timeout) { - scheduler.scheduleAtFixedRate( - () -> { - final Duration elapsed = Duration.milliseconds(System.currentTimeMillis() - session.getLastIoTime()); - if (elapsed.compareTo(timeout) > 0) { - log.trace("No message sent or received since {}", elapsed); - } - }, - timeout.toSeconds(), timeout.toSeconds() / 2, TimeUnit.SECONDS - ); + @Override + public void exceptionCaught(IoSession session, Throwable cause) { + log.error("Exception caught", cause); } + @Override + public void messageReceived(IoSession session, Object message) { + if (!(message instanceof MessageToShardNode)) { + log.error("Unknown message type {} in {}", message.getClass(), message); + return; + } + + log.trace("{} received {} from {}", environment.getName(), message.getClass().getSimpleName(), session.getRemoteAddress()); + ReactingJob job = new ReactingJob<>((MessageToShardNode) message, context); + + if (message instanceof SlowMessage slowMessage) { + slowMessage.setProgressReporter(job.getProgressReporter()); + jobManager.addSlowJob(job); + } + else { + jobManager.addFastJob(job); + } + } + + @Override + public void messageSent(IoSession session, Object message) { + log.trace("Message sent: {}", message); + } + + @Override + public void inputClosed(IoSession session) { + log.info("Input closed."); + session.closeNow(); + scheduler.schedule(this::disconnectFromCluster, 0, TimeUnit.SECONDS); + } + + @Override + public void event(IoSession session, FilterEvent event) throws Exception { + log.trace("Event handled: {}", event); + } + + @Override + public void start() throws Exception { + + + jobManager = new JobManager(environment.getName(), config.isFailOnError()); + + scheduler = environment.lifecycle().scheduledExecutorService("cluster-connection-shard").build(); + // Connect async as the manager might not be up jet or is started by a test in succession + scheduler.schedule(this::connectToCluster, 0, TimeUnit.MINUTES); + + scheduler.scheduleAtFixedRate(this::reportJobManagerStatus, 30, 1, TimeUnit.SECONDS); + + } private void reportJobManagerStatus() { if (context == null || !context.isConnected()) { @@ -257,20 +268,6 @@ private void reportJobManagerStatus() { } } - @Override - public void start() throws Exception { - - - jobManager = new JobManager(environment.getName(), config.isFailOnError()); - - scheduler = environment.lifecycle().scheduledExecutorService("cluster-connection-shard").build(); - // Connect async as the manager might not be up jet or is started by a test in succession - scheduler.schedule(this::connectToCluster, 0, TimeUnit.MINUTES); - - scheduler.scheduleAtFixedRate(this::reportJobManagerStatus, 30, 1, TimeUnit.SECONDS); - - } - public boolean isBusy() { return jobManager.isSlowWorkerBusy(); } diff --git a/backend/src/test/java/com/bakdata/conquery/command/DistributedCommandsTest.java b/backend/src/test/java/com/bakdata/conquery/command/DistributedCommandsTest.java index 5fbd90e3c2..510561c2aa 100644 --- a/backend/src/test/java/com/bakdata/conquery/command/DistributedCommandsTest.java +++ b/backend/src/test/java/com/bakdata/conquery/command/DistributedCommandsTest.java @@ -1,8 +1,10 @@ package com.bakdata.conquery.command; import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; import java.util.Map; +import java.util.concurrent.TimeUnit; import jakarta.ws.rs.client.Client; import jakarta.ws.rs.core.GenericType; import jakarta.ws.rs.core.Response; @@ -42,18 +44,16 @@ public class DistributedCommandsTest { this.getCluster().setConnectRetryTimeout(CONNECT_RETRY_TIMEOUT); this.setStorage(new NonPersistentStoreFactory()); }}; - - private static final DropwizardAppExtension MANAGER = new DropwizardAppExtension<>( - Conquery.class, - CONQUERY_CONFIG_MANAGER, - ServerCommand::new - ); - private static final DropwizardAppExtension SHARD = new DropwizardAppExtension<>( Conquery.class, CONQUERY_CONFIG_SHARD, application -> new ShardCommand() ); + private static final DropwizardAppExtension MANAGER = new DropwizardAppExtension<>( + Conquery.class, + CONQUERY_CONFIG_MANAGER, + ServerCommand::new + ); @Test @Order(0) @@ -83,27 +83,28 @@ void checkHttpUpManager() { @Test @Order(1) - void clusterEstablished() throws InterruptedException { + void clusterEstablished() { Client client = MANAGER.client(); // Wait for Shard to be connected - // May use https://github.com/awaitility/awaitility here in the future - Thread.sleep(2 * CONNECT_RETRY_TIMEOUT.toMilliseconds()); + await().atMost(5, TimeUnit.SECONDS).pollInterval(1, TimeUnit.SECONDS).untilAsserted(() -> { + Response response = client.target( + String.format("http://localhost:%d/healthcheck", MANAGER.getAdminPort())) + .request() + .get(); - Response response = client.target( - String.format("http://localhost:%d/healthcheck", MANAGER.getAdminPort())) - .request() - .get(); - assertThat(response.getStatus()).isEqualTo(200); + assertThat(response.getStatus()).isEqualTo(200); + + Map healthCheck = response.readEntity(new GenericType<>() { + }); - Map healthCheck = response.readEntity(new GenericType<>() { + assertThat(healthCheck).containsKey("cluster"); + assertThat(healthCheck.get("cluster").healthy).isTrue(); + assertThat(healthCheck.get("cluster").getMessage()).isEqualTo(String.format(ClusterHealthCheck.HEALTHY_MESSAGE_FMT, 1)); }); - assertThat(healthCheck).containsKey("cluster"); - assertThat(healthCheck.get("cluster").healthy).isTrue(); - assertThat(healthCheck.get("cluster").getMessage()).isEqualTo(String.format(ClusterHealthCheck.HEALTHY_MESSAGE_FMT, 1)); } @Data