This is an automated email from the ASF dual-hosted git repository. taybou pushed a commit to branch UNOMI-419-segment-it in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 1b9b53a94c197bda74f5ec6f9bac8b8cb784f839 Author: Taybou <[email protected]> AuthorDate: Mon Feb 22 17:43:18 2021 +0100 UNOMI-419 add IT to cover condition validation --- .../exceptions/BadSegmentConditionException.java | 24 ++++++++++++ .../java/org/apache/unomi/itests/SegmentIT.java | 45 +++++++++++++++++++++- .../ElasticSearchPersistenceServiceImpl.java | 10 ++--- .../unomi/persistence/spi/PersistenceService.java | 4 +- .../services/impl/segments/SegmentServiceImpl.java | 7 ++-- 5 files changed, 78 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/org/apache/unomi/api/exceptions/BadSegmentConditionException.java b/api/src/main/java/org/apache/unomi/api/exceptions/BadSegmentConditionException.java new file mode 100644 index 0000000..bed7131 --- /dev/null +++ b/api/src/main/java/org/apache/unomi/api/exceptions/BadSegmentConditionException.java @@ -0,0 +1,24 @@ +/* + * 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.api.exceptions; + +public class BadSegmentConditionException extends RuntimeException { + public BadSegmentConditionException() { + super(); + } +} diff --git a/itests/src/test/java/org/apache/unomi/itests/SegmentIT.java b/itests/src/test/java/org/apache/unomi/itests/SegmentIT.java index cd4e917..aa99574 100644 --- a/itests/src/test/java/org/apache/unomi/itests/SegmentIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/SegmentIT.java @@ -18,8 +18,10 @@ package org.apache.unomi.itests; import org.apache.unomi.api.Metadata; +import org.apache.unomi.api.conditions.Condition; import org.apache.unomi.api.segments.Segment; import org.apache.unomi.api.services.SegmentService; +import org.apache.unomi.api.exceptions.BadSegmentConditionException; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -38,8 +40,10 @@ import java.util.List; @ExamReactorStrategy(PerSuite.class) public class SegmentIT extends BaseIT { private final static Logger LOGGER = LoggerFactory.getLogger(SegmentIT.class); + private final static String SEGMENT_ID = "test-segment-id-2"; - @Inject @Filter(timeout = 600000) + @Inject + @Filter(timeout = 600000) protected SegmentService segmentService; @Before @@ -54,4 +58,43 @@ public class SegmentIT extends BaseIT { Assert.assertEquals("Segment metadata list should be empty", 0, segmentMetadatas.size()); LOGGER.info("Retrieved " + segmentMetadatas.size() + " segment metadata entries"); } + + @Test(expected = BadSegmentConditionException.class) + public void testSegmentWithNullCondition() { + Metadata segmentMetadata = new Metadata(SEGMENT_ID); + Segment segment = new Segment(); + segment.setMetadata(segmentMetadata); + segment.setCondition(null); + + segmentService.setSegmentDefinition(segment); + } + + @Test(expected = BadSegmentConditionException.class) + public void testSegmentWithInValidCondition() { + Metadata segmentMetadata = new Metadata(SEGMENT_ID); + Segment segment = new Segment(); + segment.setMetadata(segmentMetadata); + Condition condition = new Condition(); + condition.setParameter("param", "param value"); + condition.setConditionTypeId("fakeConditionId"); + segment.setCondition(condition); + + segmentService.setSegmentDefinition(segment); + } + + @Test + public void testSegmentWithValidCondition() { + Metadata segmentMetadata = new Metadata(SEGMENT_ID); + Segment segment = new Segment(segmentMetadata); + Condition segmentCondition = new Condition(definitionsService.getConditionType("pastEventCondition")); + segmentCondition.setParameter("minimumEventCount", 2); + segmentCondition.setParameter("numberOfDays", 10); + Condition pastEventEventCondition = new Condition(definitionsService.getConditionType("eventTypeCondition")); + pastEventEventCondition.setParameter("eventTypeId", "test-event-type"); + segmentCondition.setParameter("eventCondition", pastEventEventCondition); + segment.setCondition(segmentCondition); + segmentService.setSegmentDefinition(segment); + + segmentService.removeSegmentDefinition(SEGMENT_ID, false); + } } diff --git a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java index ae73987..c263ac9 100644 --- a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java +++ b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java @@ -1582,14 +1582,14 @@ public class ElasticSearchPersistenceServiceImpl implements PersistenceService, } @Override - public boolean isValidCondition(Condition query, Item item) { + public boolean isValidCondition(Condition condition, Item item) { try { - conditionEvaluatorDispatcher.eval(query, item); - QueryBuilder builder = QueryBuilders.boolQuery() + conditionEvaluatorDispatcher.eval(condition, item); + QueryBuilders.boolQuery() .must(QueryBuilders.idsQuery().addIds(item.getItemId())) - .must(conditionESQueryBuilderDispatcher.buildFilter(query)); + .must(conditionESQueryBuilderDispatcher.buildFilter(condition)); } catch (Exception e) { - logger.error("failed at setSegmentDefinition, condition={}", query.toString(), e); + logger.error("Failed to validate condition, condition={}", condition, e); return false; } return true; diff --git a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java index bd66ce7..31c0239 100644 --- a/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java +++ b/persistence-spi/src/main/java/org/apache/unomi/persistence/spi/PersistenceService.java @@ -320,11 +320,11 @@ public interface PersistenceService { /** * validates if a condition throws exception at query build. * - * @param query the condition we're testing the specified item against + * @param condition the condition we're testing the specified item against * @param item the item we're checking against the specified condition * @return {@code true} if the item satisfies the condition, {@code false} otherwise */ - boolean isValidCondition(Condition query, Item item); + boolean isValidCondition(Condition condition, Item item); /** * Same as {@code query(fieldName, fieldValue, sortBy, clazz, 0, -1).getList()} * diff --git a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java index 363395d..7d8c2ce 100644 --- a/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java +++ b/services/src/main/java/org/apache/unomi/services/impl/segments/SegmentServiceImpl.java @@ -39,6 +39,7 @@ import org.apache.unomi.persistence.spi.CustomObjectMapper; import org.apache.unomi.persistence.spi.aggregate.TermsAggregate; import org.apache.unomi.services.impl.AbstractServiceImpl; import org.apache.unomi.services.impl.ParserHelper; +import org.apache.unomi.api.exceptions.BadSegmentConditionException; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleEvent; @@ -257,8 +258,9 @@ public class SegmentServiceImpl extends AbstractServiceImpl implements SegmentSe public void setSegmentDefinition(Segment segment) { ParserHelper.resolveConditionType(definitionsService, segment.getCondition()); - if (persistenceService.isValidCondition(segment.getCondition(), new Profile()) == false) + if (!persistenceService.isValidCondition(segment.getCondition(), new Profile())) { throw new BadSegmentConditionException(); + } if (segment.getMetadata().isEnabled() && !segment.getMetadata().isMissingPlugins()) { updateAutoGeneratedRules(segment.getMetadata(), segment.getCondition()); @@ -1142,7 +1144,4 @@ public class SegmentServiceImpl extends AbstractServiceImpl implements SegmentSe public void setTaskExecutionPeriod(long taskExecutionPeriod) { this.taskExecutionPeriod = taskExecutionPeriod; } - - private class BadSegmentConditionException extends RuntimeException { - } }
