Skip to content

Commit

Permalink
Sorting over binary metadata / micronode content (#1599)
Browse files Browse the repository at this point in the history 
* SUP-16258: Sorting removes permission checks

* UT. Fixes.

* Tests. More fixes.

* NULL order checks ignore

* Unneeded test

* Changelog

* Non content sorting fix

* Adapt use name param

* SQL injection prevention (ugly)

* Correct check for SQL injected sort param

* Sanity fix

* Fix paging of sorted data

* Total count

* Nicronode/binary/noderef sorting fixes

* Match the filter/sort fields with type provider values

* More sorting tests

* Micronode/binary sort

* More missed permission checks

* More occurrences of manual count fetch

* Fix paging fetch

* Add missing test

* No demo in Jenkinsfiles

* Update changelog items

* More UTs

* Fix for the node field sorting

* Fix incorrect permission application

* Permission regression

* Prepare for next feature version

* Added request cancelation into interface; Fixed interface naming; Fixed publishing scripts

* Corrected Mesh-JS Versions

* Regression fix

* Update

* Regression fixes

---------

Co-authored-by: Norbert Pomaroli <n.pomaroli@gentics.com>
Co-authored-by: Decker Dominik <d.decker@gentics.com>
  • Loading branch information
@plyhun @npomaroli @deckdom
3 people authored Nov 6, 2024
1 parent 5f26fbe commit 19e3959
Show file tree
Hide file tree
Showing 33 changed files with 581 additions and 81 deletions.
2 changes: 1 addition & 1 deletion bom/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<graphql.version>21.5</graphql.version>
<graphql-scalars.version>21.0</graphql-scalars.version>
<graphql-dataloader.version>3.2.2</graphql-dataloader.version>
<graphql-filter.version>3.0.5</graphql-filter.version>
<graphql-filter.version>3.0.6</graphql-filter.version>
<pf4j.version>3.11.0</pf4j.version>
<asm.version>3.3.1</asm.version>
<spring.security.version>5.5.7</spring.security.version>
Expand Down
1 change: 1 addition & 0 deletions changelog/src/changelog/entries/2024/04/7757.GPU-1413.enhancement
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Core: Now it is possible to sort over binary metadata, node reference and micronode non-list fields.
16 changes: 16 additions & 0 deletions mdm/orientdb-api/src/main/java/com/gentics/mesh/core/data/MeshVertex.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
import com.gentics.graphqlfilter.filter.operation.FilterOperation;
import com.gentics.mesh.context.BulkActionContext;
import com.gentics.mesh.context.impl.DummyBulkActionContext;
import com.gentics.mesh.core.data.dao.PersistingRootDao;
import com.gentics.mesh.core.data.perm.InternalPermission;
import com.gentics.mesh.core.data.user.HibUser;
import com.gentics.mesh.core.rest.common.ContainerType;
import com.gentics.mesh.graphdb.model.MeshElement;
import com.gentics.mesh.madl.frame.VertexFrame;
import com.gentics.mesh.parameter.PagingParameters;
import com.tinkerpop.blueprints.Vertex;

/**
Expand Down Expand Up @@ -96,4 +98,18 @@ default void delete() {
default String parseFilter(FilterOperation<?> filter, ContainerType ctype, HibUser user, InternalPermission permission, Optional<String> maybeOwner) {
return parseFilter(filter, ctype) + permissionFilter(user, permission, maybeOwner, Optional.ofNullable(ctype)).map(permFilter -> " AND " + permFilter).orElse(StringUtils.EMPTY);
}

/**
* Set up native permission filter if the sorting is requested, since the sorting forces the native SQL data fetcher.
*
* @param pagingInfo
* @param user
* @param permission
* @param maybeOwner
* @param containerType
* @return
*/
default Optional<String> permissionFilterIfRequired(PagingParameters pagingInfo, HibUser user, InternalPermission permission, Optional<String> maybeOwner, Optional<ContainerType> containerType) {
return PersistingRootDao.shouldSort(pagingInfo) ? permissionFilter(user, permission, maybeOwner, containerType) : Optional.empty();
}
}
43 changes: 42 additions & 1 deletion ...orientdb-api/src/main/java/com/gentics/mesh/core/data/relationship/GraphRelationship.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.gentics.mesh.core.data.relationship;

import java.util.Optional;

/**
* A class for determining the relationship of a vertex class over the edge labels and edge properties
*
Expand All @@ -12,6 +14,8 @@ public class GraphRelationship {
private final Class<?> relatedVertexClass;
private final String edgeFieldName;
private final String defaultEdgeFieldFilterValue;
private final Optional<String[]> maybeEdgeLevelFields;
private final boolean skipMapping;

/**
* Constructor
Expand All @@ -20,13 +24,43 @@ public class GraphRelationship {
* @param relatedVertexClass a vertex class name, where the edge goes to
* @param edgeFieldName name of an edge property, that is being used in the filtering. If null/empty, no edge property filtering used
* @param defaultEdgeFieldFilterValue default value for the edge property filtering. Unused, if edgeFieldName is null/empty.
* @param maybeEdgeLevelFields optional list of entity fields, that are applied on an input edge level.
*/
public GraphRelationship(String edgeName, Class<?> relatedVertexClass, String edgeFieldName,
String defaultEdgeFieldFilterValue) {
String defaultEdgeFieldFilterValue, Optional<String[]> maybeEdgeLevelFields) {
this(edgeName, relatedVertexClass, edgeFieldName, defaultEdgeFieldFilterValue, maybeEdgeLevelFields, false);
}

/**
* Constructor
*
* @param edgeName an edge label
* @param relatedVertexClass a vertex class name, where the edge goes to
* @param edgeFieldName name of an edge property, that is being used in the filtering. If null/empty, no edge property filtering used
* @param defaultEdgeFieldFilterValue default value for the edge property filtering. Unused, if edgeFieldName is null/empty.
* @param maybeEdgeLevelFields optional list of entity fields, that are applied on an input edge level.
* @param skipMapping this mapping is artificial and should not be used in either filtering or sorting.
*/
public GraphRelationship(String edgeName, Class<?> relatedVertexClass, String edgeFieldName,
String defaultEdgeFieldFilterValue, Optional<String[]> maybeEdgeLevelFields, boolean skipMapping) {
this.edgeName = edgeName;
this.relatedVertexClass = relatedVertexClass;
this.edgeFieldName = edgeFieldName;
this.defaultEdgeFieldFilterValue = defaultEdgeFieldFilterValue;
this.maybeEdgeLevelFields = maybeEdgeLevelFields;
this.skipMapping = skipMapping;
}
/**
* Constructor with optional edge fields.
*
* @param edgeName an edge label
* @param relatedVertexClass a vertex class name, where the edge goes to
* @param edgeFieldName name of an edge property, that is being used in the filtering. If null/empty, no edge property filtering used
* @param defaultEdgeFieldFilterValue default value for the edge property filtering. Unused, if edgeFieldName is null/empty.
*/
public GraphRelationship(String edgeName, Class<?> relatedVertexClass, String edgeFieldName,
String defaultEdgeFieldFilterValue) {
this(edgeName, relatedVertexClass, edgeFieldName, defaultEdgeFieldFilterValue, Optional.empty());
}
public String getEdgeName() {
return edgeName;
Expand All @@ -40,4 +74,11 @@ public String getEdgeFieldName() {
public String getDefaultEdgeFieldFilterValue() {
return defaultEdgeFieldFilterValue;
}
public Optional<String[]> getMaybeEdgeLevelFields() {
return maybeEdgeLevelFields;
}

public boolean isSkipMapping() {
return skipMapping;
}
}
26 changes: 24 additions & 2 deletions ...rientdb-api/src/main/java/com/gentics/mesh/core/data/relationship/GraphRelationships.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

import com.gentics.madl.index.IndexHandler;
import com.gentics.madl.type.TypeHandler;
Expand All @@ -19,11 +20,32 @@ public class GraphRelationships {
* Add a relation between entities through an edge.
*/
public static <K extends MeshVertex, V extends MeshVertex> void addRelation(Class<K> keyClass, Class<V> valueClass, String mappingName, String relationName, String edgeFieldName, String defaultEdgeFieldFilterValue) {
addRelation(keyClass, valueClass, mappingName, relationName, edgeFieldName, defaultEdgeFieldFilterValue, Optional.empty(), false);
}

/**
* Add a relation between entities through an edge, with optional edge fields.
*/
public static <K extends MeshVertex, V extends MeshVertex> void addRelation(Class<K> keyClass, Class<V> valueClass, String mappingName, String relationName, String edgeFieldName, String defaultEdgeFieldFilterValue, Optional<String[]> maybeEdgeLevelFieldNames, boolean skipMapping) {
Map<String, GraphRelationship> relations = VERTEX_RELATIONS.getOrDefault(keyClass, new HashMap<>());
relations.put(mappingName, new GraphRelationship(relationName, valueClass, edgeFieldName, defaultEdgeFieldFilterValue));
relations.put(mappingName, new GraphRelationship(relationName, valueClass, edgeFieldName, defaultEdgeFieldFilterValue, maybeEdgeLevelFieldNames, skipMapping));
VERTEX_RELATIONS.put(keyClass, relations);
}

/**
* Add a relation between entities through UUID.
*
* @param <K>
* @param <V>
* @param keyClass
* @param valueClass
* @param mappingName
* @param relationName
*/
public static <K extends MeshVertex, V extends MeshVertex> void addUnmappedRelation(Class<K> keyClass, Class<V> valueClass, String mappingName) {
addRelation(keyClass, valueClass, mappingName, MeshVertex.UUID_KEY, mappingName, null, Optional.empty(), true);
}

/**
* Add a relation between entities through UUID.
*
Expand All @@ -35,7 +57,7 @@ public static <K extends MeshVertex, V extends MeshVertex> void addRelation(Clas
* @param relationName
*/
public static <K extends MeshVertex, V extends MeshVertex> void addRelation(Class<K> keyClass, Class<V> valueClass, String mappingName) {
addRelation(keyClass, valueClass, mappingName, MeshVertex.UUID_KEY, mappingName, null);
addRelation(keyClass, valueClass, mappingName, MeshVertex.UUID_KEY, mappingName, null, Optional.empty(), false);
}

/**
Expand Down
57 changes: 44 additions & 13 deletions mdm/orientdb-api/src/main/java/com/gentics/mesh/graphdb/MeshOrientGraphQuery.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.gentics.mesh.graphdb;

import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -204,12 +205,24 @@ protected void buildOrderFieldRequest(StringBuilder text, boolean isEdgeRequest,
String sanitizedPart = sanitizeInput(sortParts[0]);
String[] pathParts = sanitizedPart.split("\\.");
text.append(", ");
Map<String, GraphRelationship> relation = GraphRelationships.findRelation(currentMapping);
Map<String, GraphRelationship> relation = null;
Optional<String[]> maybeEdgeLevelFields = Optional.empty();
for (int i = 0; i < pathParts.length; i++) {
String pathPart = pathParts[i];
relation = GraphRelationships.findRelation(currentMapping);
Map<String, GraphRelationship> localRelation = relation;
Optional<String> maybeFieldTypeMapping = Optional.ofNullable(localRelation)
.flatMap(relation1 -> relation1.keySet().stream()
.filter(key -> key.startsWith("*") && pathPart.matches("[\\S]" + key))
.findAny());
if (relation != null && !sanitizedPart.endsWith(pathPart)
&& (relation.containsKey(pathPart) || (relation.containsKey("*")))) {
GraphRelationship relationMapping = relation.get(pathPart) != null ? relation.get(pathPart) : relation.get("*");
&& (relation.containsKey(pathPart) || (relation.containsKey("*") || maybeFieldTypeMapping.isPresent()))) {
GraphRelationship relationMapping = maybeFieldTypeMapping.map(key -> localRelation.get(key))
.or(() -> Optional.ofNullable(localRelation.get(pathPart))).orElseGet(() -> localRelation.get("*"));
if (relationMapping.isSkipMapping()) {
continue;
}
maybeEdgeLevelFields = relationMapping.getMaybeEdgeLevelFields();
if (useEdgeFilters && relationMapping != null) {
if (MeshVertex.UUID_KEY.equals(relationMapping.getEdgeName())) {
String partName = pathParts.length > 1 ? pathParts[1] : pathPart;
Expand All @@ -230,6 +243,10 @@ protected void buildOrderFieldRequest(StringBuilder text, boolean isEdgeRequest,
escapeFieldNameIfRequired(text, relationMapping.getEdgeFieldName());
text.append(")");
} else {
String localPathPart = pathPart;
if (maybeFieldTypeMapping.isPresent()) {
localPathPart = pathPart.split("-")[0];
}
if (i < 1 && isEdgeRequest) {
text.append(relationDirection.name().toLowerCase());
text.append("V().");
Expand All @@ -240,10 +257,12 @@ protected void buildOrderFieldRequest(StringBuilder text, boolean isEdgeRequest,
text.append("')[");
text.append(relationMapping.getEdgeFieldName());
text.append("='");
text.append(relation.get(pathPart) != null ? relationMapping.getDefaultEdgeFieldFilterValue() : pathPart);
text.append(localRelation.get(localPathPart) != null ? relationMapping.getDefaultEdgeFieldFilterValue() : localPathPart);
text.append("'].");
text.append(vertexLookupDirOpposite.name().toLowerCase());
text.append("V()");
if (maybeEdgeLevelFields.isEmpty()) {
text.append(vertexLookupDirOpposite.name().toLowerCase());
text.append("V()");
}
}
} else {
if (i < 1 && isEdgeRequest) {
Expand All @@ -255,19 +274,31 @@ protected void buildOrderFieldRequest(StringBuilder text, boolean isEdgeRequest,
escapeFieldNameIfRequired(text, relationMapping.getEdgeName());
text.append(")");
}
text.append("[0].");
currentMapping = relationMapping.getRelatedVertexClass();
} else if (i == 1 && "fields".equals(pathParts[0]) && pathParts.length > 2) {
// skip the schema name
if (maybeEdgeLevelFields.isEmpty()) {
text.append("[0].");
currentMapping = relationMapping.getRelatedVertexClass();
}
} else if ((i > 0 && "fields".equals(pathParts[i-1])) || (i < (pathParts.length-1) && "fields".equals(pathParts[i+1])) && pathParts.length > 2) {
// skip the schema name or the self-reference
continue;
} else {
if (i < 1 && isEdgeRequest) {
text.append(relationDirection.name().toLowerCase());
text.append("V().");
}
text.append("`");
text.append(pathPart);
text.append("`");
maybeEdgeLevelFields
.flatMap(edgeLevelFields -> Arrays.stream(edgeLevelFields).filter(field -> field.equals(pathPart)).findAny())
.ifPresentOrElse(field -> {
text.append("`");
text.append(field);
text.append("`");
text.append("[0]");
}, () -> {
text.append("`");
text.append(pathPart);
text.append("`");
});
maybeEdgeLevelFields = Optional.empty();
}
}
text.append(" as ");
Expand Down
27 changes: 27 additions & 0 deletions .../main/java/com/gentics/mesh/core/data/container/impl/AbstractGraphFieldContainerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR;

import java.util.List;
import java.util.Optional;

import org.apache.commons.lang3.StringUtils;

import com.gentics.madl.index.IndexHandler;
import com.gentics.madl.type.TypeHandler;
import com.gentics.mesh.context.BulkActionContext;
import com.gentics.mesh.context.InternalActionContext;
import com.gentics.mesh.core.data.GraphFieldContainer;
Expand All @@ -20,6 +23,7 @@
import com.gentics.mesh.core.data.binary.HibBinary;
import com.gentics.mesh.core.data.binary.HibImageVariant;
import com.gentics.mesh.core.data.binary.impl.BinaryGraphFieldVariantImpl;
import com.gentics.mesh.core.data.binary.impl.BinaryImpl;
import com.gentics.mesh.core.data.impl.GraphFieldTypes;
import com.gentics.mesh.core.data.node.HibNode;
import com.gentics.mesh.core.data.node.Micronode;
Expand Down Expand Up @@ -60,7 +64,10 @@
import com.gentics.mesh.core.data.node.field.nesting.MicronodeGraphField;
import com.gentics.mesh.core.data.node.field.nesting.NodeGraphField;
import com.gentics.mesh.core.data.node.impl.MicronodeImpl;
import com.gentics.mesh.core.data.node.impl.NodeImpl;
import com.gentics.mesh.core.data.relationship.GraphRelationships;
import com.gentics.mesh.core.data.s3binary.S3HibBinary;
import com.gentics.mesh.core.data.s3binary.impl.S3BinaryImpl;
import com.gentics.mesh.core.data.schema.HibMicroschemaVersion;
import com.gentics.mesh.core.rest.error.GenericRestException;
import com.gentics.mesh.core.rest.node.FieldMap;
Expand All @@ -78,6 +85,26 @@
public abstract class AbstractGraphFieldContainerImpl extends AbstractBasicGraphFieldContainerImpl implements GraphFieldContainer {
static final Logger log = LoggerFactory.getLogger(AbstractGraphFieldContainerImpl.class);

/**
* Initialize the vertex type and indices.
*
* @param type
* @param index
*/
public static void init(TypeHandler type, IndexHandler index, Class<? extends AbstractGraphFieldContainerImpl> cls) {
GraphRelationships.addRelation(cls, MicronodeImpl.class, "*-micronode", HAS_FIELD, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
GraphRelationships.addRelation(cls, NodeImpl.class, "*-node", HAS_FIELD, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
GraphRelationships.addRelation(cls, BinaryImpl.class, "*-binary", HAS_FIELD, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY,
Optional.of(new String[] { BinaryGraphField.BINARY_IMAGE_DOMINANT_COLOR_PROPERTY_KEY, BinaryGraphField.BINARY_CONTENT_TYPE_PROPERTY_KEY, BinaryGraphField.BINARY_FILENAME_PROPERTY_KEY }), false);
GraphRelationships.addRelation(cls, S3BinaryImpl.class, "*-s3binary", HAS_FIELD, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
GraphRelationships.addRelation(cls, StringGraphFieldListImpl.class, "*-stringlist", HAS_LIST, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
GraphRelationships.addRelation(cls, NumberGraphFieldListImpl.class, "*-numberlist", HAS_LIST, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
GraphRelationships.addRelation(cls, BooleanGraphFieldListImpl.class, "*-booleanlist", HAS_LIST, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
GraphRelationships.addRelation(cls, HtmlGraphFieldListImpl.class, "*-htmllist", HAS_LIST, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
GraphRelationships.addRelation(cls, NodeGraphFieldListImpl.class, "*-nodelist", HAS_LIST, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
GraphRelationships.addRelation(cls, MicronodeGraphFieldListImpl.class, "*-micronodelist", HAS_LIST, GraphField.FIELD_KEY_PROPERTY_KEY, StringUtils.EMPTY);
}

/**
* Return the parent node of the field container.
*
Expand Down
Loading

0 comments on commit 19e3959

Please sign in to comment.