This is an automated email from the ASF dual-hosted git repository.
sergehuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/master by this push:
new e59989a3f UNOMI-1021: Reject unsupported date aggregate intervals as a
bad request (#929)
e59989a3f is described below
commit e59989a3f5e36fea39ee84b00ee173faa5d24d89
Author: Serge Huber <[email protected]>
AuthorDate: Tue Oct 27 07:51:13 2026 +0100
UNOMI-1021: Reject unsupported date aggregate intervals as a bad request
(#929)
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
.../org/apache/unomi/itests/AggregateQueryIT.java | 65 ++++++++++++++++++++++
.../test/java/org/apache/unomi/itests/AllITs.java | 1 +
.../apache/unomi/itests/CorePersistenceITs.java | 1 +
.../persistence/spi/aggregate/DateAggregate.java | 52 +++++++++++------
.../spi/aggregate/DateAggregateTest.java | 59 ++++++++++++++++++++
.../services/impl/goals/GoalsServiceImpl.java | 6 +-
.../services/impl/queries/QueryServiceImpl.java | 7 ++-
7 files changed, 169 insertions(+), 22 deletions(-)
diff --git a/itests/src/test/java/org/apache/unomi/itests/AggregateQueryIT.java
b/itests/src/test/java/org/apache/unomi/itests/AggregateQueryIT.java
new file mode 100644
index 000000000..e1a922d07
--- /dev/null
+++ b/itests/src/test/java/org/apache/unomi/itests/AggregateQueryIT.java
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.itests;
+
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Aggregate queries through the REST API ({@code POST
/cxs/query/{type}/{property}}).
+ */
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class AggregateQueryIT extends BaseIT {
+
+ @Test
+ public void unsupportedDateIntervalIsABadRequest() throws Exception {
+ assertEquals(400, dateAggregateStatus("2d"));
+ assertEquals(400, dateAggregateStatus("day"));
+ // not text: used to fail with a ClassCastException (HTTP 500)
+ assertEquals(400, dateAggregateStatusForIntervalJson("2"));
+ }
+
+ @Test
+ public void supportedDateIntervalsAreAccepted() throws Exception {
+ assertEquals(200, dateAggregateStatus("1d"));
+ assertEquals(200, dateAggregateStatus("Day"));
+ }
+
+ private int dateAggregateStatus(String interval) throws Exception {
+ return dateAggregateStatusForIntervalJson("\"" + interval + "\"");
+ }
+
+ private int dateAggregateStatusForIntervalJson(String intervalJson) throws
Exception {
+ HttpPost request = new
HttpPost(getFullUrl("/cxs/query/profile/properties.lastVisit"));
+ request.setEntity(new
StringEntity("{\"condition\":{\"type\":\"matchAllCondition\",\"parameterValues\":{}},"
+ +
"\"aggregate\":{\"type\":\"date\",\"parameters\":{\"interval\":" + intervalJson
+ "}}}",
+ ContentType.APPLICATION_JSON));
+ try (CloseableHttpResponse response = executeHttpRequest(request)) {
+ return response.getStatusLine().getStatusCode();
+ }
+ }
+}
diff --git a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
index dfe8b3a9c..4d484e840 100644
--- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
+++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
@@ -41,6 +41,7 @@ import org.junit.runners.Suite.SuiteClasses;
Migrate40IndexMappingIT.class,
Migrate40TenantAssignmentIT.class,
PersistenceServiceIT.class,
+ AggregateQueryIT.class,
BasicIT.class,
ConditionEvaluatorIT.class,
ConditionQueryBuilderIT.class,
diff --git
a/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java
b/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java
index f7a8d8fa0..cb32e892f 100644
--- a/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java
+++ b/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java
@@ -42,6 +42,7 @@ import org.junit.runners.Suite.SuiteClasses;
Migrate40IndexMappingIT.class,
Migrate40TenantAssignmentIT.class,
PersistenceServiceIT.class,
+ AggregateQueryIT.class,
BasicIT.class,
ConditionEvaluatorIT.class,
ConditionQueryBuilderIT.class,
diff --git
a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/aggregate/DateAggregate.java
b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/aggregate/DateAggregate.java
index 86d6b2655..d34217fca 100644
---
a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/aggregate/DateAggregate.java
+++
b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/aggregate/DateAggregate.java
@@ -19,11 +19,13 @@ package org.apache.unomi.persistence.spi.aggregate;
import java.util.Collections;
import java.util.HashMap;
+import java.util.LinkedHashMap;
import java.util.Map;
/**
* Aggregation that buckets documents by time intervals on a date/datetime
field.
- * Supports legacy short interval formats (e.g., {@code 1M}) and new named
formats (e.g., {@code Month}).
+ * Supports legacy short interval formats (e.g., {@code 1M}) and new named
formats (e.g., {@code Month}); no other
+ * interval is accepted. Without an interval, buckets are one month ({@code
1M}).
*/
public class DateAggregate extends BaseAggregate {
private static final String DEFAULT_INTERVAL = "1M";
@@ -31,20 +33,27 @@ public class DateAggregate extends BaseAggregate {
private String interval;
private String format;
- // Bidirectional maps for conversion between interval formats
- private static final Map<String, String> OLD_TO_NEW_FORMAT = Map.ofEntries(
- Map.entry("1s", "Second"),
- Map.entry("1m", "Minute"),
- Map.entry("1h", "Hour"),
- Map.entry("1d", "Day"),
- Map.entry("1w", "Week"),
- Map.entry("1M", "Month"),
- Map.entry("1q", "Quarter"),
- Map.entry("1y", "Year")
- );
+ // Bidirectional maps for conversion between interval formats, in
increasing order
+ private static final Map<String, String> OLD_TO_NEW_FORMAT =
createFormatMap();
private static final Map<String, String> NEW_TO_OLD_FORMAT =
createReverseMap();
+ private static final String SUPPORTED_INTERVALS =
+ String.join(", ", OLD_TO_NEW_FORMAT.keySet()) + ", " +
String.join(", ", OLD_TO_NEW_FORMAT.values());
+
+ private static Map<String, String> createFormatMap() {
+ Map<String, String> formats = new LinkedHashMap<>();
+ formats.put("1s", "Second");
+ formats.put("1m", "Minute");
+ formats.put("1h", "Hour");
+ formats.put("1d", "Day");
+ formats.put("1w", "Week");
+ formats.put("1M", "Month");
+ formats.put("1q", "Quarter");
+ formats.put("1y", "Year");
+ return Collections.unmodifiableMap(formats);
+ }
+
private static Map<String, String> createReverseMap() {
Map<String, String> reverseMap = new HashMap<>();
for (Map.Entry<String, String> entry :
DateAggregate.OLD_TO_NEW_FORMAT.entrySet()) {
@@ -68,6 +77,7 @@ public class DateAggregate extends BaseAggregate {
*
* @param field the field to aggregate on
* @param interval the interval, in old (e.g., {@code 1M}) or new (e.g.,
{@code Month}) format
+ * @throws IllegalArgumentException if the interval is not supported (see
{@link #setInterval(String)})
*/
public DateAggregate(String field, String interval) {
super(field);
@@ -80,6 +90,7 @@ public class DateAggregate extends BaseAggregate {
* @param field the field to aggregate on
* @param interval the interval, in old or new format
* @param format an optional output format understood by the persistence
layer
+ * @throws IllegalArgumentException if the interval is not supported (see
{@link #setInterval(String)})
*/
public DateAggregate(String field, String interval, String format) {
super(field);
@@ -88,16 +99,25 @@ public class DateAggregate extends BaseAggregate {
}
/**
- * Sets the interval; falls back to default when null/empty.
+ * Sets the interval. A null or empty value selects the default ({@code
1M}).
*
- * @param interval the interval, in old or new format
+ * @param interval the interval, in old (e.g. {@code 1d}) or new (e.g.
{@code Day}) format
+ * @throws IllegalArgumentException if the interval is in neither format,
since the search engines cannot run it
*/
public void setInterval(String interval) {
- this.interval = (interval != null && !interval.isEmpty()) ? interval :
DEFAULT_INTERVAL;
+ if (interval == null || interval.isEmpty()) {
+ this.interval = DEFAULT_INTERVAL;
+ return;
+ }
+ if (!isOldFormat(interval) && !isNewFormat(interval)) {
+ throw new IllegalArgumentException(
+ "Unsupported date aggregate interval '" + interval + "',
use one of: " + SUPPORTED_INTERVALS);
+ }
+ this.interval = interval;
}
/**
- * Returns the interval as it was originally defined.
+ * Returns the interval as it was set: always a supported value, in old or
new format.
*
* @return the interval string
*/
diff --git
a/persistence-spi/src/test/java/org/apache/unomi/persistence/spi/aggregate/DateAggregateTest.java
b/persistence-spi/src/test/java/org/apache/unomi/persistence/spi/aggregate/DateAggregateTest.java
new file mode 100644
index 000000000..fb3e98d72
--- /dev/null
+++
b/persistence-spi/src/test/java/org/apache/unomi/persistence/spi/aggregate/DateAggregateTest.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.persistence.spi.aggregate;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+public class DateAggregateTest {
+
+ @Test
+ public void acceptsShortAndNamedIntervals() {
+ for (String interval : new String[]{"1s", "1m", "1h", "1d", "1w",
"1M", "1q", "1y",
+ "Second", "Minute", "Hour", "Day", "Week", "Month", "Quarter",
"Year"}) {
+ assertEquals(interval, new DateAggregate("properties.lastVisit",
interval).getInterval());
+ }
+ }
+
+ @Test
+ public void defaultsToAMonthWithoutAnInterval() {
+ assertEquals("1M", new
DateAggregate("properties.lastVisit").getInterval());
+ assertEquals("1M", new DateAggregate("properties.lastVisit",
null).getInterval());
+ assertEquals("1M", new DateAggregate("properties.lastVisit", "",
"yyyy-MM-dd").getInterval());
+ }
+
+ @Test
+ public void rejectsIntervalsTheEnginesCannotRun() {
+ for (String interval : new String[]{"2d", "day", "30m", "1D", "Days"})
{
+ IllegalArgumentException e =
assertThrows(IllegalArgumentException.class,
+ () -> new DateAggregate("properties.lastVisit", interval));
+ assertTrue(e.getMessage(), e.getMessage().contains("'" + interval
+ "'"));
+ assertTrue(e.getMessage(), e.getMessage().contains("1d"));
+ assertTrue(e.getMessage(), e.getMessage().contains("Quarter"));
+ }
+ }
+
+ @Test
+ public void setIntervalKeepsTheValidIntervalWhenRejecting() {
+ DateAggregate aggregate = new DateAggregate("properties.lastVisit",
"Day");
+ assertThrows(IllegalArgumentException.class, () ->
aggregate.setInterval("2d"));
+ assertEquals("Day", aggregate.getInterval());
+ }
+}
diff --git
a/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java
b/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java
index 18f7b2637..a4ca8b57a 100644
---
a/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java
+++
b/services/src/main/java/org/apache/unomi/services/impl/goals/GoalsServiceImpl.java
@@ -558,11 +558,11 @@ public class GoalsServiceImpl extends
AbstractMultiTypeCachingService implements
if (query.getAggregate().getType() != null){
// try to guess the aggregate type
if(query.getAggregate().getType().equals("date")) {
- String interval = (String)
query.getAggregate().getParameters().get("interval");
- String format = (String)
query.getAggregate().getParameters().get("format");
+ String interval =
Objects.toString(query.getAggregate().getParameters().get("interval"), null);
+ String format =
Objects.toString(query.getAggregate().getParameters().get("format"), null);
aggregate = new DateAggregate(property, interval, format);
} else if (query.getAggregate().getType().equals("dateRange")
&& query.getAggregate().getDateRanges() != null &&
query.getAggregate().getDateRanges().size() > 0) {
- String format = (String)
query.getAggregate().getParameters().get("format");
+ String format =
Objects.toString(query.getAggregate().getParameters().get("format"), null);
aggregate = new DateRangeAggregate(property, format,
query.getAggregate().getDateRanges());
} else if
(query.getAggregate().getType().equals("numericRange") &&
query.getAggregate().getNumericRanges() != null &&
query.getAggregate().getNumericRanges().size() > 0) {
aggregate = new NumericRangeAggregate(property,
query.getAggregate().getNumericRanges());
diff --git
a/services/src/main/java/org/apache/unomi/services/impl/queries/QueryServiceImpl.java
b/services/src/main/java/org/apache/unomi/services/impl/queries/QueryServiceImpl.java
index 7c820d398..d8874e974 100644
---
a/services/src/main/java/org/apache/unomi/services/impl/queries/QueryServiceImpl.java
+++
b/services/src/main/java/org/apache/unomi/services/impl/queries/QueryServiceImpl.java
@@ -28,6 +28,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Map;
+import java.util.Objects;
public class QueryServiceImpl implements QueryService {
private static final Logger LOGGER =
LoggerFactory.getLogger(QueryServiceImpl.class.getName());
@@ -99,12 +100,12 @@ public class QueryServiceImpl implements QueryService {
if (aggregateType != null) {
// try to guess the aggregate type
if (aggregateType.equals("date")) {
- String interval = (String)
query.getAggregate().getParameters().get("interval");
- String format = (String)
query.getAggregate().getParameters().get("format");
+ String interval =
Objects.toString(query.getAggregate().getParameters().get("interval"), null);
+ String format =
Objects.toString(query.getAggregate().getParameters().get("format"), null);
baseAggregate = new DateAggregate(property, interval,
format);
} else if (aggregateType.equals("dateRange") &&
query.getAggregate().getDateRanges() != null && !query.getAggregate()
.getDateRanges().isEmpty()) {
- String format = (String)
query.getAggregate().getParameters().get("format");
+ String format =
Objects.toString(query.getAggregate().getParameters().get("format"), null);
baseAggregate = new
DateRangeAggregate(query.getAggregate().getProperty(), format,
query.getAggregate().getDateRanges());
} else if (aggregateType.equals("numericRange") &&
query.getAggregate().getNumericRanges() != null && !query.getAggregate()
.getNumericRanges().isEmpty()) {
