From 4b455e85c6a4e1c490376a094247c6d452cfcd62 Mon Sep 17 00:00:00 2001 From: anton Date: Tue, 6 Aug 2024 13:42:02 +0200 Subject: [PATCH 01/11] FAIRSPC-108: initial commit with updated API for views meta info --- .../saturn/services/views/ViewConfigDto.java | 11 +++++++++++ .../io/fairspace/saturn/services/views/ViewDTO.java | 1 + .../fairspace/saturn/services/views/ViewService.java | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java new file mode 100644 index 000000000..1ad73726d --- /dev/null +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java @@ -0,0 +1,11 @@ +package io.fairspace.saturn.services.views; + +import lombok.Value; + +@Value +public class ViewConfigDto { + + long maxDisplayCount; + +} + diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java index 2efe598d5..5d8d900c8 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java @@ -9,4 +9,5 @@ public class ViewDTO { String name; String title; List columns; + ViewConfigDto config; } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java index 6444b347e..075e7f19a 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java @@ -189,7 +189,7 @@ protected List fetchViews() { columns.add(new ColumnDTO(joinView.name + "_" + c.name, c.title, c.type, j.displayIndex)); } } - return new ViewDTO(v.name, v.title, columns); + return new ViewDTO(v.name, v.title, columns, new ViewConfigDto(1000000L)); }) .collect(toList()); } From 27753b1ec4b9744406b4ed08cdf3f5c45d31e64f Mon Sep 17 00:00:00 2001 From: ewelinagr Date: Wed, 7 Aug 2024 11:35:14 +0200 Subject: [PATCH 02/11] Add display count limit handling to the pagination component. --- .../components/TablePaginationActions.js | 26 +++++---- .../src/metadata/views/MetadataViewAPI.js | 7 ++- .../views/MetadataViewTableContainer.js | 55 +++++++++++++++---- .../src/metadata/views/MetadataViewTabs.js | 1 + .../mercury/src/metadata/views/UseViewData.js | 24 +++++--- 5 files changed, 83 insertions(+), 30 deletions(-) diff --git a/projects/mercury/src/common/components/TablePaginationActions.js b/projects/mercury/src/common/components/TablePaginationActions.js index 4abe1ca0b..cccb60567 100644 --- a/projects/mercury/src/common/components/TablePaginationActions.js +++ b/projects/mercury/src/common/components/TablePaginationActions.js @@ -2,7 +2,7 @@ import React from 'react'; import IconButton from '@mui/material/IconButton'; import {KeyboardArrowLeft, KeyboardArrowRight, LastPage, FirstPage} from '@mui/icons-material'; import makeStyles from '@mui/styles/makeStyles'; -import {Typography} from '@mui/material'; +import {Tooltip, Typography} from '@mui/material'; const useStyles = makeStyles(theme => ({ root: { @@ -20,7 +20,9 @@ export type TablePaginationActionsProperties = { const TablePaginationActions = (props: TablePaginationActionsProperties) => { const classes = useStyles(); - const {count, page, rowsPerPage, onPageChange} = props; + const {count, page, rowsPerPage, onPageChange, countDisplayLimit} = props; + + const countDisplayLimitReached = countDisplayLimit !== undefined && countDisplayLimit === count; const handleFirstPageButtonClick = event => { onPageChange(event, 0); @@ -62,14 +64,18 @@ const TablePaginationActions = (props: TablePaginationActionsProperties) => { > - = Math.ceil(count / rowsPerPage) - 1} - aria-label="last page" - size="medium" - > - - + + + = Math.ceil(count / rowsPerPage) - 1} + aria-label="last page" + size="medium" + > + + + + ); }; diff --git a/projects/mercury/src/metadata/views/MetadataViewAPI.js b/projects/mercury/src/metadata/views/MetadataViewAPI.js index 778ec0142..24396f49d 100644 --- a/projects/mercury/src/metadata/views/MetadataViewAPI.js +++ b/projects/mercury/src/metadata/views/MetadataViewAPI.js @@ -40,10 +40,15 @@ export type MetadataViewColumn = { type: ValueType }; +export type MetadataViewsConfig = { + maxDisplayCount: number +}; + export type MetadataViewOptions = { name: string, title: string, - columns: MetadataViewColumn[] + columns: MetadataViewColumn[], + config: MetadataViewsConfig }; export type MetadataViews = { diff --git a/projects/mercury/src/metadata/views/MetadataViewTableContainer.js b/projects/mercury/src/metadata/views/MetadataViewTableContainer.js index 8430ae1df..b55cf1806 100644 --- a/projects/mercury/src/metadata/views/MetadataViewTableContainer.js +++ b/projects/mercury/src/metadata/views/MetadataViewTableContainer.js @@ -11,7 +11,7 @@ import { } from '@mui/material'; import withStyles from '@mui/styles/withStyles'; import {useHistory} from 'react-router-dom'; -import {Addchart, ViewColumn, Check} from '@mui/icons-material'; +import {Addchart, ViewColumn, Check, ErrorOutline} from '@mui/icons-material'; import Checkbox from '@mui/material/Checkbox'; import FormControl from '@mui/material/FormControl'; import Popover from '@mui/material/Popover'; @@ -46,6 +46,7 @@ type MetadataViewTableContainerProperties = { setTextFiltersObject: () => {}, toggleRow: () => {}, view: string, + viewCountDisplayLimit: number, collections: Collection[], locationContext: string, selected: MetadataViewEntityWithLinkedFiles, @@ -68,6 +69,10 @@ const styles = theme => ({ fontSize: 12, padding: 2 }, + countErrorIcon: { + fontSize: 16, + marginLeft: 3 + }, tableContents: { minHeight: '200px', maxHeight: 'calc(100vh - 250px)', @@ -87,11 +92,15 @@ const styles = theme => ({ float: 'right', maxWidth: 50 }, + tablePaginationLabel: { + fontWeight: 'bold', + alignContent: 'center' + }, viewColumnsFormControl: { padding: 10 }, messageBox: { - padding: 5 + padding: 0 } }); @@ -99,7 +108,8 @@ const LOCAL_STORAGE_METADATA_TABLE_ROWS_NUM_KEY = 'FAIRSPACE_METADATA_TABLE_ROWS const SESSION_STORAGE_VISIBLE_COLUMNS_KEY_PREFIX = 'FAIRSPACE_METADATA_VISIBLE_COLUMNS'; export const MetadataViewTableContainer = (props: MetadataViewTableContainerProperties) => { - const {view, filters, columns, idColumn, hasInactiveFilters, locationContext, classes} = props; + const {view, viewCountDisplayLimit, filters, columns, idColumn, hasInactiveFilters, locationContext, classes} = + props; const {textFiltersObject, setTextFiltersObject} = props; const {isFeatureEnabled} = useContext(FeaturesContext); @@ -119,7 +129,7 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp const columnSelectorOpen = Boolean(anchorEl); const history = useHistory(); - const {data, count, error, loading, loadingCount, refreshDataOnly} = useViewData( + const {data, count, error, countError, loading, loadingCount, refreshDataOnly} = useViewData( view, filters, textFiltersObject, @@ -243,13 +253,21 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp const renderMessages = () => (
- {count.timeout && } + {count.timeout && ( + + )} {hasInactiveFilters && ( )}
@@ -301,14 +319,27 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp ); - const labelDisplayedRows = ({from, to, count: totalCount, countIsLoading}) => ( + const getTotalCountString = (totalCount: number, to: number) => { + if (totalCount === undefined) return '...'; + if (totalCount === -1) return `more than ${to}`; + if (viewCountDisplayLimit !== undefined && totalCount === viewCountDisplayLimit) + return `more than ${viewCountDisplayLimit - 1}`; + return totalCount; + }; + + const renderTablePaginationLabel = ({from, to, countIsLoading, countHasError}) => ( {from}-{to} of{' '} - - {totalCount !== undefined && totalCount !== -1 ? totalCount.toLocaleString() : 'more than ' + to} + + {getTotalCountString(count?.count, to)} {countIsLoading && } + {(countHasError || count?.timeout) && ( + + + + )} ); @@ -318,7 +349,7 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp return ; } - if (count.count === 0 && !data.timeout && !count.timeout) { + if (count.count === 0 && !data.timeout) { return ; } if (data && data.timeout) { @@ -422,8 +453,10 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp onPageChange={handleChangePage} onRowsPerPageChange={handleChangeRowsPerPage} className={classes.tableFooter} - labelDisplayedRows={d => labelDisplayedRows({...d, countIsLoading: loadingCount})} - ActionsComponent={TablePaginationActions} + labelDisplayedRows={d => + renderTablePaginationLabel({...d, countIsLoading: loadingCount, countHasError: countError}) + } + ActionsComponent={p => TablePaginationActions({...p, countDisplayLimit: viewCountDisplayLimit})} /> diff --git a/projects/mercury/src/metadata/views/MetadataViewTabs.js b/projects/mercury/src/metadata/views/MetadataViewTabs.js index 9fa9dd7a6..877a48fb7 100644 --- a/projects/mercury/src/metadata/views/MetadataViewTabs.js +++ b/projects/mercury/src/metadata/views/MetadataViewTabs.js @@ -92,6 +92,7 @@ export const MetadataViewTabs = (props: MetadataViewTabsProperties) => { columns={appendCustomColumns(view)} idColumn={idColumn} view={view.name} + viewCountDisplayLimit={view.config?.maxDisplayCount} filters={filters} locationContext={locationContext} selected={selected} diff --git a/projects/mercury/src/metadata/views/UseViewData.js b/projects/mercury/src/metadata/views/UseViewData.js index a158a1a2c..72006b1aa 100644 --- a/projects/mercury/src/metadata/views/UseViewData.js +++ b/projects/mercury/src/metadata/views/UseViewData.js @@ -36,6 +36,7 @@ const useViewData = ( const [loading, setLoading] = useState(true); const [loadingCount, setLoadingCount] = useState(true); const [error, setError] = useState(); + const [countError, setCountError] = useState(); const [countRequestCancelToken, setCountRequestCancelToken] = useState(); const [viewDataRequestCancelToken, setViewDataRequestCancelToken] = useState(); @@ -63,15 +64,21 @@ const useViewData = ( } const token = axios.CancelToken.source(); setCountRequestCancelToken(token); - metadataViewAPI.getCount(token, view, allFilters).then(res => { - if (res) { - if (res.count == null) { - res.count = -1; + metadataViewAPI + .getCount(token, view, allFilters) + .then(res => { + if (res) { + if (res.count == null) { + res.count = -1; + } + setCount(res); } - setCount(res); - setLoadingCount(false); - } - }); + }) + .catch(e => { + setCountError(e || true); + console.error(e || new Error('Error while fetching counts.')); + }) + .finally(() => setLoadingCount(false)); }; const fetchViewData = (newPage: number, newRowsPerPage: number): Promise => { @@ -133,6 +140,7 @@ const useViewData = ( loading, loadingCount, error, + countError, refreshDataOnly }; }; From 57fca804aeb5ed34d4b505c4114c22875a9f3de3 Mon Sep 17 00:00:00 2001 From: anton Date: Wed, 7 Aug 2024 22:57:19 +0200 Subject: [PATCH 03/11] FAIRSPC-108: added implementation for Postgres --- .../io/fairspace/saturn/config/Services.java | 2 +- .../fairspace/saturn/config/ViewsConfig.java | 19 +++++ .../services/views/JdbcQueryService.java | 25 ++++--- .../saturn/services/views/ViewConfigDto.java | 5 ++ .../saturn/services/views/ViewService.java | 30 ++++---- .../services/views/ViewStoreReader.java | 69 ++++++++++++------- .../services/views/JdbcQueryServiceTest.java | 50 +++++++------- .../services/views/ViewServiceTest.java | 44 ++++++------ 8 files changed, 144 insertions(+), 100 deletions(-) diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/config/Services.java b/projects/saturn/src/main/java/io/fairspace/saturn/config/Services.java index 88ec0a271..6a1054e56 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/config/Services.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/config/Services.java @@ -113,7 +113,7 @@ public Services( queryService = viewStoreClientFactory == null ? new SparqlQueryService(config.search, viewsConfig, filteredDataset) - : new JdbcQueryService(config.search, viewStoreClientFactory, transactions, davFactory.root); + : new JdbcQueryService(config.search, viewsConfig, viewStoreClientFactory, transactions, davFactory.root); viewService = new ViewService(config, viewsConfig, filteredDataset, viewStoreClientFactory, metadataPermissions); diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java b/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java index ab9e058b1..f75566942 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java @@ -1,6 +1,8 @@ package io.fairspace.saturn.config; import java.util.*; +import java.util.function.Function; +import java.util.stream.Collectors; import com.fasterxml.jackson.annotation.*; import com.fasterxml.jackson.databind.*; @@ -8,11 +10,22 @@ import jakarta.validation.constraints.*; public class ViewsConfig { + + private Map viewConfig; + public static final ObjectMapper MAPPER = new ObjectMapper(new YAMLFactory()); @JsonSetter(nulls = Nulls.AS_EMPTY) public List views = new ArrayList<>(); + public View getViewConfig(String viewName) { + if (viewConfig == null) { + viewConfig = views.stream() + .collect(Collectors.toMap(view -> view.name, Function.identity())); + } + return viewConfig.get(viewName); + } + public enum ColumnType { Text, Set, @@ -68,6 +81,12 @@ public static class View { * The name of the items that appear as rows. */ public String itemName; + /** + * The max count value to be requested for a view total count. + * If total count is greater than this max value, the total count value will look like 'more than 1000' on FE. + * This is to prevent performance issues when the total count is too large. + */ + public long maxDisplayCount; /** * The URLs of the types of entities that should be indexed in this view. */ diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/JdbcQueryService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/JdbcQueryService.java index dabcd8dac..01a50d12a 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/JdbcQueryService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/JdbcQueryService.java @@ -1,5 +1,15 @@ package io.fairspace.saturn.services.views; +import io.fairspace.saturn.config.Config; +import io.fairspace.saturn.config.ViewsConfig; +import io.fairspace.saturn.rdf.transactions.Transactions; +import io.fairspace.saturn.rdf.transactions.TxnIndexDatasetGraph; +import io.fairspace.saturn.services.search.FileSearchRequest; +import io.fairspace.saturn.services.search.SearchResultDTO; +import io.milton.resource.CollectionResource; +import lombok.SneakyThrows; +import lombok.extern.log4j.Log4j2; + import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.sql.SQLException; @@ -11,16 +21,6 @@ import java.util.Set; import java.util.stream.Collectors; -import io.milton.resource.CollectionResource; -import lombok.SneakyThrows; -import lombok.extern.log4j.Log4j2; - -import io.fairspace.saturn.config.Config; -import io.fairspace.saturn.rdf.transactions.Transactions; -import io.fairspace.saturn.rdf.transactions.TxnIndexDatasetGraph; -import io.fairspace.saturn.services.search.FileSearchRequest; -import io.fairspace.saturn.services.search.SearchResultDTO; - import static java.lang.Integer.min; /** @@ -35,10 +35,12 @@ public class JdbcQueryService implements QueryService { private final Transactions transactions; private final CollectionResource rootSubject; private final Config.Search searchConfig; + private final ViewsConfig viewsConfig; private final ViewStoreClientFactory viewStoreClientFactory; public JdbcQueryService( Config.Search searchConfig, + ViewsConfig viewsConfig, ViewStoreClientFactory viewStoreClientFactory, Transactions transactions, CollectionResource rootSubject) { @@ -46,6 +48,7 @@ public JdbcQueryService( this.viewStoreClientFactory = viewStoreClientFactory; this.transactions = transactions; this.rootSubject = rootSubject; + this.viewsConfig = viewsConfig; } public String getCollectionName(String uri) { @@ -55,7 +58,7 @@ public String getCollectionName(String uri) { } ViewStoreReader getViewStoreReader() throws SQLException { - return new ViewStoreReader(searchConfig, viewStoreClientFactory); + return new ViewStoreReader(searchConfig, viewsConfig, viewStoreClientFactory); } @SneakyThrows diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java index 1ad73726d..2f56728ce 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java @@ -1,5 +1,6 @@ package io.fairspace.saturn.services.views; +import io.fairspace.saturn.config.ViewsConfig; import lombok.Value; @Value @@ -7,5 +8,9 @@ public class ViewConfigDto { long maxDisplayCount; + public static ViewConfigDto from(ViewsConfig.View config) { + return new ViewConfigDto(config.maxDisplayCount); + } + } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java index 075e7f19a..64554d6dd 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java @@ -1,15 +1,14 @@ package io.fairspace.saturn.services.views; -import java.util.ArrayList; -import java.util.EnumSet; -import java.util.List; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.function.Supplier; - import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import io.fairspace.saturn.config.Config; +import io.fairspace.saturn.config.ViewsConfig; +import io.fairspace.saturn.rdf.search.FilteredDatasetGraph; +import io.fairspace.saturn.services.AccessDeniedException; +import io.fairspace.saturn.services.metadata.MetadataPermissions; +import io.fairspace.saturn.vocabulary.FS; import lombok.SneakyThrows; import lombok.extern.log4j.Log4j2; import org.apache.jena.datatypes.xsd.XSDDateTime; @@ -21,16 +20,15 @@ import org.apache.jena.query.QuerySolutionMap; import org.apache.jena.rdf.model.Literal; -import io.fairspace.saturn.config.Config; -import io.fairspace.saturn.config.ViewsConfig; -import io.fairspace.saturn.rdf.search.FilteredDatasetGraph; -import io.fairspace.saturn.services.AccessDeniedException; -import io.fairspace.saturn.services.metadata.MetadataPermissions; -import io.fairspace.saturn.vocabulary.FS; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; import static io.fairspace.saturn.config.ViewsConfig.ColumnType; import static io.fairspace.saturn.config.ViewsConfig.View; - import static java.time.Instant.ofEpochMilli; import static java.util.Optional.ofNullable; import static java.util.stream.Collectors.toList; @@ -189,7 +187,7 @@ protected List fetchViews() { columns.add(new ColumnDTO(joinView.name + "_" + c.name, c.title, c.type, j.displayIndex)); } } - return new ViewDTO(v.name, v.title, columns, new ViewConfigDto(1000000L)); + return new ViewDTO(v.name, v.title, columns, ViewConfigDto.from(viewsConfig.getViewConfig(v.name))); }) .collect(toList()); } @@ -285,7 +283,7 @@ private Range getColumnRange(View view, View.Column column) { if (!EnumSet.of(ColumnType.Date, ColumnType.Number).contains(column.type)) { return null; } - try (var reader = new ViewStoreReader(searchConfig, viewStoreClientFactory)) { + try (var reader = new ViewStoreReader(searchConfig, viewsConfig, viewStoreClientFactory)) { return reader.aggregate(view.name, column.name); } } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java index 284ed0c88..31391e2fa 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java @@ -1,5 +1,17 @@ package io.fairspace.saturn.services.views; +import com.google.common.collect.Sets; +import io.fairspace.saturn.config.Config; +import io.fairspace.saturn.config.ViewsConfig; +import io.fairspace.saturn.config.ViewsConfig.ColumnType; +import io.fairspace.saturn.config.ViewsConfig.View; +import io.fairspace.saturn.services.search.FileSearchRequest; +import io.fairspace.saturn.services.search.SearchResultDTO; +import io.fairspace.saturn.vocabulary.FS; +import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; + import java.sql.Array; import java.sql.Connection; import java.sql.PreparedStatement; @@ -21,18 +33,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import com.google.common.collect.Sets; -import lombok.SneakyThrows; -import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang3.StringUtils; - -import io.fairspace.saturn.config.Config; -import io.fairspace.saturn.config.ViewsConfig.ColumnType; -import io.fairspace.saturn.config.ViewsConfig.View; -import io.fairspace.saturn.services.search.FileSearchRequest; -import io.fairspace.saturn.services.search.SearchResultDTO; -import io.fairspace.saturn.vocabulary.FS; - import static io.fairspace.saturn.config.ViewsConfig.ColumnType.Date; import static io.fairspace.saturn.services.views.Table.idColumn; @@ -46,14 +46,17 @@ */ @Slf4j public class ViewStoreReader implements AutoCloseable { + final Config.Search searchConfig; + final ViewsConfig viewsConfig; final ViewStoreClient.ViewStoreConfiguration configuration; final Connection connection; // TODO: in whole class, use StringBuilder instead of String concats - public ViewStoreReader(Config.Search searchConfig, ViewStoreClientFactory viewStoreClientFactory) + public ViewStoreReader(Config.Search searchConfig, ViewsConfig viewsConfig, ViewStoreClientFactory viewStoreClientFactory) throws SQLException { this.searchConfig = searchConfig; + this.viewsConfig = viewsConfig; this.configuration = viewStoreClientFactory.configuration; this.connection = viewStoreClientFactory.getConnection(); } @@ -287,7 +290,7 @@ String sqlFilter(String alias, View view, List filters, List .collect(Collectors.joining(" and ")); } - PreparedStatement query(String view, String projection, List filters, String scope) + PreparedStatement query(String view, List filters, String scope, boolean isCount) throws SQLException { if (filters == null) { filters = Collections.emptyList(); @@ -326,20 +329,40 @@ PreparedStatement query(String view, String projection, List filters .collect(Collectors.joining(" and ")); var viewTable = configuration.viewTables.get(view); - var query = connection.prepareStatement("select " + projection + " from " + String query = "select %s from " + viewTable.name + " v " + (constraints.isBlank() ? "" : " where " + constraints) - + (scope == null ? "" : (" " + scope))); + + (scope == null ? "" : (" " + scope)); + + query = isCount ? transformToCountQuery(view, query) : query.formatted("*"); + var preparedStatement = connection.prepareStatement(query); for (var i = 0; i < values.size(); i++) { var value = values.get(i); if (value instanceof Number) { - query.setFloat(i + 1, ((Number) value).floatValue()); + preparedStatement.setFloat(i + 1, ((Number) value).floatValue()); } else if (value instanceof Instant) { - query.setTimestamp(i + 1, Timestamp.from((Instant) value)); + preparedStatement.setTimestamp(i + 1, Timestamp.from((Instant) value)); } else { - query.setString(i + 1, value.toString()); + preparedStatement.setString(i + 1, value.toString()); } } - log.debug("Query: {}", query.toString()); + log.debug("Query: {}", preparedStatement.toString()); + return preparedStatement; + } + + private String transformToCountQuery(String viewName, String query) { + boolean isCountLimitDefeined = Optional.ofNullable(viewsConfig.getViewConfig(viewName)) + .map(c -> c.maxDisplayCount > 0) + .orElse(false); + if (isCountLimitDefeined) { // limit defined + query = query.formatted("*"); // set projection to * + // limitation is applied to improve performance + // we just set limit for the query and then wrap with count query + // on UI user either exact number if less the limit or "more than 'limit'" if more + query = "select count(*) as rowCount from ( " + query + " limit %s) as count"; + query = query.formatted(viewsConfig.getViewConfig(viewName).maxDisplayCount); + } else { + query = query.formatted("count(*) as rowCount"); + } return query; } @@ -376,10 +399,10 @@ private Map getViewRowsForNonSetType(View view, List 0 ? String.format("offset %d", offset) : "", limit))) { + "order by id %s limit %d", offset > 0 ? String.format("offset %d", offset) : "", limit), + false)) { query.setQueryTimeout((int) searchConfig.pageRequestTimeout); var result = query.executeQuery(); Map rowsById = new HashMap<>(); @@ -572,7 +595,7 @@ private void addJoinTableRowsForRow(ViewRow viewRow, List allJoinTableR } public long countRows(String view, List filters) throws SQLTimeoutException { - try (var q = query(view, "count(*) as rowCount", filters, null)) { + try (var q = query(view, filters, null, true)) { q.setQueryTimeout((int) searchConfig.countRequestTimeout); var result = q.executeQuery(); result.next(); diff --git a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/JdbcQueryServiceTest.java b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/JdbcQueryServiceTest.java index 6c1021613..99b304f79 100644 --- a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/JdbcQueryServiceTest.java +++ b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/JdbcQueryServiceTest.java @@ -1,29 +1,5 @@ package io.fairspace.saturn.services.views; -import java.io.IOException; -import java.sql.SQLException; -import java.util.Collections; -import java.util.Set; -import java.util.stream.Collectors; - -import io.milton.http.ResourceFactory; -import io.milton.http.exceptions.BadRequestException; -import io.milton.http.exceptions.ConflictException; -import io.milton.http.exceptions.NotAuthorizedException; -import io.milton.resource.MakeCollectionableResource; -import io.milton.resource.PutableResource; -import org.apache.jena.query.Dataset; -import org.apache.jena.rdf.model.Model; -import org.apache.jena.sparql.core.DatasetGraphFactory; -import org.apache.jena.sparql.util.Context; -import org.eclipse.jetty.server.Authentication; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - import io.fairspace.saturn.PostgresAwareTest; import io.fairspace.saturn.config.Config; import io.fairspace.saturn.config.ConfigLoader; @@ -46,13 +22,35 @@ import io.fairspace.saturn.webdav.DavFactory; import io.fairspace.saturn.webdav.blobstore.BlobInfo; import io.fairspace.saturn.webdav.blobstore.BlobStore; +import io.milton.http.ResourceFactory; +import io.milton.http.exceptions.BadRequestException; +import io.milton.http.exceptions.ConflictException; +import io.milton.http.exceptions.NotAuthorizedException; +import io.milton.resource.MakeCollectionableResource; +import io.milton.resource.PutableResource; +import org.apache.jena.query.Dataset; +import org.apache.jena.rdf.model.Model; +import org.apache.jena.sparql.core.DatasetGraphFactory; +import org.apache.jena.sparql.util.Context; +import org.eclipse.jetty.server.Authentication; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.io.IOException; +import java.sql.SQLException; +import java.util.Collections; +import java.util.Set; +import java.util.stream.Collectors; import static io.fairspace.saturn.TestUtils.createTestUser; import static io.fairspace.saturn.TestUtils.loadViewsConfig; import static io.fairspace.saturn.TestUtils.mockAuthentication; import static io.fairspace.saturn.TestUtils.setupRequestContext; import static io.fairspace.saturn.auth.RequestContext.getCurrentRequest; - import static org.apache.jena.query.DatasetFactory.wrap; import static org.mockito.Mockito.any; import static org.mockito.Mockito.lenient; @@ -125,7 +123,7 @@ public void before() var davFactory = new DavFactory(model.createResource(baseUri), store, userService, context); - sut = new JdbcQueryService(ConfigLoader.CONFIG.search, viewStoreClientFactory, tx, davFactory.root); + sut = new JdbcQueryService(ConfigLoader.CONFIG.search, ConfigLoader.VIEWS_CONFIG, viewStoreClientFactory, tx, davFactory.root); when(permissions.canWriteMetadata(any())).thenReturn(true); diff --git a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/ViewServiceTest.java b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/ViewServiceTest.java index acfbb8ad9..0976308cc 100644 --- a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/ViewServiceTest.java +++ b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/ViewServiceTest.java @@ -1,27 +1,5 @@ package io.fairspace.saturn.services.views; -import java.io.IOException; -import java.sql.SQLException; -import java.util.stream.Collectors; - -import io.milton.http.ResourceFactory; -import io.milton.http.exceptions.BadRequestException; -import io.milton.http.exceptions.ConflictException; -import io.milton.http.exceptions.NotAuthorizedException; -import io.milton.resource.MakeCollectionableResource; -import io.milton.resource.PutableResource; -import org.apache.jena.query.Dataset; -import org.apache.jena.rdf.model.Model; -import org.apache.jena.sparql.core.DatasetGraphFactory; -import org.apache.jena.sparql.util.Context; -import org.eclipse.jetty.server.Authentication; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - import io.fairspace.saturn.PostgresAwareTest; import io.fairspace.saturn.config.Config; import io.fairspace.saturn.config.ConfigLoader; @@ -41,6 +19,27 @@ import io.fairspace.saturn.webdav.DavFactory; import io.fairspace.saturn.webdav.blobstore.BlobInfo; import io.fairspace.saturn.webdav.blobstore.BlobStore; +import io.milton.http.ResourceFactory; +import io.milton.http.exceptions.BadRequestException; +import io.milton.http.exceptions.ConflictException; +import io.milton.http.exceptions.NotAuthorizedException; +import io.milton.resource.MakeCollectionableResource; +import io.milton.resource.PutableResource; +import org.apache.jena.query.Dataset; +import org.apache.jena.rdf.model.Model; +import org.apache.jena.sparql.core.DatasetGraphFactory; +import org.apache.jena.sparql.util.Context; +import org.eclipse.jetty.server.Authentication; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.io.IOException; +import java.sql.SQLException; +import java.util.stream.Collectors; import static io.fairspace.saturn.TestUtils.createTestUser; import static io.fairspace.saturn.TestUtils.loadViewsConfig; @@ -48,7 +47,6 @@ import static io.fairspace.saturn.TestUtils.setupRequestContext; import static io.fairspace.saturn.auth.RequestContext.getCurrentRequest; import static io.fairspace.saturn.services.views.ViewService.USER_DOES_NOT_HAVE_PERMISSIONS_TO_READ_FACETS; - import static org.apache.jena.query.DatasetFactory.wrap; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; From 7639fbf91ff2dc51d0328b5e043a249227433b36 Mon Sep 17 00:00:00 2001 From: ewelinagr Date: Thu, 8 Aug 2024 10:27:02 +0200 Subject: [PATCH 04/11] Fix metadata view table pagination next page on limit, add tests. --- .../components/TablePaginationActions.js | 10 +-- .../__tests__/TablePaginationAcrions.js | 82 +++++++++++++++++++ .../views/MetadataViewTableContainer.js | 14 +++- 3 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 projects/mercury/src/common/components/__tests__/TablePaginationAcrions.js diff --git a/projects/mercury/src/common/components/TablePaginationActions.js b/projects/mercury/src/common/components/TablePaginationActions.js index cccb60567..176483fc7 100644 --- a/projects/mercury/src/common/components/TablePaginationActions.js +++ b/projects/mercury/src/common/components/TablePaginationActions.js @@ -15,14 +15,14 @@ export type TablePaginationActionsProperties = { count: number, onPageChange: () => {}, page: number, - rowsPerPage: number + rowsPerPage: number, + countDisplayLimitReached?: boolean, + currentPageCount?: number }; const TablePaginationActions = (props: TablePaginationActionsProperties) => { const classes = useStyles(); - const {count, page, rowsPerPage, onPageChange, countDisplayLimit} = props; - - const countDisplayLimitReached = countDisplayLimit !== undefined && countDisplayLimit === count; + const {count, page, rowsPerPage, onPageChange, countDisplayLimitReached = false, hasNextFlag = false} = props; const handleFirstPageButtonClick = event => { onPageChange(event, 0); @@ -58,7 +58,7 @@ const TablePaginationActions = (props: TablePaginationActionsProperties) => { = Math.ceil(count / rowsPerPage) - 1} + disabled={page >= Math.ceil(count / rowsPerPage) - 1 && !(countDisplayLimitReached && hasNextFlag)} aria-label="next page" size="medium" > diff --git a/projects/mercury/src/common/components/__tests__/TablePaginationAcrions.js b/projects/mercury/src/common/components/__tests__/TablePaginationAcrions.js new file mode 100644 index 000000000..a23a3d1c0 --- /dev/null +++ b/projects/mercury/src/common/components/__tests__/TablePaginationAcrions.js @@ -0,0 +1,82 @@ +import React from 'react'; +import {render, fireEvent} from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; +import {ThemeProvider} from '@mui/material/styles'; +import TablePaginationActions from '../TablePaginationActions'; +import theme from '../../../App.theme'; + +describe('TablePaginationActions', () => { + const defaultProps = { + count: 100, + page: 0, + rowsPerPage: 10, + onPageChange: jest.fn() + }; + + const renderComponent = (props = {}) => { + return render( + + + + ); + }; + + it('renders all arrow buttons', () => { + const {getByLabelText} = renderComponent(); + expect(getByLabelText('first page')).toBeInTheDocument(); + expect(getByLabelText('previous page')).toBeInTheDocument(); + expect(getByLabelText('next page')).toBeInTheDocument(); + expect(getByLabelText('last page')).toBeInTheDocument(); + }); + + it('disables next and last buttons on last page', () => { + const props = {...defaultProps, page: 9}; + const {getByLabelText} = renderComponent(props); + expect(getByLabelText('next page')).toBeDisabled(); + expect(getByLabelText('last page')).toBeDisabled(); + }); + + it('calls onPageChange with correct arguments when first page button is clicked', () => { + const props = {...defaultProps, page: 1}; + const {getByLabelText} = renderComponent(props); + fireEvent.click(getByLabelText('first page')); + expect(defaultProps.onPageChange).toHaveBeenCalledWith(expect.anything(), 0); + }); + it('calls onPageChange with correct arguments when previous page button is clicked', () => { + const props = {...defaultProps, page: 1}; + const {getByLabelText} = renderComponent(props); + fireEvent.click(getByLabelText('previous page')); + expect(props.onPageChange).toHaveBeenCalledWith(expect.anything(), 0); + }); + + it('calls onPageChange with correct arguments when next page button is clicked', () => { + const {getByLabelText} = renderComponent(); + fireEvent.click(getByLabelText('next page')); + expect(defaultProps.onPageChange).toHaveBeenCalledWith(expect.anything(), 1); + }); + + it('calls onPageChange with correct arguments when last page button is clicked', () => { + const {getByLabelText} = renderComponent(); + fireEvent.click(getByLabelText('last page')); + expect(defaultProps.onPageChange).toHaveBeenCalledWith(expect.anything(), 9); + }); + + it('disables last page button when countDisplayLimitReached is true', () => { + const props = {...defaultProps, countDisplayLimitReached: true}; + const {getByLabelText} = renderComponent(props); + expect(getByLabelText('last page')).toBeDisabled(); + }); + + it('shows tooltip when countDisplayLimitReached is true', () => { + const props = {countDisplayLimitReached: true}; + const {getByLabelText} = renderComponent(props); + fireEvent.mouseOver(getByLabelText('last page')); + expect(getByLabelText('Total page count not available')).toBeInTheDocument(); + }); + + it('enables next page button when countDisplayLimitReached is true and hasNextFlag is true', () => { + const props = {...defaultProps, countDisplayLimitReached: true, hasNextFlag: true}; + const {getByLabelText} = renderComponent(props); + expect(getByLabelText('next page')).not.toBeDisabled(); + }); +}); diff --git a/projects/mercury/src/metadata/views/MetadataViewTableContainer.js b/projects/mercury/src/metadata/views/MetadataViewTableContainer.js index b55cf1806..13b54bac4 100644 --- a/projects/mercury/src/metadata/views/MetadataViewTableContainer.js +++ b/projects/mercury/src/metadata/views/MetadataViewTableContainer.js @@ -136,6 +136,7 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp locationContext, rowsPerPage ); + const viewCountDisplayLimitReached = viewCountDisplayLimit !== undefined && count.count === viewCountDisplayLimit; const [rowCheckboxes, setRowCheckboxes] = React.useState({}); const resetRowCheckboxes = () => { @@ -322,8 +323,7 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp const getTotalCountString = (totalCount: number, to: number) => { if (totalCount === undefined) return '...'; if (totalCount === -1) return `more than ${to}`; - if (viewCountDisplayLimit !== undefined && totalCount === viewCountDisplayLimit) - return `more than ${viewCountDisplayLimit - 1}`; + if (viewCountDisplayLimitReached) return `more than ${viewCountDisplayLimit - 1}`; return totalCount; }; @@ -375,7 +375,7 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp setPage(0); }, [filters]); - useEffect(() => { + useDeepCompareEffect(() => { resetRowCheckboxes(); }, [data]); @@ -456,7 +456,13 @@ export const MetadataViewTableContainer = (props: MetadataViewTableContainerProp labelDisplayedRows={d => renderTablePaginationLabel({...d, countIsLoading: loadingCount, countHasError: countError}) } - ActionsComponent={p => TablePaginationActions({...p, countDisplayLimit: viewCountDisplayLimit})} + ActionsComponent={p => + TablePaginationActions({ + ...p, + countDisplayLimitReached: viewCountDisplayLimitReached, + hasNextFlag: data?.hasNext + }) + } /> From 37df834e3d23885f21001c02e8add4c06a26c985 Mon Sep 17 00:00:00 2001 From: anton Date: Thu, 8 Aug 2024 11:30:43 +0200 Subject: [PATCH 05/11] FAIRSPC-108: made the max display count settings flat --- .../main/java/io/fairspace/saturn/config/ViewsConfig.java | 6 +++--- .../java/io/fairspace/saturn/services/views/ViewDTO.java | 4 +++- .../io/fairspace/saturn/services/views/ViewService.java | 5 ++++- .../io/fairspace/saturn/services/views/ViewStoreReader.java | 6 +++--- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java b/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java index f75566942..0bacff8bb 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java @@ -18,12 +18,12 @@ public class ViewsConfig { @JsonSetter(nulls = Nulls.AS_EMPTY) public List views = new ArrayList<>(); - public View getViewConfig(String viewName) { + public Optional getViewConfig(String viewName) { if (viewConfig == null) { viewConfig = views.stream() .collect(Collectors.toMap(view -> view.name, Function.identity())); } - return viewConfig.get(viewName); + return Optional.ofNullable(viewConfig.get(viewName)); } public enum ColumnType { @@ -86,7 +86,7 @@ public static class View { * If total count is greater than this max value, the total count value will look like 'more than 1000' on FE. * This is to prevent performance issues when the total count is too large. */ - public long maxDisplayCount; + public Long maxDisplayCount; /** * The URLs of the types of entities that should be indexed in this view. */ diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java index 5d8d900c8..196737d7b 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java @@ -2,6 +2,7 @@ import java.util.List; +import com.fasterxml.jackson.annotation.JsonInclude; import lombok.Value; @Value @@ -9,5 +10,6 @@ public class ViewDTO { String name; String title; List columns; - ViewConfigDto config; + @JsonInclude(JsonInclude.Include.NON_NULL) + Long maxDisplayCount; } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java index 64554d6dd..a59ded189 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java @@ -187,7 +187,10 @@ protected List fetchViews() { columns.add(new ColumnDTO(joinView.name + "_" + c.name, c.title, c.type, j.displayIndex)); } } - return new ViewDTO(v.name, v.title, columns, ViewConfigDto.from(viewsConfig.getViewConfig(v.name))); + Long maxDisplayCount = viewsConfig.getViewConfig(v.name) + .map(t -> t.maxDisplayCount) + .orElse(null); + return new ViewDTO(v.name, v.title, columns, maxDisplayCount); }) .collect(toList()); } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java index 31391e2fa..d69eb57fa 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java @@ -350,16 +350,16 @@ PreparedStatement query(String view, List filters, String scope, boo } private String transformToCountQuery(String viewName, String query) { - boolean isCountLimitDefeined = Optional.ofNullable(viewsConfig.getViewConfig(viewName)) + boolean isCountLimitDefined = viewsConfig.getViewConfig(viewName) .map(c -> c.maxDisplayCount > 0) .orElse(false); - if (isCountLimitDefeined) { // limit defined + if (isCountLimitDefined) { // limit defined query = query.formatted("*"); // set projection to * // limitation is applied to improve performance // we just set limit for the query and then wrap with count query // on UI user either exact number if less the limit or "more than 'limit'" if more query = "select count(*) as rowCount from ( " + query + " limit %s) as count"; - query = query.formatted(viewsConfig.getViewConfig(viewName).maxDisplayCount); + query = query.formatted(viewsConfig.getViewConfig(viewName).map(c -> c.maxDisplayCount).orElseThrow()); } else { query = query.formatted("count(*) as rowCount"); } From 0d25ea209c5ffa1904cca04270a3409c7c4cf2e3 Mon Sep 17 00:00:00 2001 From: ewelinagr Date: Thu, 8 Aug 2024 11:08:13 +0200 Subject: [PATCH 06/11] Update views request response format on the UI side. --- projects/mercury/src/metadata/views/MetadataViewAPI.js | 6 +----- projects/mercury/src/metadata/views/MetadataViewTabs.js | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/projects/mercury/src/metadata/views/MetadataViewAPI.js b/projects/mercury/src/metadata/views/MetadataViewAPI.js index 24396f49d..351124cc2 100644 --- a/projects/mercury/src/metadata/views/MetadataViewAPI.js +++ b/projects/mercury/src/metadata/views/MetadataViewAPI.js @@ -40,15 +40,11 @@ export type MetadataViewColumn = { type: ValueType }; -export type MetadataViewsConfig = { - maxDisplayCount: number -}; - export type MetadataViewOptions = { name: string, title: string, columns: MetadataViewColumn[], - config: MetadataViewsConfig + maxDisplayCount: number }; export type MetadataViews = { diff --git a/projects/mercury/src/metadata/views/MetadataViewTabs.js b/projects/mercury/src/metadata/views/MetadataViewTabs.js index 877a48fb7..defc82bde 100644 --- a/projects/mercury/src/metadata/views/MetadataViewTabs.js +++ b/projects/mercury/src/metadata/views/MetadataViewTabs.js @@ -92,7 +92,7 @@ export const MetadataViewTabs = (props: MetadataViewTabsProperties) => { columns={appendCustomColumns(view)} idColumn={idColumn} view={view.name} - viewCountDisplayLimit={view.config?.maxDisplayCount} + viewCountDisplayLimit={view.maxDisplayCount} filters={filters} locationContext={locationContext} selected={selected} From 992105d3bf14272487fb187b6430b432b166ad99 Mon Sep 17 00:00:00 2001 From: anton Date: Thu, 8 Aug 2024 16:12:03 +0200 Subject: [PATCH 07/11] FAIRSPC-108: added improvement for count SPARQL --- .../services/views/SparqlQueryService.java | 135 ++++++++++++------ .../services/views/ViewStoreReader.java | 2 +- 2 files changed, 91 insertions(+), 46 deletions(-) diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java index 4d31697b0..0881c7f09 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java @@ -1,18 +1,5 @@ package io.fairspace.saturn.services.views; -import java.time.*; -import java.util.*; - -import lombok.extern.log4j.*; -import org.apache.jena.datatypes.xsd.XSDDateTime; -import org.apache.jena.query.*; -import org.apache.jena.rdf.model.RDFNode; -import org.apache.jena.rdf.model.Resource; -import org.apache.jena.rdf.model.Statement; -import org.apache.jena.sparql.expr.*; -import org.apache.jena.sparql.syntax.ElementFilter; -import org.apache.jena.vocabulary.RDFS; - import io.fairspace.saturn.config.Config; import io.fairspace.saturn.config.ViewsConfig; import io.fairspace.saturn.config.ViewsConfig.ColumnType; @@ -21,17 +8,55 @@ import io.fairspace.saturn.services.search.FileSearchRequest; import io.fairspace.saturn.services.search.SearchResultDTO; import io.fairspace.saturn.vocabulary.FS; +import lombok.extern.log4j.Log4j2; +import org.apache.jena.datatypes.xsd.XSDDateTime; +import org.apache.jena.query.Dataset; +import org.apache.jena.query.Query; +import org.apache.jena.query.QueryCancelledException; +import org.apache.jena.query.QueryExecutionFactory; +import org.apache.jena.query.QueryFactory; +import org.apache.jena.query.QuerySolutionMap; +import org.apache.jena.rdf.model.RDFNode; +import org.apache.jena.rdf.model.Resource; +import org.apache.jena.rdf.model.Statement; +import org.apache.jena.sparql.expr.E_Equals; +import org.apache.jena.sparql.expr.E_GreaterThanOrEqual; +import org.apache.jena.sparql.expr.E_LessThanOrEqual; +import org.apache.jena.sparql.expr.E_LogicalAnd; +import org.apache.jena.sparql.expr.E_OneOf; +import org.apache.jena.sparql.expr.E_StrLowerCase; +import org.apache.jena.sparql.expr.E_StrStartsWith; +import org.apache.jena.sparql.expr.Expr; +import org.apache.jena.sparql.expr.ExprList; +import org.apache.jena.sparql.expr.ExprVar; +import org.apache.jena.sparql.expr.NodeValue; +import org.apache.jena.sparql.syntax.ElementFilter; +import org.apache.jena.vocabulary.RDFS; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.Calendar; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import static io.fairspace.saturn.rdf.ModelUtils.getResourceProperties; import static io.fairspace.saturn.util.ValidationUtils.validateIRI; - import static java.time.Instant.ofEpochMilli; import static java.util.Comparator.comparing; -import static java.util.stream.Collectors.*; +import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toCollection; +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; import static org.apache.jena.graph.NodeFactory.createURI; import static org.apache.jena.rdf.model.ResourceFactory.createProperty; import static org.apache.jena.rdf.model.ResourceFactory.createStringLiteral; -import static org.apache.jena.sparql.expr.NodeValue.*; +import static org.apache.jena.sparql.expr.NodeValue.makeBoolean; +import static org.apache.jena.sparql.expr.NodeValue.makeDate; +import static org.apache.jena.sparql.expr.NodeValue.makeDecimal; +import static org.apache.jena.sparql.expr.NodeValue.makeNode; import static org.apache.jena.sparql.expr.NodeValue.makeString; import static org.apache.jena.system.Txn.calculateRead; @@ -39,17 +64,17 @@ public class SparqlQueryService implements QueryService { private static final String RESOURCES_VIEW = "Resource"; private final Config.Search config; - private final ViewsConfig searchConfig; + private final ViewsConfig viewsConfig; private final Dataset ds; public SparqlQueryService(Config.Search config, ViewsConfig viewsConfig, Dataset ds) { this.config = config; - this.searchConfig = viewsConfig; + this.viewsConfig = viewsConfig; this.ds = ds; } public ViewPageDTO retrieveViewPage(ViewRequest request) { - var query = getQuery(request); + var query = getQuery(request, false); log.debug("Executing query:\n{}", query); @@ -105,8 +130,8 @@ private Map> fetch(Resource resource, String viewName) { var prop = createProperty(j.on); var refs = j.reverse ? resource.getModel() - .listResourcesWithProperty(prop, resource) - .toList() + .listResourcesWithProperty(prop, resource) + .toList() : getResourceProperties(resource, prop); for (var colName : j.include) { @@ -146,9 +171,7 @@ private Set getValues(Resource resource, View.Column column) { } private View getView(String viewName) { - return searchConfig.views.stream() - .filter(v -> v.name.equals(viewName)) - .findFirst() + return viewsConfig.getViewConfig(viewName) .orElseThrow(() -> new IllegalArgumentException("Unknown view: " + viewName)); } @@ -169,13 +192,12 @@ private ValueDTO toValueDTO(RDFNode node) { return new ValueDTO(label, resource.getURI()); } - private Query getQuery(CountRequest request) { + private Query getQuery(CountRequest request, boolean isCount) { var view = getView(request.getView()); - var builder = new StringBuilder("PREFIX fs: <") - .append(FS.NS) - .append(">\n\nSELECT ?") - .append(view.name) + var builder = new StringBuilder() + .append("SELECT ") + .append("%s") .append("\nWHERE {\n"); if (request.getFilters() != null) { @@ -278,7 +300,28 @@ private Query getQuery(CountRequest request) { .append(view.name) .append(" fs:dateDeleted ?any }\n}"); - return QueryFactory.create(builder.toString()); + var query = isCount + ? transformToCountQuery(builder.toString(), view) + : builder.toString().formatted("?" + view.name); + + query = "PREFIX fs: <%s>\n\n" .formatted(FS.NS) + query; // adding prefix to the query + + return QueryFactory.create(query); + } + + private String transformToCountQuery(String queryTemplate, View view) { + String query; + if (view.maxDisplayCount == null) { + query = queryTemplate.formatted("(COUNT(?%s) AS ?count)" .formatted(view.name)); + } else { + query = new StringBuilder() + .append("SELECT (COUNT(?%s) AS ?count)" .formatted(view.name)) + .append("\nWHERE {\n{\n") + .append(queryTemplate.formatted("?" + view.name)) + .append("\nLIMIT %d\n}\n}" .formatted(view.maxDisplayCount)) + .toString(); + } + return query; } private String toFilterString(ViewFilter filter, ColumnType type, String field) { @@ -364,8 +407,8 @@ private static NodeValue toNodeValue(Object o, ColumnType type) { case Identifier, Term, TermSet -> makeNode(createURI(o.toString())); case Text, Set -> makeString(o.toString()); case Number -> makeDecimal(o.toString()); - // Extract date only as it comes in format "yyyy-MM-ddTHH:mm:ss.SSSX" - // Leave it as is in case of adding new DateTime filter format + // Extract date only as it comes in format "yyyy-MM-ddTHH:mm:ss.SSSX" + // Leave it as is in case of adding new DateTime filter format case Date -> makeDate(o.toString().split("T")[0]); case Boolean -> makeBoolean(convertBooleanValue(o.toString())); }; @@ -376,23 +419,25 @@ private static boolean convertBooleanValue(String value) { } public CountDTO count(CountRequest request) { - var query = getQuery(request); + var query = getQuery(request, true); log.debug("Querying the total number of matches: \n{}", query); - var execution = QueryExecutionFactory.create(query, ds); - execution.setTimeout(config.countRequestTimeout); + try (var execution = QueryExecutionFactory.create(query, ds)) { + execution.setTimeout(config.countRequestTimeout); - return calculateRead(ds, () -> { - long count = 0; - try (execution) { - for (var it = execution.execSelect(); it.hasNext(); it.next()) { - count++; + return calculateRead(ds, () -> { + var queryResult = execution.execSelect(); + if (queryResult.hasNext()) { + var row = queryResult.next(); + var count = row.getLiteral("count").getLong(); + return new CountDTO(count, false); + } else { + return new CountDTO(-1, false); } - return new CountDTO(count, false); - } catch (QueryCancelledException e) { - return new CountDTO(count, true); - } - }); + }); + } catch (QueryCancelledException e) { + return new CountDTO(-1, true); + } } } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java index d69eb57fa..ee1486b02 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java @@ -351,7 +351,7 @@ PreparedStatement query(String view, List filters, String scope, boo private String transformToCountQuery(String viewName, String query) { boolean isCountLimitDefined = viewsConfig.getViewConfig(viewName) - .map(c -> c.maxDisplayCount > 0) + .map(c -> c.maxDisplayCount != null) .orElse(false); if (isCountLimitDefined) { // limit defined query = query.formatted("*"); // set projection to * From 092fbb4153ef879e573d03433b4e86f09f94f926 Mon Sep 17 00:00:00 2001 From: anton Date: Thu, 8 Aug 2024 21:24:44 +0200 Subject: [PATCH 08/11] FAIRSPC-108: added tests covering cases of max display count value --- .../io/fairspace/saturn/config/Services.java | 3 +- .../fairspace/saturn/config/ViewsConfig.java | 3 +- .../services/views/JdbcQueryService.java | 21 ++--- .../services/views/SparqlQueryService.java | 58 +++++++------- .../saturn/services/views/ViewConfigDto.java | 5 +- .../saturn/services/views/ViewDTO.java | 1 + .../saturn/services/views/ViewService.java | 29 ++++--- .../services/views/ViewStoreReader.java | 42 +++++----- .../services/views/JdbcQueryServiceTest.java | 79 ++++++++++++------- .../views/SparqlQueryServiceTest.java | 18 ++++- .../services/views/ViewServiceTest.java | 44 ++++++----- .../saturn/src/test/resources/test-views.yaml | 2 + 12 files changed, 179 insertions(+), 126 deletions(-) diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/config/Services.java b/projects/saturn/src/main/java/io/fairspace/saturn/config/Services.java index 6a1054e56..f392857ce 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/config/Services.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/config/Services.java @@ -113,7 +113,8 @@ public Services( queryService = viewStoreClientFactory == null ? new SparqlQueryService(config.search, viewsConfig, filteredDataset) - : new JdbcQueryService(config.search, viewsConfig, viewStoreClientFactory, transactions, davFactory.root); + : new JdbcQueryService( + config.search, viewsConfig, viewStoreClientFactory, transactions, davFactory.root); viewService = new ViewService(config, viewsConfig, filteredDataset, viewStoreClientFactory, metadataPermissions); diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java b/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java index 0bacff8bb..4e1e27cdf 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java @@ -20,8 +20,7 @@ public class ViewsConfig { public Optional getViewConfig(String viewName) { if (viewConfig == null) { - viewConfig = views.stream() - .collect(Collectors.toMap(view -> view.name, Function.identity())); + viewConfig = views.stream().collect(Collectors.toMap(view -> view.name, Function.identity())); } return Optional.ofNullable(viewConfig.get(viewName)); } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/JdbcQueryService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/JdbcQueryService.java index 01a50d12a..a45234373 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/JdbcQueryService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/JdbcQueryService.java @@ -1,15 +1,5 @@ package io.fairspace.saturn.services.views; -import io.fairspace.saturn.config.Config; -import io.fairspace.saturn.config.ViewsConfig; -import io.fairspace.saturn.rdf.transactions.Transactions; -import io.fairspace.saturn.rdf.transactions.TxnIndexDatasetGraph; -import io.fairspace.saturn.services.search.FileSearchRequest; -import io.fairspace.saturn.services.search.SearchResultDTO; -import io.milton.resource.CollectionResource; -import lombok.SneakyThrows; -import lombok.extern.log4j.Log4j2; - import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.sql.SQLException; @@ -21,6 +11,17 @@ import java.util.Set; import java.util.stream.Collectors; +import io.milton.resource.CollectionResource; +import lombok.SneakyThrows; +import lombok.extern.log4j.Log4j2; + +import io.fairspace.saturn.config.Config; +import io.fairspace.saturn.config.ViewsConfig; +import io.fairspace.saturn.rdf.transactions.Transactions; +import io.fairspace.saturn.rdf.transactions.TxnIndexDatasetGraph; +import io.fairspace.saturn.services.search.FileSearchRequest; +import io.fairspace.saturn.services.search.SearchResultDTO; + import static java.lang.Integer.min; /** diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java index 0881c7f09..9103f01b4 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java @@ -1,13 +1,14 @@ package io.fairspace.saturn.services.views; -import io.fairspace.saturn.config.Config; -import io.fairspace.saturn.config.ViewsConfig; -import io.fairspace.saturn.config.ViewsConfig.ColumnType; -import io.fairspace.saturn.config.ViewsConfig.View; -import io.fairspace.saturn.rdf.SparqlUtils; -import io.fairspace.saturn.services.search.FileSearchRequest; -import io.fairspace.saturn.services.search.SearchResultDTO; -import io.fairspace.saturn.vocabulary.FS; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Calendar; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; + import lombok.extern.log4j.Log4j2; import org.apache.jena.datatypes.xsd.XSDDateTime; import org.apache.jena.query.Dataset; @@ -33,17 +34,18 @@ import org.apache.jena.sparql.syntax.ElementFilter; import org.apache.jena.vocabulary.RDFS; -import java.time.Instant; -import java.util.ArrayList; -import java.util.Calendar; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.TreeSet; +import io.fairspace.saturn.config.Config; +import io.fairspace.saturn.config.ViewsConfig; +import io.fairspace.saturn.config.ViewsConfig.ColumnType; +import io.fairspace.saturn.config.ViewsConfig.View; +import io.fairspace.saturn.rdf.SparqlUtils; +import io.fairspace.saturn.services.search.FileSearchRequest; +import io.fairspace.saturn.services.search.SearchResultDTO; +import io.fairspace.saturn.vocabulary.FS; import static io.fairspace.saturn.rdf.ModelUtils.getResourceProperties; import static io.fairspace.saturn.util.ValidationUtils.validateIRI; + import static java.time.Instant.ofEpochMilli; import static java.util.Comparator.comparing; import static java.util.stream.Collectors.joining; @@ -130,8 +132,8 @@ private Map> fetch(Resource resource, String viewName) { var prop = createProperty(j.on); var refs = j.reverse ? resource.getModel() - .listResourcesWithProperty(prop, resource) - .toList() + .listResourcesWithProperty(prop, resource) + .toList() : getResourceProperties(resource, prop); for (var colName : j.include) { @@ -171,7 +173,8 @@ private Set getValues(Resource resource, View.Column column) { } private View getView(String viewName) { - return viewsConfig.getViewConfig(viewName) + return viewsConfig + .getViewConfig(viewName) .orElseThrow(() -> new IllegalArgumentException("Unknown view: " + viewName)); } @@ -195,10 +198,7 @@ private ValueDTO toValueDTO(RDFNode node) { private Query getQuery(CountRequest request, boolean isCount) { var view = getView(request.getView()); - var builder = new StringBuilder() - .append("SELECT ") - .append("%s") - .append("\nWHERE {\n"); + var builder = new StringBuilder().append("SELECT ").append("%s").append("\nWHERE {\n"); if (request.getFilters() != null) { var filters = new ArrayList<>(request.getFilters()); @@ -304,7 +304,7 @@ private Query getQuery(CountRequest request, boolean isCount) { ? transformToCountQuery(builder.toString(), view) : builder.toString().formatted("?" + view.name); - query = "PREFIX fs: <%s>\n\n" .formatted(FS.NS) + query; // adding prefix to the query + query = "PREFIX fs: <%s>\n\n".formatted(FS.NS) + query; // adding prefix to the query return QueryFactory.create(query); } @@ -312,13 +312,13 @@ private Query getQuery(CountRequest request, boolean isCount) { private String transformToCountQuery(String queryTemplate, View view) { String query; if (view.maxDisplayCount == null) { - query = queryTemplate.formatted("(COUNT(?%s) AS ?count)" .formatted(view.name)); + query = queryTemplate.formatted("(COUNT(?%s) AS ?count)".formatted(view.name)); } else { query = new StringBuilder() - .append("SELECT (COUNT(?%s) AS ?count)" .formatted(view.name)) + .append("SELECT (COUNT(?%s) AS ?count)".formatted(view.name)) .append("\nWHERE {\n{\n") .append(queryTemplate.formatted("?" + view.name)) - .append("\nLIMIT %d\n}\n}" .formatted(view.maxDisplayCount)) + .append("\nLIMIT %d\n}\n}".formatted(view.maxDisplayCount)) .toString(); } return query; @@ -407,8 +407,8 @@ private static NodeValue toNodeValue(Object o, ColumnType type) { case Identifier, Term, TermSet -> makeNode(createURI(o.toString())); case Text, Set -> makeString(o.toString()); case Number -> makeDecimal(o.toString()); - // Extract date only as it comes in format "yyyy-MM-ddTHH:mm:ss.SSSX" - // Leave it as is in case of adding new DateTime filter format + // Extract date only as it comes in format "yyyy-MM-ddTHH:mm:ss.SSSX" + // Leave it as is in case of adding new DateTime filter format case Date -> makeDate(o.toString().split("T")[0]); case Boolean -> makeBoolean(convertBooleanValue(o.toString())); }; diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java index 2f56728ce..a39494a5b 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java @@ -1,8 +1,9 @@ package io.fairspace.saturn.services.views; -import io.fairspace.saturn.config.ViewsConfig; import lombok.Value; +import io.fairspace.saturn.config.ViewsConfig; + @Value public class ViewConfigDto { @@ -11,6 +12,4 @@ public class ViewConfigDto { public static ViewConfigDto from(ViewsConfig.View config) { return new ViewConfigDto(config.maxDisplayCount); } - } - diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java index 196737d7b..6ba67f863 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewDTO.java @@ -10,6 +10,7 @@ public class ViewDTO { String name; String title; List columns; + @JsonInclude(JsonInclude.Include.NON_NULL) Long maxDisplayCount; } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java index a59ded189..d12c7d51b 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java @@ -1,14 +1,15 @@ package io.fairspace.saturn.services.views; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; + import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; -import io.fairspace.saturn.config.Config; -import io.fairspace.saturn.config.ViewsConfig; -import io.fairspace.saturn.rdf.search.FilteredDatasetGraph; -import io.fairspace.saturn.services.AccessDeniedException; -import io.fairspace.saturn.services.metadata.MetadataPermissions; -import io.fairspace.saturn.vocabulary.FS; import lombok.SneakyThrows; import lombok.extern.log4j.Log4j2; import org.apache.jena.datatypes.xsd.XSDDateTime; @@ -20,15 +21,16 @@ import org.apache.jena.query.QuerySolutionMap; import org.apache.jena.rdf.model.Literal; -import java.util.ArrayList; -import java.util.EnumSet; -import java.util.List; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.function.Supplier; +import io.fairspace.saturn.config.Config; +import io.fairspace.saturn.config.ViewsConfig; +import io.fairspace.saturn.rdf.search.FilteredDatasetGraph; +import io.fairspace.saturn.services.AccessDeniedException; +import io.fairspace.saturn.services.metadata.MetadataPermissions; +import io.fairspace.saturn.vocabulary.FS; import static io.fairspace.saturn.config.ViewsConfig.ColumnType; import static io.fairspace.saturn.config.ViewsConfig.View; + import static java.time.Instant.ofEpochMilli; import static java.util.Optional.ofNullable; import static java.util.stream.Collectors.toList; @@ -187,7 +189,8 @@ protected List fetchViews() { columns.add(new ColumnDTO(joinView.name + "_" + c.name, c.title, c.type, j.displayIndex)); } } - Long maxDisplayCount = viewsConfig.getViewConfig(v.name) + Long maxDisplayCount = viewsConfig + .getViewConfig(v.name) .map(t -> t.maxDisplayCount) .orElse(null); return new ViewDTO(v.name, v.title, columns, maxDisplayCount); diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java index ee1486b02..3aaa8d43b 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreReader.java @@ -1,17 +1,5 @@ package io.fairspace.saturn.services.views; -import com.google.common.collect.Sets; -import io.fairspace.saturn.config.Config; -import io.fairspace.saturn.config.ViewsConfig; -import io.fairspace.saturn.config.ViewsConfig.ColumnType; -import io.fairspace.saturn.config.ViewsConfig.View; -import io.fairspace.saturn.services.search.FileSearchRequest; -import io.fairspace.saturn.services.search.SearchResultDTO; -import io.fairspace.saturn.vocabulary.FS; -import lombok.SneakyThrows; -import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang3.StringUtils; - import java.sql.Array; import java.sql.Connection; import java.sql.PreparedStatement; @@ -33,6 +21,19 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import com.google.common.collect.Sets; +import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; + +import io.fairspace.saturn.config.Config; +import io.fairspace.saturn.config.ViewsConfig; +import io.fairspace.saturn.config.ViewsConfig.ColumnType; +import io.fairspace.saturn.config.ViewsConfig.View; +import io.fairspace.saturn.services.search.FileSearchRequest; +import io.fairspace.saturn.services.search.SearchResultDTO; +import io.fairspace.saturn.vocabulary.FS; + import static io.fairspace.saturn.config.ViewsConfig.ColumnType.Date; import static io.fairspace.saturn.services.views.Table.idColumn; @@ -53,7 +54,8 @@ public class ViewStoreReader implements AutoCloseable { final Connection connection; // TODO: in whole class, use StringBuilder instead of String concats - public ViewStoreReader(Config.Search searchConfig, ViewsConfig viewsConfig, ViewStoreClientFactory viewStoreClientFactory) + public ViewStoreReader( + Config.Search searchConfig, ViewsConfig viewsConfig, ViewStoreClientFactory viewStoreClientFactory) throws SQLException { this.searchConfig = searchConfig; this.viewsConfig = viewsConfig; @@ -290,8 +292,7 @@ String sqlFilter(String alias, View view, List filters, List .collect(Collectors.joining(" and ")); } - PreparedStatement query(String view, List filters, String scope, boolean isCount) - throws SQLException { + PreparedStatement query(String view, List filters, String scope, boolean isCount) throws SQLException { if (filters == null) { filters = Collections.emptyList(); } @@ -350,7 +351,8 @@ PreparedStatement query(String view, List filters, String scope, boo } private String transformToCountQuery(String viewName, String query) { - boolean isCountLimitDefined = viewsConfig.getViewConfig(viewName) + boolean isCountLimitDefined = viewsConfig + .getViewConfig(viewName) .map(c -> c.maxDisplayCount != null) .orElse(false); if (isCountLimitDefined) { // limit defined @@ -359,7 +361,10 @@ private String transformToCountQuery(String viewName, String query) { // we just set limit for the query and then wrap with count query // on UI user either exact number if less the limit or "more than 'limit'" if more query = "select count(*) as rowCount from ( " + query + " limit %s) as count"; - query = query.formatted(viewsConfig.getViewConfig(viewName).map(c -> c.maxDisplayCount).orElseThrow()); + query = query.formatted(viewsConfig + .getViewConfig(viewName) + .map(c -> c.maxDisplayCount) + .orElseThrow()); } else { query = query.formatted("count(*) as rowCount"); } @@ -400,8 +405,7 @@ private Map getViewRowsForNonSetType(View view, List 0 ? String.format("offset %d", offset) : "", limit), + String.format("order by id %s limit %d", offset > 0 ? String.format("offset %d", offset) : "", limit), false)) { query.setQueryTimeout((int) searchConfig.pageRequestTimeout); var result = query.executeQuery(); diff --git a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/JdbcQueryServiceTest.java b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/JdbcQueryServiceTest.java index 99b304f79..8053a7f25 100644 --- a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/JdbcQueryServiceTest.java +++ b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/JdbcQueryServiceTest.java @@ -1,5 +1,29 @@ package io.fairspace.saturn.services.views; +import java.io.IOException; +import java.sql.SQLException; +import java.util.Collections; +import java.util.Set; +import java.util.stream.Collectors; + +import io.milton.http.ResourceFactory; +import io.milton.http.exceptions.BadRequestException; +import io.milton.http.exceptions.ConflictException; +import io.milton.http.exceptions.NotAuthorizedException; +import io.milton.resource.MakeCollectionableResource; +import io.milton.resource.PutableResource; +import org.apache.jena.query.Dataset; +import org.apache.jena.rdf.model.Model; +import org.apache.jena.sparql.core.DatasetGraphFactory; +import org.apache.jena.sparql.util.Context; +import org.eclipse.jetty.server.Authentication; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + import io.fairspace.saturn.PostgresAwareTest; import io.fairspace.saturn.config.Config; import io.fairspace.saturn.config.ConfigLoader; @@ -22,36 +46,15 @@ import io.fairspace.saturn.webdav.DavFactory; import io.fairspace.saturn.webdav.blobstore.BlobInfo; import io.fairspace.saturn.webdav.blobstore.BlobStore; -import io.milton.http.ResourceFactory; -import io.milton.http.exceptions.BadRequestException; -import io.milton.http.exceptions.ConflictException; -import io.milton.http.exceptions.NotAuthorizedException; -import io.milton.resource.MakeCollectionableResource; -import io.milton.resource.PutableResource; -import org.apache.jena.query.Dataset; -import org.apache.jena.rdf.model.Model; -import org.apache.jena.sparql.core.DatasetGraphFactory; -import org.apache.jena.sparql.util.Context; -import org.eclipse.jetty.server.Authentication; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - -import java.io.IOException; -import java.sql.SQLException; -import java.util.Collections; -import java.util.Set; -import java.util.stream.Collectors; import static io.fairspace.saturn.TestUtils.createTestUser; import static io.fairspace.saturn.TestUtils.loadViewsConfig; import static io.fairspace.saturn.TestUtils.mockAuthentication; import static io.fairspace.saturn.TestUtils.setupRequestContext; import static io.fairspace.saturn.auth.RequestContext.getCurrentRequest; + import static org.apache.jena.query.DatasetFactory.wrap; +import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.any; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.when; @@ -123,7 +126,12 @@ public void before() var davFactory = new DavFactory(model.createResource(baseUri), store, userService, context); - sut = new JdbcQueryService(ConfigLoader.CONFIG.search, ConfigLoader.VIEWS_CONFIG, viewStoreClientFactory, tx, davFactory.root); + sut = new JdbcQueryService( + ConfigLoader.CONFIG.search, + loadViewsConfig("src/test/resources/test-views.yaml"), + viewStoreClientFactory, + tx, + davFactory.root); when(permissions.canWriteMetadata(any())).thenReturn(true); @@ -341,11 +349,28 @@ public void testRetrieveSamplePageIncludeJoinAfterReindexing() { } @Test - public void testCountSamples() { + public void testCountSamplesWithoutMaxDisplayCount() { + selectRegularUser(); + var requestParams = new CountRequest(); + requestParams.setView("Sample"); + var result = sut.count(requestParams); + assertEquals(2, result.getCount()); + } + + @Test + public void testCountSubjectWithMaxDisplayCountLimitLessThanTotalCount() { var request = new CountRequest(); - request.setView("Sample"); + request.setView("Subject"); + var result = sut.count(request); + Assert.assertEquals(1, result.getCount()); + } + + @Test + public void testCountResourceWithMaxDisplayCountLimitMoreThanTotalCount() { + var request = new CountRequest(); + request.setView("Resource"); var result = sut.count(request); - Assert.assertEquals(2, result.getCount()); + Assert.assertEquals(4, result.getCount()); } @Test diff --git a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/SparqlQueryServiceTest.java b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/SparqlQueryServiceTest.java index cf90f05d0..7affd5628 100644 --- a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/SparqlQueryServiceTest.java +++ b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/SparqlQueryServiceTest.java @@ -184,7 +184,7 @@ public void testRetrieveSamplePage() { } @Test - public void testCountSamples() { + public void testCountSamplesWithoutMaxDisplayCount() { selectRegularUser(); var requestParams = new CountRequest(); requestParams.setView("Sample"); @@ -192,6 +192,22 @@ public void testCountSamples() { assertEquals(2, result.getCount()); } + @Test + public void testCountSubjectWithMaxDisplayCountLimitLessThanTotalCount() { + var request = new CountRequest(); + request.setView("Subject"); + var result = queryService.count(request); + Assert.assertEquals(1, result.getCount()); + } + + @Test + public void testCountResourceWithMaxDisplayCountLimitMoreThanTotalCount() { + var request = new CountRequest(); + request.setView("Resource"); + var result = queryService.count(request); + Assert.assertEquals(3, result.getCount()); + } + @Test public void testCountSamplesWithoutViewAccess() { selectExternalUser(); diff --git a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/ViewServiceTest.java b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/ViewServiceTest.java index 0976308cc..acfbb8ad9 100644 --- a/projects/saturn/src/test/java/io/fairspace/saturn/services/views/ViewServiceTest.java +++ b/projects/saturn/src/test/java/io/fairspace/saturn/services/views/ViewServiceTest.java @@ -1,5 +1,27 @@ package io.fairspace.saturn.services.views; +import java.io.IOException; +import java.sql.SQLException; +import java.util.stream.Collectors; + +import io.milton.http.ResourceFactory; +import io.milton.http.exceptions.BadRequestException; +import io.milton.http.exceptions.ConflictException; +import io.milton.http.exceptions.NotAuthorizedException; +import io.milton.resource.MakeCollectionableResource; +import io.milton.resource.PutableResource; +import org.apache.jena.query.Dataset; +import org.apache.jena.rdf.model.Model; +import org.apache.jena.sparql.core.DatasetGraphFactory; +import org.apache.jena.sparql.util.Context; +import org.eclipse.jetty.server.Authentication; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + import io.fairspace.saturn.PostgresAwareTest; import io.fairspace.saturn.config.Config; import io.fairspace.saturn.config.ConfigLoader; @@ -19,27 +41,6 @@ import io.fairspace.saturn.webdav.DavFactory; import io.fairspace.saturn.webdav.blobstore.BlobInfo; import io.fairspace.saturn.webdav.blobstore.BlobStore; -import io.milton.http.ResourceFactory; -import io.milton.http.exceptions.BadRequestException; -import io.milton.http.exceptions.ConflictException; -import io.milton.http.exceptions.NotAuthorizedException; -import io.milton.resource.MakeCollectionableResource; -import io.milton.resource.PutableResource; -import org.apache.jena.query.Dataset; -import org.apache.jena.rdf.model.Model; -import org.apache.jena.sparql.core.DatasetGraphFactory; -import org.apache.jena.sparql.util.Context; -import org.eclipse.jetty.server.Authentication; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - -import java.io.IOException; -import java.sql.SQLException; -import java.util.stream.Collectors; import static io.fairspace.saturn.TestUtils.createTestUser; import static io.fairspace.saturn.TestUtils.loadViewsConfig; @@ -47,6 +48,7 @@ import static io.fairspace.saturn.TestUtils.setupRequestContext; import static io.fairspace.saturn.auth.RequestContext.getCurrentRequest; import static io.fairspace.saturn.services.views.ViewService.USER_DOES_NOT_HAVE_PERMISSIONS_TO_READ_FACETS; + import static org.apache.jena.query.DatasetFactory.wrap; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; diff --git a/projects/saturn/src/test/resources/test-views.yaml b/projects/saturn/src/test/resources/test-views.yaml index 99d271eb4..f378f537e 100644 --- a/projects/saturn/src/test/resources/test-views.yaml +++ b/projects/saturn/src/test/resources/test-views.yaml @@ -2,6 +2,7 @@ views: - name: Subject title: Subjects itemName: Subject + maxDisplayCount: 1 types: - https://institut-curie.org/ontology#Subject columns: @@ -269,6 +270,7 @@ views: - name: Resource title: Collections and files itemName: Name + maxDisplayCount: 5 types: - https://fairspace.nl/ontology#Collection - https://fairspace.nl/ontology#Directory From 01d68e8ab34101b49da115e89f1d95f181d97d57 Mon Sep 17 00:00:00 2001 From: anton Date: Fri, 9 Aug 2024 16:27:53 +0200 Subject: [PATCH 09/11] FAIRSPC-108: code review comments fixes --- .../io/fairspace/saturn/config/ViewsConfig.java | 8 ++++---- .../saturn/services/views/SparqlQueryService.java | 4 ++-- .../saturn/services/views/ViewConfigDto.java | 15 --------------- .../saturn/services/views/ViewService.java | 6 +----- .../services/views/ViewStoreClientFactory.java | 1 + 5 files changed, 8 insertions(+), 26 deletions(-) delete mode 100644 projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java b/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java index 4e1e27cdf..6a9a1dad4 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/config/ViewsConfig.java @@ -11,7 +11,7 @@ public class ViewsConfig { - private Map viewConfig; + private Map nameToViewConfigMap; public static final ObjectMapper MAPPER = new ObjectMapper(new YAMLFactory()); @@ -19,10 +19,10 @@ public class ViewsConfig { public List views = new ArrayList<>(); public Optional getViewConfig(String viewName) { - if (viewConfig == null) { - viewConfig = views.stream().collect(Collectors.toMap(view -> view.name, Function.identity())); + if (nameToViewConfigMap == null) { + nameToViewConfigMap = views.stream().collect(Collectors.toMap(view -> view.name, Function.identity())); } - return Optional.ofNullable(viewConfig.get(viewName)); + return Optional.ofNullable(nameToViewConfigMap.get(viewName)); } public enum ColumnType { diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java index 9103f01b4..ad15561e3 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java @@ -433,11 +433,11 @@ public CountDTO count(CountRequest request) { var count = row.getLiteral("count").getLong(); return new CountDTO(count, false); } else { - return new CountDTO(-1, false); + return new CountDTO(0, false); } }); } catch (QueryCancelledException e) { - return new CountDTO(-1, true); + return new CountDTO(0, true); } } } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java deleted file mode 100644 index a39494a5b..000000000 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewConfigDto.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.fairspace.saturn.services.views; - -import lombok.Value; - -import io.fairspace.saturn.config.ViewsConfig; - -@Value -public class ViewConfigDto { - - long maxDisplayCount; - - public static ViewConfigDto from(ViewsConfig.View config) { - return new ViewConfigDto(config.maxDisplayCount); - } -} diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java index d12c7d51b..cb235eab3 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java @@ -189,11 +189,7 @@ protected List fetchViews() { columns.add(new ColumnDTO(joinView.name + "_" + c.name, c.title, c.type, j.displayIndex)); } } - Long maxDisplayCount = viewsConfig - .getViewConfig(v.name) - .map(t -> t.maxDisplayCount) - .orElse(null); - return new ViewDTO(v.name, v.title, columns, maxDisplayCount); + return new ViewDTO(v.name, v.title, columns, v.maxDisplayCount); }) .collect(toList()); } diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreClientFactory.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreClientFactory.java index 31bfc2a92..0a98e5c4e 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreClientFactory.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewStoreClientFactory.java @@ -76,6 +76,7 @@ public ViewStoreClientFactory(ViewsConfig viewsConfig, Config.ViewDatabase viewD List.of(idColumn(), valueColumn("type", ColumnType.Text), valueColumn("label", ColumnType.Text)))); configuration = new ViewStoreClient.ViewStoreConfiguration(viewsConfig); + // todo: configuration is initialized within the loop below, do the initialization in constructor for (View view : viewsConfig.views) { createOrUpdateView(view); } From d2c8375b8637d6e63c87baf098a50ff3dbfefb57 Mon Sep 17 00:00:00 2001 From: anton Date: Fri, 9 Aug 2024 22:35:31 +0200 Subject: [PATCH 10/11] FAIRSPC-108: code review comments fixes --- .../services/views/SparqlQueryService.java | 192 +++++++++--------- .../saturn/services/views/ViewService.java | 5 +- 2 files changed, 102 insertions(+), 95 deletions(-) diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java index ad15561e3..28c43c85a 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/SparqlQueryService.java @@ -198,100 +198,13 @@ private ValueDTO toValueDTO(RDFNode node) { private Query getQuery(CountRequest request, boolean isCount) { var view = getView(request.getView()); - var builder = new StringBuilder().append("SELECT ").append("%s").append("\nWHERE {\n"); + var builder = new StringBuilder().append("SELECT %s \nWHERE {\n"); // prefix to be added later if (request.getFilters() != null) { - var filters = new ArrayList<>(request.getFilters()); - - filters.stream().filter(f -> f.field.equals("location")).findFirst().ifPresent(locationFilter -> { - filters.remove(locationFilter); - - if (locationFilter.values != null && !locationFilter.values.isEmpty()) { - locationFilter.values.forEach(v -> validateIRI(v.toString())); - var fileLink = view.join.stream() - .filter(v -> v.view.equals(RESOURCES_VIEW)) - .findFirst() - .orElse(null); - if (fileLink != null) { - builder.append("FILTER EXISTS {\n") - .append("?file fs:belongsTo* ?location .\n FILTER (?location IN (") - .append(locationFilter.values.stream() - .map(v -> "<" + v + ">") - .collect(joining(", "))) - .append("))\n ?file <") - .append(fileLink.on) - .append("> ?") - .append(view.name) - .append(" . \n") - .append("}\n"); - } else { - builder.append("?") - .append(view.name) - .append(" fs:belongsTo* ?location .\n FILTER (?location IN (") - .append(locationFilter.values.stream() - .map(v -> "<" + v + ">") - .collect(joining(", "))) - .append("))\n"); - } - } - }); - - filters.stream() - .map(f -> f.field) - .sorted(comparing(field -> field.contains("_") ? getColumn(field).priority : 0)) - .map(field -> field.split("_")[0]) - .distinct() - .forEach(entity -> { - if (!entity.equals(view.name)) { - builder.append("FILTER EXISTS {\n"); - - var join = view.join.stream() - .filter(j -> j.view.equals(entity)) - .findFirst() - .orElseThrow(() -> new RuntimeException("Unknown view: " + entity)); - - builder.append("?") - .append(view.name) - .append(join.reverse ? " ^<" : " <") - .append(join.on) - .append("> ?") - .append(join.view) - .append(" .\n"); - } - - request.getFilters().stream() - .filter(f -> f.getField().startsWith(entity)) - .sorted(comparing(f -> f.field.contains("_") ? getColumn(f.field).priority : 0)) - .forEach(f -> { - String condition, property, field; - if (f.getField().equals(entity)) { - field = f.field + "_id"; - property = RDFS.label.toString(); - condition = toFilterString(f, ColumnType.Identifier, field); - } else { - field = f.field; - property = getColumn(f.field).source; - condition = toFilterString(f, getColumn(f.field).type, f.field); - } - if (condition != null) { - builder.append("?") - .append(entity) - .append(" <") - .append(property) - .append("> ?") - .append(field) - .append(" .\n") - .append(condition) - .append(" \n"); - } - }); - if (!entity.equals(view.name)) { - builder.append("}"); - } - builder.append("\n"); - }); + buildAndAddFilterToQuery(request, view, builder); } + // add type condition and check if the entity is not deleted builder.append("?") .append(view.name) .append(" a ?type .\nFILTER (?type IN (") @@ -304,11 +217,108 @@ private Query getQuery(CountRequest request, boolean isCount) { ? transformToCountQuery(builder.toString(), view) : builder.toString().formatted("?" + view.name); - query = "PREFIX fs: <%s>\n\n".formatted(FS.NS) + query; // adding prefix to the query + // adding prefix at the top of the query + // prefix cannot be added earlier due to transformation for the count query + query = "PREFIX fs: <%s>\n\n".formatted(FS.NS) + query; return QueryFactory.create(query); } + private void buildAndAddFilterToQuery(CountRequest request, View view, StringBuilder builder) { + var filters = new ArrayList<>(request.getFilters()); + filters.stream().filter(f -> f.field.equals("location")).findFirst().ifPresent(locationFilter -> { + filters.remove(locationFilter); + + if (locationFilter.values != null && !locationFilter.values.isEmpty()) { + locationFilter.values.forEach(v -> validateIRI(v.toString())); + var fileLink = view.join.stream() + .filter(v -> v.view.equals(RESOURCES_VIEW)) + .findFirst() + .orElse(null); + if (fileLink != null) { + builder.append("FILTER EXISTS {\n") + .append("?file fs:belongsTo* ?location .\n FILTER (?location IN (") + .append(locationFilter.values.stream() + .map(v -> "<" + v + ">") + .collect(joining(", "))) + .append("))\n ?file <") + .append(fileLink.on) + .append("> ?") + .append(view.name) + .append(" . \n") + .append("}\n"); + } else { + builder.append("?") + .append(view.name) + .append(" fs:belongsTo* ?location .\n FILTER (?location IN (") + .append(locationFilter.values.stream() + .map(v -> "<" + v + ">") + .collect(joining(", "))) + .append("))\n"); + } + } + }); + + filters.stream() + .map(f -> f.field) + .sorted(comparing(field -> field.contains("_") ? getColumn(field).priority : 0)) + .map(field -> field.split("_")[0]) + .distinct() + .forEach(entity -> { + if (!entity.equals(view.name)) { + builder.append("FILTER EXISTS {\n"); + + var join = view.join.stream() + .filter(j -> j.view.equals(entity)) + .findFirst() + .orElseThrow(() -> new RuntimeException("Unknown view: " + entity)); + + builder.append("?") + .append(view.name) + .append(join.reverse ? " ^<" : " <") + .append(join.on) + .append("> ?") + .append(join.view) + .append(" .\n"); + } + + request.getFilters().stream() + .filter(f -> f.getField().startsWith(entity)) + .sorted(comparing(f -> f.field.contains("_") ? getColumn(f.field).priority : 0)) + .forEach(f -> { + String condition, property, field; + if (f.getField().equals(entity)) { + field = f.field + "_id"; + property = RDFS.label.toString(); + condition = toFilterString(f, ColumnType.Identifier, field); + } else { + field = f.field; + property = getColumn(f.field).source; + condition = toFilterString(f, getColumn(f.field).type, f.field); + } + if (condition != null) { + builder.append("?") + .append(entity) + .append(" <") + .append(property) + .append("> ?") + .append(field) + .append(" .\n") + .append(condition) + .append(" \n"); + } + }); + if (!entity.equals(view.name)) { + builder.append("}"); + } + builder.append("\n"); + }); + } + + /** + * The way count is calculated depends on the view configuration. If the view has a maxDisplayCount set, + * the count query will be limited to that number of results. Otherwise, the count query will return the total + */ private String transformToCountQuery(String queryTemplate, View view) { String query; if (view.maxDisplayCount == null) { diff --git a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java index cb235eab3..22599e91c 100644 --- a/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java +++ b/projects/saturn/src/main/java/io/fairspace/saturn/services/views/ViewService.java @@ -171,10 +171,7 @@ protected List fetchViews() { columns.add(new ColumnDTO(v.name + "_" + c.name, c.title, c.type, c.displayIndex)); } for (var j : v.join) { - var joinView = viewsConfig.views.stream() - .filter(view -> view.name.equalsIgnoreCase(j.view)) - .findFirst() - .orElse(null); + var joinView = viewsConfig.getViewConfig(j.view).orElse(null); if (joinView == null) { continue; } From d2fdb000bf2c12d89880b2dfb30ee054524035b1 Mon Sep 17 00:00:00 2001 From: anton Date: Mon, 12 Aug 2024 10:56:10 +0200 Subject: [PATCH 11/11] FAIRSPC-108: updated readme for count endpoint --- README.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.adoc b/README.adoc index 5db4c9ffb..f4f3ab3e6 100644 --- a/README.adoc +++ b/README.adoc @@ -1239,7 +1239,7 @@ curl -X POST -H 'Content-type: application/json' -H 'Accept: application/json' - |=== 3+| ``POST /api/views/count`` -3+| Count rows of a view matching request filters. +3+| Count rows of a view matching request filters. If `maxDisplayCount` configured in the `views.yaml` for a view, then the count for the view is limited by this value if total count exceeds it. Otherwise, the total count is returned. 3+| _Parameters:_ | ``view`` | string