Skip to content

Commit 395ebf0

Browse files
harveymmaundersharveymmaunders
committed
refactor(calm-hub): improve decorator query validation and storage layer
Move query parameter validation from custom methods to Jakarta Bean Validation annotations (@SiZe, @pattern). Refactor decorator store implementations into smaller, more maintainable methods. - Replace sanitizeQueryParam with @pattern validation - Add QUERY_PARAM_NO_WHITESPACE_REGEX constant (^[A-Za-z0-9_/-]+$) - Break down getDecoratorsForNamespace into focused methods - Update tests for new validation behavior Refs #2168
1 parent d4685a0 commit 395ebf0

File tree

5 files changed

+218
-120
lines changed

5 files changed

+218
-120
lines changed

‎calm-hub/src/main/java/org/finos/calm/resources/DecoratorResource.java‎

Lines changed: 6 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import jakarta.inject.Inject;
44
import jakarta.validation.constraints.Pattern;
5+
import jakarta.validation.constraints.Size;
56
import jakarta.ws.rs.GET;
67
import jakarta.ws.rs.Path;
78
import jakarta.ws.rs.PathParam;
@@ -20,6 +21,8 @@
2021

2122
import static org.finos.calm.resources.ResourceValidationConstants.NAMESPACE_MESSAGE;
2223
import static org.finos.calm.resources.ResourceValidationConstants.NAMESPACE_REGEX;
24+
import static org.finos.calm.resources.ResourceValidationConstants.QUERY_PARAM_NO_WHITESPACE_MESSAGE;
25+
import static org.finos.calm.resources.ResourceValidationConstants.QUERY_PARAM_NO_WHITESPACE_REGEX;
2326

2427
/**
2528
* Resource for managing decorators in a given namespace
@@ -53,44 +56,14 @@ public DecoratorResource(DecoratorStore decoratorStore) {
5356
@PermittedScopes({CalmHubScopes.ARCHITECTURES_ALL, CalmHubScopes.ARCHITECTURES_READ})
5457
public Response getDecoratorsForNamespace(
5558
@PathParam("namespace") @Pattern(regexp = NAMESPACE_REGEX, message = NAMESPACE_MESSAGE) String namespace,
56-
@QueryParam("target") String target,
57-
@QueryParam("type") String type
59+
@QueryParam("target") @Size(max = 500) @Pattern(regexp = QUERY_PARAM_NO_WHITESPACE_REGEX, message = QUERY_PARAM_NO_WHITESPACE_MESSAGE) String target,
60+
@QueryParam("type") @Size(max = 100) @Pattern(regexp = QUERY_PARAM_NO_WHITESPACE_REGEX, message = QUERY_PARAM_NO_WHITESPACE_MESSAGE) String type
5861
) {
5962
try {
60-
// Sanitize query parameters
61-
String sanitizedTarget = sanitizeQueryParam(target, 500);
62-
String sanitizedType = sanitizeQueryParam(type, 100);
63-
64-
return Response.ok(new ValueWrapper<>(decoratorStore.getDecoratorsForNamespace(namespace, sanitizedTarget, sanitizedType))).build();
63+
return Response.ok(new ValueWrapper<>(decoratorStore.getDecoratorsForNamespace(namespace, target, type))).build();
6564
} catch (NamespaceNotFoundException e) {
6665
logger.error("Invalid namespace [{}] when retrieving decorators", namespace, e);
6766
return CalmResourceErrorResponses.invalidNamespaceResponse(namespace);
68-
} catch (IllegalArgumentException e) {
69-
logger.error("Invalid query parameter when retrieving decorators for namespace [{}]", namespace, e);
70-
return Response.status(Response.Status.BAD_REQUEST)
71-
.entity(e.getMessage())
72-
.build();
7367
}
7468
}
75-
76-
/**
77-
* Sanitizes a query parameter by trimming and validating length
78-
*
79-
* @param param the parameter to sanitize
80-
* @param maxLength maximum allowed length
81-
* @return null if param is null/empty, otherwise the trimmed value
82-
* @throws IllegalArgumentException if parameter exceeds max length
83-
*/
84-
private String sanitizeQueryParam(String param, int maxLength) {
85-
if (param == null || param.trim().isEmpty()) {
86-
return null;
87-
}
88-
89-
String trimmed = param.trim();
90-
if (trimmed.length() > maxLength) {
91-
throw new IllegalArgumentException("Query parameter exceeds maximum length of " + maxLength);
92-
}
93-
94-
return trimmed;
95-
}
9669
}

‎calm-hub/src/main/java/org/finos/calm/resources/ResourceValidationConstants.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ public class ResourceValidationConstants {
1010
public static final String DOMAIN_NAME_MESSAGE = "domain name must match pattern '^[A-Za-z0-9-]+$'";
1111
public static final String VERSION_REGEX = "^(0|[1-9][0-9]*)[-.]?(0|[1-9][0-9]*)[-.]?(0|[1-9][0-9]*)$";
1212
public static final String VERSION_MESSAGE = "version must match pattern '^(0|[1-9][0-9]*)[-.]?(0|[1-9][0-9]*)[-.]?(0|[1-9][0-9]*)$'";
13+
public static final String QUERY_PARAM_NO_WHITESPACE_REGEX = "^[A-Za-z0-9_/-]+$";
14+
public static final String QUERY_PARAM_NO_WHITESPACE_MESSAGE = "Query parameter must contain only alphanumeric characters, hyphens, underscores, and forward slashes";
1315
public static final PolicyFactory STRICT_SANITIZATION_POLICY = new HtmlPolicyBuilder().toFactory();
1416

1517
}

‎calm-hub/src/main/java/org/finos/calm/store/mongo/MongoDecoratorStore.java‎

Lines changed: 78 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -34,58 +34,107 @@ public MongoDecoratorStore(MongoDatabase database, MongoNamespaceStore namespace
3434

3535
@Override
3636
public List<Integer> getDecoratorsForNamespace(String namespace, String target, String type) throws NamespaceNotFoundException {
37+
validateNamespace(namespace);
38+
39+
Document namespaceDocument = fetchNamespaceDocument(namespace);
40+
if (namespaceDocument == null || namespaceDocument.isEmpty()) {
41+
LOG.debug("No decorators found for namespace '{}'", namespace);
42+
return List.of();
43+
}
44+
45+
List<Document> decorators = extractDecorators(namespaceDocument, namespace);
46+
if (decorators.isEmpty()) {
47+
return List.of();
48+
}
49+
50+
List<Integer> decoratorIds = filterDecorators(decorators, target, type);
51+
52+
LOG.debug("Retrieved {} decorators for namespace '{}' with filters (target: {}, type: {})",
53+
decoratorIds.size(), namespace, target, type);
54+
return decoratorIds;
55+
}
56+
57+
/**
58+
* Validates that the namespace exists, throwing an exception if it doesn't
59+
*/
60+
private void validateNamespace(String namespace) throws NamespaceNotFoundException {
3761
if (!namespaceStore.namespaceExists(namespace)) {
3862
LOG.warn("Namespace '{}' not found when retrieving decorators", namespace);
3963
throw new NamespaceNotFoundException();
4064
}
65+
}
4166

42-
Document namespaceDocument = decoratorCollection.find(Filters.eq("namespace", namespace)).first();
43-
44-
// Protects from an unpopulated mongo collection
45-
if (namespaceDocument == null || namespaceDocument.isEmpty()) {
46-
LOG.debug("No decorators found for namespace '{}'", namespace);
47-
return List.of();
48-
}
67+
/**
68+
* Fetches the namespace document from MongoDB
69+
*/
70+
private Document fetchNamespaceDocument(String namespace) {
71+
return decoratorCollection.find(Filters.eq("namespace", namespace)).first();
72+
}
4973

74+
/**
75+
* Extracts the list of decorators from the namespace document
76+
*/
77+
private List<Document> extractDecorators(Document namespaceDocument, String namespace) {
5078
List<Document> decorators = namespaceDocument.getList("decorators", Document.class);
5179
if (decorators == null || decorators.isEmpty()) {
5280
LOG.debug("Decorators list is empty for namespace '{}'", namespace);
5381
return List.of();
5482
}
83+
return decorators;
84+
}
5585

86+
/**
87+
* Filters decorators based on target and type criteria
88+
*/
89+
private List<Integer> filterDecorators(List<Document> decorators, String target, String type) {
5690
List<Integer> decoratorIds = new ArrayList<>();
91+
5792
for (Document decoratorDoc : decorators) {
5893
Integer decoratorId = decoratorDoc.getInteger("decoratorId");
5994
if (decoratorId == null) {
6095
continue;
6196
}
6297

63-
// Apply filters if provided
6498
Document decorator = decoratorDoc.get("decorator", Document.class);
65-
if (decorator != null) {
66-
// Filter by type if provided
67-
if (type != null && !type.isEmpty()) {
68-
String decoratorType = decorator.getString("type");
69-
if (decoratorType == null || !decoratorType.equals(type)) {
70-
continue;
71-
}
72-
}
73-
74-
// Filter by target if provided
75-
if (target != null && !target.isEmpty()) {
76-
@SuppressWarnings("unchecked")
77-
List<String> targets = (List<String>) decorator.get("target");
78-
if (targets == null || !targets.contains(target)) {
79-
continue;
80-
}
81-
}
99+
if (decorator != null && matchesFilters(decorator, target, type)) {
100+
decoratorIds.add(decoratorId);
101+
} else if (decorator == null) {
102+
decoratorIds.add(decoratorId);
82103
}
104+
}
105+
106+
return decoratorIds;
107+
}
108+
109+
/**
110+
* Checks if a decorator matches the provided filters
111+
*/
112+
private boolean matchesFilters(Document decorator, String target, String type) {
113+
return matchesTypeFilter(decorator, type) && matchesTargetFilter(decorator, target);
114+
}
83115

84-
decoratorIds.add(decoratorId);
116+
/**
117+
* Checks if the decorator matches the type filter (if provided)
118+
*/
119+
private boolean matchesTypeFilter(Document decorator, String type) {
120+
if (type == null || type.isEmpty()) {
121+
return true;
85122
}
123+
124+
String decoratorType = decorator.getString("type");
125+
return decoratorType != null && decoratorType.equals(type);
126+
}
86127

87-
LOG.debug("Retrieved {} decorators for namespace '{}' with filters (target: {}, type: {})",
88-
decoratorIds.size(), namespace, target, type);
89-
return decoratorIds;
128+
/**
129+
* Checks if the decorator matches the target filter (if provided)
130+
*/
131+
private boolean matchesTargetFilter(Document decorator, String target) {
132+
if (target == null || target.isEmpty()) {
133+
return true;
134+
}
135+
136+
@SuppressWarnings("unchecked")
137+
List<String> targets = (List<String>) decorator.get("target");
138+
return targets != null && targets.contains(target);
90139
}
91140
}

‎calm-hub/src/main/java/org/finos/calm/store/nitrite/NitriteDecoratorStore.java‎

Lines changed: 83 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -45,65 +45,115 @@ public NitriteDecoratorStore(@StandaloneQualifier Nitrite db, NitriteNamespaceSt
4545

4646
@Override
4747
public List<Integer> getDecoratorsForNamespace(String namespace, String target, String type) throws NamespaceNotFoundException {
48+
validateNamespace(namespace);
49+
50+
Document namespaceDoc = fetchNamespaceDocument(namespace);
51+
if (namespaceDoc == null) {
52+
LOG.debug("No decorators found for namespace '{}'", namespace);
53+
return List.of();
54+
}
55+
56+
List<Document> decorators = extractDecorators(namespaceDoc, namespace);
57+
if (decorators.isEmpty()) {
58+
return List.of();
59+
}
60+
61+
List<Integer> decoratorIds = filterDecorators(decorators, target, type);
62+
63+
LOG.debug("Retrieved {} decorators for namespace '{}' with filters (target: {}, type: {})",
64+
decoratorIds.size(), namespace, target, type);
65+
return decoratorIds;
66+
}
67+
68+
/**
69+
* Validates that the namespace exists, throwing an exception if it doesn't
70+
*/
71+
private void validateNamespace(String namespace) throws NamespaceNotFoundException {
4872
if (!namespaceStore.namespaceExists(namespace)) {
4973
LOG.warn("Namespace '{}' not found when retrieving decorators", namespace);
5074
throw new NamespaceNotFoundException();
5175
}
76+
}
5277

78+
/**
79+
* Fetches the namespace document from NitriteDB
80+
*/
81+
private Document fetchNamespaceDocument(String namespace) {
5382
Filter filter = where(NAMESPACE_FIELD).eq(namespace);
54-
Document namespaceDoc = decoratorCollection.find(filter).firstOrNull();
55-
56-
if (namespaceDoc == null) {
57-
LOG.debug("No decorators found for namespace '{}'", namespace);
58-
return List.of();
59-
}
83+
return decoratorCollection.find(filter).firstOrNull();
84+
}
6085

86+
/**
87+
* Extracts the list of decorators from the namespace document
88+
*/
89+
private List<Document> extractDecorators(Document namespaceDoc, String namespace) {
6190
TypeSafeNitriteDocument<Document> typeSafeDoc = new TypeSafeNitriteDocument<>(namespaceDoc, Document.class);
6291
List<Document> decorators = typeSafeDoc.getList(DECORATORS_FIELD);
6392

6493
if (decorators == null || decorators.isEmpty()) {
6594
LOG.debug("Decorators list is empty for namespace '{}'", namespace);
6695
return List.of();
6796
}
97+
return decorators;
98+
}
6899

100+
/**
101+
* Filters decorators based on target and type criteria
102+
*/
103+
private List<Integer> filterDecorators(List<Document> decorators, String target, String type) {
69104
List<Integer> decoratorIds = new ArrayList<>();
105+
70106
for (Document decoratorDoc : decorators) {
71107
Integer decoratorId = decoratorDoc.get(DECORATOR_ID_FIELD, Integer.class);
72108
if (decoratorId == null) {
73109
continue;
74110
}
75111

76-
// Apply filters if provided
77112
Document decorator = decoratorDoc.get("decorator", Document.class);
78-
if (decorator != null) {
79-
// Filter by type if provided
80-
if (type != null && !type.isEmpty()) {
81-
String decoratorType = decorator.get("type", String.class);
82-
if (decoratorType == null || !decoratorType.equals(type)) {
83-
continue;
84-
}
85-
}
86-
87-
// Filter by target if provided
88-
if (target != null && !target.isEmpty()) {
89-
Object targetObj = decorator.get("target");
90-
List<String> targets = null;
91-
if (targetObj instanceof List) {
92-
@SuppressWarnings("unchecked")
93-
List<String> targetList = (List<String>) targetObj;
94-
targets = targetList;
95-
}
96-
if (targets == null || !targets.contains(target)) {
97-
continue;
98-
}
99-
}
113+
if (decorator != null && matchesFilters(decorator, target, type)) {
114+
decoratorIds.add(decoratorId);
115+
} else if (decorator == null) {
116+
decoratorIds.add(decoratorId);
100117
}
118+
}
119+
120+
return decoratorIds;
121+
}
101122

102-
decoratorIds.add(decoratorId);
123+
/**
124+
* Checks if a decorator matches the provided filters
125+
*/
126+
private boolean matchesFilters(Document decorator, String target, String type) {
127+
return matchesTypeFilter(decorator, type) && matchesTargetFilter(decorator, target);
128+
}
129+
130+
/**
131+
* Checks if the decorator matches the type filter (if provided)
132+
*/
133+
private boolean matchesTypeFilter(Document decorator, String type) {
134+
if (type == null || type.isEmpty()) {
135+
return true;
103136
}
137+
138+
String decoratorType = decorator.get("type", String.class);
139+
return decoratorType != null && decoratorType.equals(type);
140+
}
104141

105-
LOG.debug("Retrieved {} decorators for namespace '{}' with filters (target: {}, type: {})",
106-
decoratorIds.size(), namespace, target, type);
107-
return decoratorIds;
142+
/**
143+
* Checks if the decorator matches the target filter (if provided)
144+
*/
145+
private boolean matchesTargetFilter(Document decorator, String target) {
146+
if (target == null || target.isEmpty()) {
147+
return true;
148+
}
149+
150+
Object targetObj = decorator.get("target");
151+
if (!(targetObj instanceof List)) {
152+
return false;
153+
}
154+
155+
@SuppressWarnings("unchecked")
156+
List<String> targets = (List<String>) targetObj;
157+
return targets.contains(target);
108158
}
109159
}

0 commit comments

Comments
 (0)