You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by "sergehuber (via GitHub)" <gi...@apache.org> on 2026/06/11 17:39:58 UTC
[PR] UNOMI-881: Add in-memory test harness and service unit tests (part 1/2) [unomi]
sergehuber opened a new pull request, #772:
URL: https://github.com/apache/unomi/pull/772
## Summary
- Backport the in-memory persistence test harness from `unomi-3-dev` and land the first batch of independent service unit tests (300 tests in `services`).
- Add minimal `persistence-spi` hooks (`addEvaluator`/`removeEvaluator`, `setDefinitionsService`) and modern date-type handling in `DateUtils` so the harness compiles and runs on `master` without tracing or validation modules.
- Adapt test utilities for master: no-op `TestRequestTracer`, stripped validation/tracing factories from `TestHelper`, explicit crash simulation in scheduler recovery test.
Part **1/2** of UNOMI-881. Tests that depend on tracing (UNOMI-873) or condition validation (UNOMI-883) are deferred to a follow-up PR.
## Test plan
- [x] `mvn -pl services -am test -Dmaven.build.cache.enabled=false -Dsurefire.failIfNoSpecifiedTests=false`
- [x] 300 unit tests pass (InMemoryPersistence, cache, cluster, profiles, scheduler, execution context, tenant harness)
- [ ] CI green on PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@unomi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UNOMI-881: Add in-memory test harness and service unit tests (part 1/2) [unomi]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #772:
URL: https://github.com/apache/unomi/pull/772#discussion_r3398746305
##########
services/src/test/java/org/apache/unomi/services/impl/cache/MultiTypeCacheServiceImplTest.java:
##########
@@ -0,0 +1,644 @@
+/*
+ * 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.services.impl.cache;
+
+import org.apache.unomi.api.services.cache.CacheableTypeConfig;
+import org.apache.unomi.api.services.cache.MultiTypeCacheService;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.Serializable;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+public class MultiTypeCacheServiceImplTest {
+
+ private static final String SYSTEM_TENANT = "system";
+ private static final String TEST_TENANT = "test-tenant";
+ private static final String TEST_TYPE = "test-type";
+ private static final String TEST_ID = "test-id";
+
+ private MultiTypeCacheServiceImpl cacheService;
+
+ // Test plugin type implementation
+ private static class TestSerializable implements Serializable {
+ private String id;
+
+ public TestSerializable(String id) {
+ this.id = id;
+ }
+
+
+ public String getId() {
+ return id;
+ }
+ }
+
+ @BeforeEach
+ public void setUp() {
+ cacheService = new MultiTypeCacheServiceImpl();
+ }
+
+ @Test
+ public void testRegisterType() {
+ // Create test configuration
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+
+ // Register type
+ cacheService.registerType(config);
+
+ // Put a value and verify it's cached
+ TestSerializable value = new TestSerializable(TEST_ID);
+ cacheService.put(TEST_TYPE, TEST_ID, TEST_TENANT, value);
+
+ // Verify value can be retrieved
+ TestSerializable retrieved = cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class);
+ assertNotNull(retrieved, "Retrieved value should not be null");
+ assertEquals(value.getId(), retrieved.getId(), "Retrieved value should match original");
+ }
+
+ @Test
+ public void testInheritanceFromSystemTenant() {
+ // Create test configuration with inheritance enabled
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Put a value in system tenant
+ TestSerializable systemValue = new TestSerializable("system-value");
+ cacheService.put(TEST_TYPE, TEST_ID, SYSTEM_TENANT, systemValue);
+
+ // Verify value can be retrieved from another tenant
+ TestSerializable retrieved = cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class);
+ assertNotNull(retrieved, "Retrieved value should not be null");
+ assertEquals(systemValue.getId(), retrieved.getId(), "Retrieved value should match system tenant value");
+
+ // Put a tenant-specific value
+ TestSerializable tenantValue = new TestSerializable("tenant-value");
+ cacheService.put(TEST_TYPE, TEST_ID, TEST_TENANT, tenantValue);
+
+ // Verify tenant-specific value overrides system tenant
+ retrieved = cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class);
+ assertNotNull(retrieved, "Retrieved value should not be null");
+ assertEquals(tenantValue.getId(), retrieved.getId(), "Retrieved value should match tenant value");
+ }
+
+ @Test
+ public void testGetValuesByPredicateWithInheritance() {
+ // Create test configuration with inheritance enabled
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Put values in system tenant
+ TestSerializable systemValue1 = new TestSerializable("system1");
+ TestSerializable systemValue2 = new TestSerializable("system2");
+ cacheService.put(TEST_TYPE, "system1", SYSTEM_TENANT, systemValue1);
+ cacheService.put(TEST_TYPE, "system2", SYSTEM_TENANT, systemValue2);
+
+ // Put values in test tenant
+ TestSerializable tenantValue1 = new TestSerializable("tenant1");
+ TestSerializable tenantValue2 = new TestSerializable("tenant2");
+ cacheService.put(TEST_TYPE, "tenant1", TEST_TENANT, tenantValue1);
+ cacheService.put(TEST_TYPE, "tenant2", TEST_TENANT, tenantValue2);
+
+ // Get values by predicate
+ Set<TestSerializable> values = cacheService.getValuesByPredicateWithInheritance(
+ TEST_TENANT,
+ TestSerializable.class,
+ value -> value.getId().startsWith("system")
+ );
+
+ assertEquals(2, values.size(), "Should find 2 values matching predicate");
+ assertTrue(values.contains(systemValue1), "Should contain system values");
+ assertTrue(values.contains(systemValue2), "Should contain system values");
+ }
+
+ @Test
+ public void testStatistics() {
+ // Create test configuration
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Put a value
+ TestSerializable value = new TestSerializable(TEST_ID);
+ cacheService.put(TEST_TYPE, TEST_ID, TEST_TENANT, value);
+
+ // Get the value (hit)
+ cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class);
+
+ // Try to get non-existent value (miss)
+ cacheService.getWithInheritance("non-existent", TEST_TENANT, TestSerializable.class);
+
+ // Verify statistics
+ MultiTypeCacheService.CacheStatistics stats = cacheService.getStatistics();
+ MultiTypeCacheService.CacheStatistics.TypeStatistics typeStats = stats.getAllStats().get(TEST_TYPE);
+
+ assertNotNull(typeStats, "Type statistics should exist");
+ assertEquals(1, typeStats.getHits(), "Should have 1 hit");
+ assertEquals(1, typeStats.getMisses(), "Should have 1 miss");
+ assertEquals(1, typeStats.getUpdates(), "Should have 1 update");
+
+ // Reset statistics
+ stats.reset();
+ assertTrue(stats.getAllStats().isEmpty(), "Statistics should be empty after reset");
+ }
+
+ @Test
+ public void testClearTenantCache() {
+ // Create test configuration
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Put values in different tenants
+ TestSerializable value1 = new TestSerializable("value1");
+ TestSerializable value2 = new TestSerializable("value2");
+ cacheService.put(TEST_TYPE, "value1", TEST_TENANT, value1);
+ cacheService.put(TEST_TYPE, "value2", SYSTEM_TENANT, value2);
+
+ // Clear test tenant cache
+ cacheService.clear(TEST_TENANT);
+
+ // Verify test tenant value is gone but system tenant value remains
+ assertNull(
+ cacheService.getWithInheritance("value1", TEST_TENANT, TestSerializable.class),
+ "Test tenant value should be cleared");
+ assertNotNull(
+ cacheService.getWithInheritance("value2", SYSTEM_TENANT, TestSerializable.class),
+ "System tenant value should remain");
+ }
+
+ @Test
+ public void testRemoveValue() {
+ // Create test configuration
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Put a value
+ TestSerializable value = new TestSerializable(TEST_ID);
+ cacheService.put(TEST_TYPE, TEST_ID, TEST_TENANT, value);
+
+ // Remove the value
+ cacheService.remove(TEST_TYPE, TEST_ID, TEST_TENANT, TestSerializable.class);
+
+ // Verify value is removed
+ assertNull(
+ cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class),
+ "Value should be removed");
+ }
+
+ @Test
+ public void testNullParameters() {
+ // Create test configuration
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Test null parameters for put
+ cacheService.put(null, TEST_ID, TEST_TENANT, new TestSerializable(TEST_ID));
+ cacheService.put(TEST_TYPE, null, TEST_TENANT, new TestSerializable(TEST_ID));
+ cacheService.put(TEST_TYPE, TEST_ID, null, new TestSerializable(TEST_ID));
+ cacheService.put(TEST_TYPE, TEST_ID, TEST_TENANT, null);
+
+ // Verify no values were cached
+ assertNull(
+ cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class),
+ "No value should be cached with null parameters");
+
+ // Test null parameters for get
+ assertNull(
+ cacheService.getWithInheritance(null, TEST_TENANT, TestSerializable.class),
+ "Get with null ID should return null");
+ assertNull(
+ cacheService.getWithInheritance(TEST_ID, null, TestSerializable.class),
+ "Get with null tenant should return null");
+ assertNull(
+ cacheService.getWithInheritance(TEST_ID, TEST_TENANT, null),
+ "Get with null type should return null");
+ }
+
+ @Test
+ public void testGetTenantCache() {
+ // Create test configuration
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Put multiple values
+ TestSerializable value1 = new TestSerializable("value1");
+ TestSerializable value2 = new TestSerializable("value2");
+ cacheService.put(TEST_TYPE, "value1", TEST_TENANT, value1);
+ cacheService.put(TEST_TYPE, "value2", TEST_TENANT, value2);
+
+ // Get tenant cache
+ Map<String, TestSerializable> tenantCache = cacheService.getTenantCache(TEST_TENANT, TestSerializable.class);
+ assertNotNull(tenantCache, "Tenant cache should not be null");
+ assertEquals(2, tenantCache.size(), "Tenant cache should contain 2 values");
+ assertEquals(value1.getId(), tenantCache.get("value1").getId(), "Value1 should match");
+ assertEquals(value2.getId(), tenantCache.get("value2").getId(), "Value2 should match");
+
+ // Verify cache is unmodifiable
+ try {
+ tenantCache.put("value3", new TestSerializable("value3"));
+ fail("Tenant cache should be unmodifiable");
+ } catch (UnsupportedOperationException e) {
+ // Expected
+ }
+ }
+
+ @Test
+ public void testUnregisteredType() {
+ // Try to get value for unregistered type
+ assertNull(
+ cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class),
+ "Get with unregistered type should return null");
+
+ // Try to put value for unregistered type
+ TestSerializable value = new TestSerializable(TEST_ID);
+ cacheService.put(TEST_TYPE, TEST_ID, TEST_TENANT, value);
+
+ // Verify value was not cached
+ assertNull(
+ cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class),
+ "Value should not be cached for unregistered type");
+ }
+
+ @Test
+ public void testRefreshTypeCache() {
+ // Create test configuration with refresh required
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Put a value
+ TestSerializable value = new TestSerializable(TEST_ID);
+ cacheService.put(TEST_TYPE, TEST_ID, TEST_TENANT, value);
+
+ // Refresh cache
+ cacheService.refreshTypeCache(config);
+
+ // Verify statistics
+ MultiTypeCacheService.CacheStatistics.TypeStatistics typeStats =
+ cacheService.getStatistics().getAllStats().get(TEST_TYPE);
+ assertEquals(0, typeStats.getIndexingErrors(), "Should have no indexing errors");
+ }
+
+ @Test
+ public void testInvalidRefreshTypeCache() {
+ // Create test configuration with refresh disabled
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(true)
+ .withRequiresRefresh(false)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+
+ // Try to refresh with disabled config
+ cacheService.refreshTypeCache(config);
+
+ // Verify no statistics were created
+ assertTrue(
+ cacheService.getStatistics().getAllStats().isEmpty(),
+ "No statistics should be created");
+ }
+
+ @Test
+ public void testInheritanceDisabled() {
+ // Create test configuration with inheritance disabled
+ CacheableTypeConfig<TestSerializable> config = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
+ "/test/path")
+ .withInheritFromSystemTenant(false)
+ .withRequiresRefresh(true)
+ .withRefreshInterval(1000L)
+ .withIdExtractor(TestSerializable::getId)
+ .build();
+ cacheService.registerType(config);
+
+ // Put a value in system tenant
+ TestSerializable systemValue = new TestSerializable("system-value");
+ cacheService.put(TEST_TYPE, TEST_ID, SYSTEM_TENANT, systemValue);
+
+ // Verify value cannot be retrieved from another tenant when inheritance is disabled
+ TestSerializable retrieved = cacheService.getWithInheritance(TEST_ID, TEST_TENANT, TestSerializable.class);
+ assertNull(retrieved, "Should not inherit from system tenant when inheritance is disabled");
+
+ // Verify statistics
+ MultiTypeCacheService.CacheStatistics.TypeStatistics typeStats =
+ cacheService.getStatistics().getAllStats().get(TEST_TYPE);
+ assertEquals(1, typeStats.getMisses(), "Should have 1 miss");
+ assertEquals(0, typeStats.getHits(), "Should have no hits");
+ }
+
+ @Test
+ public void testMultipleTypeConfigurations() {
+ // First configuration
+ CacheableTypeConfig<TestSerializable> config1 = CacheableTypeConfig.<TestSerializable>builder(
+ TestSerializable.class,
+ TEST_TYPE,
Review Comment:
`testMultipleTypeConfigurations` is misleading/incorrect: `MultiTypeCacheServiceImpl` only supports one `CacheableTypeConfig` per Java class (keyed by `Class<?>`), so registering two configs for `TestSerializable.class` overwrites the first. Additionally the test calls `put("type1"/"type2", ...)` even though neither type is registered, and it doesn't assert any retrievability/separation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@unomi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UNOMI-881: Add in-memory test harness and service unit tests (part 1/2) [unomi]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #772:
URL: https://github.com/apache/unomi/pull/772#discussion_r3398746201
##########
services/src/test/java/org/apache/unomi/services/impl/TestEventAdmin.java:
##########
@@ -0,0 +1,465 @@
+/*
+ * 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.services.impl;
+
+import org.osgi.service.event.Event;
+import org.osgi.service.event.EventAdmin;
+import org.osgi.service.event.EventHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.*;
+import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * Mock implementation of OSGi EventAdmin for unit tests.
+ *
+ * This implementation complies with the OSGi EventAdmin Service Specification (OSGi Compendium 8.1+):
+ *
+ * 1. postEvent(Event event): Asynchronous, ordered delivery
+ * - Returns immediately (non-blocking) per spec: "returns to the caller before delivery is complete"
+ * - Events are delivered to handlers in the order they were posted per spec: "Events are delivered in the order posted"
+ * - Each handler receives events in the order they were posted
+ * - Null events are ignored (early return)
+ * - Events with null topics are ignored (logged and skipped)
+ *
+ * 2. sendEvent(Event event): Synchronous delivery
+ * - Does not return until all handlers have processed the event per spec: "does not return until all event handlers have been called"
+ * - Handlers are called directly in the current thread (synchronous)
+ * - Null events are ignored (early return)
+ * - Events with null topics are ignored (logged and skipped)
+ *
+ * 3. Exception handling: Exceptions from handlers do not stop delivery to other handlers
+ * - Per spec: exceptions should be caught and logged (using LogService if available, SLF4J otherwise)
+ * - Delivery continues to remaining handlers
+ *
+ * 4. Topic matching: Supports OSGi hierarchical topic matching
+ * - EVENT_TOPIC service property: array of topic patterns
+ * - Empty EVENT_TOPIC matches all topics (defaults to "*")
+ * - Wildcard "*" matches all topics
+ * - Wildcard "**" at end matches all subtopics (e.g., "org/apache/unomi/**")
+ * - Single-level wildcard "*" at end matches one level (e.g., "org/apache/unomi/*")
+ * - Exact topic matching
+ * - Topics are hierarchical, separated by '/' character
+ *
+ * 5. Handler registration: Manual registration for tests (in real OSGi, handlers are registered via service registry)
+ * - Handlers specify topics via EVENT_TOPIC property (mapped to topics parameter)
+ * - EVENT_FILTER property is optional and not implemented (minimal compliance)
+ *
+ * Threading model:
+ * - postEvent(): Uses a single-threaded executor to process events in order
+ * - sendEvent(): Calls handlers directly in the current thread (synchronous)
+ * - Each handler has a dedicated queue to guarantee ordered delivery per handler
+ *
+ * Note: Security (TopicPermission) is not enforced in this test mock, as it's not required for minimal compliance
+ * in a test environment. Real OSGi implementations must enforce TopicPermission checks.
+ */
+public class TestEventAdmin implements EventAdmin {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(TestEventAdmin.class);
+
+ /**
+ * Registry of event handlers with their topic filters.
+ * Key: EventHandler instance
+ * Value: Set of topic patterns (from EVENT_TOPIC service property)
+ */
+ private final Map<EventHandler, Set<String>> handlers = new ConcurrentHashMap<>();
+
+ /**
+ * Single-threaded executor for processing asynchronous events in order.
+ * This ensures events are delivered to handlers in the order they were posted.
+ */
+ private final ExecutorService asyncExecutor;
+
+ /**
+ * Queue per handler to guarantee event sequencing.
+ * Each handler has its own queue, ensuring events are processed in order.
+ */
+ private final Map<EventHandler, BlockingQueue<Event>> handlerQueues = new ConcurrentHashMap<>();
+
+ /**
+ * Worker threads per handler to process events from their queues sequentially.
+ */
+ private final Map<EventHandler, Future<?>> handlerWorkers = new ConcurrentHashMap<>();
+
+ /**
+ * Counter for tracking posted events (for test verification).
+ */
+ private final AtomicInteger postedEventCount = new AtomicInteger(0);
+
+ /**
+ * Counter for tracking sent events (for test verification).
+ */
+ private final AtomicInteger sentEventCount = new AtomicInteger(0);
+
+ /**
+ * List of all events posted (for test verification).
+ */
+ private final List<Event> postedEvents = new CopyOnWriteArrayList<>();
+
+ /**
+ * List of all events sent (for test verification).
+ */
+ private final List<Event> sentEvents = new CopyOnWriteArrayList<>();
+
+ /**
+ * Creates a TestEventAdmin with a single-threaded executor for ordered event delivery.
+ */
+ public TestEventAdmin() {
+ // Use single-threaded executor to guarantee ordered delivery
+ this.asyncExecutor = Executors.newSingleThreadExecutor(r -> {
+ Thread t = new Thread(r, "TestEventAdmin-Async-" + System.identityHashCode(this));
+ t.setDaemon(true);
+ return t;
+ });
Review Comment:
`registerHandler` submits one long-running worker per handler onto `asyncExecutor`, but `asyncExecutor` is a *single-thread* executor. The first worker blocks on `queue.take()` forever, preventing subsequent handler workers from ever running, so only the first registered handler will receive async events.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@unomi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UNOMI-881: Add in-memory test harness and service unit tests (part 1/2) [unomi]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #772:
URL: https://github.com/apache/unomi/pull/772#discussion_r3398746251
##########
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/DateUtils.java:
##########
@@ -66,6 +70,18 @@ public static Date getDate(Object value) {
}
if (value instanceof Date) {
return (Date) value;
+ }
+ if (value instanceof Instant) {
+ return Date.from((Instant) value);
+ }
+ if (value instanceof OffsetDateTime) {
+ return Date.from(((OffsetDateTime) value).toInstant());
+ }
+ if (value instanceof ZonedDateTime) {
+ return Date.from(((ZonedDateTime) value).toInstant());
+ }
+ if (value instanceof LocalDateTime) {
+ return Date.from(((LocalDateTime) value).atZone(ZoneId.systemDefault()).toInstant());
} else {
Review Comment:
`DateUtils` Javadoc states parsing/conversion is evaluated in UTC, but `LocalDateTime` is currently converted using `ZoneId.systemDefault()`, which makes results machine-dependent and not UTC.
##########
services/src/test/java/org/apache/unomi/services/impl/TestEventAdmin.java:
##########
@@ -0,0 +1,465 @@
+/*
+ * 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.services.impl;
+
+import org.osgi.service.event.Event;
+import org.osgi.service.event.EventAdmin;
+import org.osgi.service.event.EventHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.*;
+import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * Mock implementation of OSGi EventAdmin for unit tests.
+ *
+ * This implementation complies with the OSGi EventAdmin Service Specification (OSGi Compendium 8.1+):
+ *
+ * 1. postEvent(Event event): Asynchronous, ordered delivery
+ * - Returns immediately (non-blocking) per spec: "returns to the caller before delivery is complete"
+ * - Events are delivered to handlers in the order they were posted per spec: "Events are delivered in the order posted"
+ * - Each handler receives events in the order they were posted
+ * - Null events are ignored (early return)
+ * - Events with null topics are ignored (logged and skipped)
+ *
+ * 2. sendEvent(Event event): Synchronous delivery
+ * - Does not return until all handlers have processed the event per spec: "does not return until all event handlers have been called"
+ * - Handlers are called directly in the current thread (synchronous)
+ * - Null events are ignored (early return)
+ * - Events with null topics are ignored (logged and skipped)
+ *
+ * 3. Exception handling: Exceptions from handlers do not stop delivery to other handlers
+ * - Per spec: exceptions should be caught and logged (using LogService if available, SLF4J otherwise)
+ * - Delivery continues to remaining handlers
+ *
+ * 4. Topic matching: Supports OSGi hierarchical topic matching
+ * - EVENT_TOPIC service property: array of topic patterns
+ * - Empty EVENT_TOPIC matches all topics (defaults to "*")
+ * - Wildcard "*" matches all topics
+ * - Wildcard "**" at end matches all subtopics (e.g., "org/apache/unomi/**")
+ * - Single-level wildcard "*" at end matches one level (e.g., "org/apache/unomi/*")
+ * - Exact topic matching
+ * - Topics are hierarchical, separated by '/' character
+ *
+ * 5. Handler registration: Manual registration for tests (in real OSGi, handlers are registered via service registry)
+ * - Handlers specify topics via EVENT_TOPIC property (mapped to topics parameter)
+ * - EVENT_FILTER property is optional and not implemented (minimal compliance)
+ *
+ * Threading model:
+ * - postEvent(): Uses a single-threaded executor to process events in order
+ * - sendEvent(): Calls handlers directly in the current thread (synchronous)
+ * - Each handler has a dedicated queue to guarantee ordered delivery per handler
+ *
+ * Note: Security (TopicPermission) is not enforced in this test mock, as it's not required for minimal compliance
+ * in a test environment. Real OSGi implementations must enforce TopicPermission checks.
+ */
+public class TestEventAdmin implements EventAdmin {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(TestEventAdmin.class);
+
+ /**
+ * Registry of event handlers with their topic filters.
+ * Key: EventHandler instance
+ * Value: Set of topic patterns (from EVENT_TOPIC service property)
+ */
+ private final Map<EventHandler, Set<String>> handlers = new ConcurrentHashMap<>();
+
+ /**
+ * Single-threaded executor for processing asynchronous events in order.
+ * This ensures events are delivered to handlers in the order they were posted.
+ */
+ private final ExecutorService asyncExecutor;
+
+ /**
+ * Queue per handler to guarantee event sequencing.
+ * Each handler has its own queue, ensuring events are processed in order.
+ */
+ private final Map<EventHandler, BlockingQueue<Event>> handlerQueues = new ConcurrentHashMap<>();
+
+ /**
+ * Worker threads per handler to process events from their queues sequentially.
+ */
+ private final Map<EventHandler, Future<?>> handlerWorkers = new ConcurrentHashMap<>();
+
+ /**
+ * Counter for tracking posted events (for test verification).
+ */
+ private final AtomicInteger postedEventCount = new AtomicInteger(0);
+
+ /**
+ * Counter for tracking sent events (for test verification).
+ */
+ private final AtomicInteger sentEventCount = new AtomicInteger(0);
+
+ /**
+ * List of all events posted (for test verification).
+ */
+ private final List<Event> postedEvents = new CopyOnWriteArrayList<>();
+
+ /**
+ * List of all events sent (for test verification).
+ */
+ private final List<Event> sentEvents = new CopyOnWriteArrayList<>();
+
+ /**
+ * Creates a TestEventAdmin with a single-threaded executor for ordered event delivery.
+ */
+ public TestEventAdmin() {
+ // Use single-threaded executor to guarantee ordered delivery
+ this.asyncExecutor = Executors.newSingleThreadExecutor(r -> {
+ Thread t = new Thread(r, "TestEventAdmin-Async-" + System.identityHashCode(this));
+ t.setDaemon(true);
+ return t;
+ });
+ }
+
+ /**
+ * Registers an event handler with topic filters.
+ * In real OSGi, handlers are registered via the service registry with EVENT_TOPIC property.
+ * For tests, we allow manual registration.
+ *
+ * @param handler the event handler to register
+ * @param topics topic patterns (supports OSGi hierarchical wildcards)
+ */
+ public void registerHandler(EventHandler handler, String... topics) {
+ if (handler == null) {
+ return;
+ }
+
+ Set<String> topicFilters = new HashSet<>();
+ if (topics != null && topics.length > 0) {
+ topicFilters.addAll(Arrays.asList(topics));
+ } else {
+ // No filter means match all topics (OSGi spec: empty EVENT_TOPIC matches all)
+ topicFilters.add("*");
+ }
+
+ handlers.put(handler, topicFilters);
+
+ // Create a dedicated queue for this handler to guarantee sequencing
+ BlockingQueue<Event> queue = new LinkedBlockingQueue<>();
+ handlerQueues.put(handler, queue);
+
+ // Start a worker thread for this handler to process events sequentially
+ Future<?> worker = asyncExecutor.submit(() -> {
+ try {
+ while (!Thread.currentThread().isInterrupted()) {
+ Event event = queue.take(); // Blocks until event is available
+ if (event != null) {
+ try {
+ handler.handleEvent(event);
+ } catch (Exception e) {
+ // OSGi spec: catch exceptions, log them, and continue
+ // If LogService is available, it should be used (we use SLF4J)
+ LOGGER.warn("Exception in event handler {} while processing event {}: {}",
+ handler.getClass().getName(), event.getTopic(), e.getMessage(), e);
+ }
+ }
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ });
+ handlerWorkers.put(handler, worker);
+
+ LOGGER.debug("Registered event handler: {} with topics: {}", handler.getClass().getName(), topicFilters);
+ }
+
+ /**
+ * Unregisters an event handler.
+ *
+ * @param handler the event handler to unregister
+ */
+ public void unregisterHandler(EventHandler handler) {
+ if (handler == null) {
+ return;
+ }
+
+ handlers.remove(handler);
+ BlockingQueue<Event> queue = handlerQueues.remove(handler);
+ Future<?> worker = handlerWorkers.remove(handler);
+
+ if (worker != null) {
+ worker.cancel(true);
+ }
+
+ // Drain remaining events from queue
+ if (queue != null) {
+ queue.clear();
+ }
+
+ LOGGER.debug("Unregistered event handler: {}", handler.getClass().getName());
+ }
+
+ /**
+ * Posts an event asynchronously (non-blocking).
+ *
+ * OSGi spec: "Initiates asynchronous, ordered delivery of an event. This method returns
+ * to the caller before delivery is complete. Events are delivered in the order posted."
+ *
+ * @param event the event to post
+ */
+ @Override
+ public void postEvent(Event event) {
+ if (event == null) {
+ return;
+ }
+
+ String topic = event.getTopic();
+ if (topic == null) {
+ // OSGi spec: Events must have a topic. Skip delivery if topic is null.
+ LOGGER.warn("Event has null topic, skipping delivery");
+ return;
+ }
+
+ postedEventCount.incrementAndGet();
+ postedEvents.add(event);
+
+ LOGGER.debug("Posting event asynchronously: {}", topic);
+
+ // Deliver to all matching handlers asynchronously
+ // Each handler gets events in order via its dedicated queue
+ for (Map.Entry<EventHandler, Set<String>> entry : handlers.entrySet()) {
+ EventHandler handler = entry.getKey();
+ Set<String> topicFilters = entry.getValue();
+
+ if (matchesTopic(topic, topicFilters)) {
+ BlockingQueue<Event> queue = handlerQueues.get(handler);
+ if (queue != null) {
+ // Add to handler's queue (non-blocking, will be processed by handler's worker thread)
+ queue.offer(event);
+ }
+ }
+ }
+ }
+
+ /**
+ * Sends an event synchronously (blocking until all handlers complete).
+ *
+ * OSGi spec: "Synchronously sends an event. This method does not return to the caller
+ * until all event handlers have been called."
+ *
+ * @param event the event to send
+ */
+ @Override
+ public void sendEvent(Event event) {
+ if (event == null) {
+ return;
+ }
+
+ String topic = event.getTopic();
+ if (topic == null) {
+ // OSGi spec: Events must have a topic. Skip delivery if topic is null.
+ LOGGER.warn("Event has null topic, skipping delivery");
+ return;
+ }
+
+ sentEventCount.incrementAndGet();
+ sentEvents.add(event);
+
+ LOGGER.debug("Sending event synchronously: {}", topic);
+
+ // Collect all matching handlers
+ List<EventHandler> matchingHandlers = new ArrayList<>();
+ for (Map.Entry<EventHandler, Set<String>> entry : handlers.entrySet()) {
+ EventHandler handler = entry.getKey();
+ Set<String> topicFilters = entry.getValue();
+
+ if (matchesTopic(topic, topicFilters)) {
+ matchingHandlers.add(handler);
+ }
+ }
+
+ // Deliver to all matching handlers synchronously in the current thread
+ // OSGi spec: "does not return until all event handlers have been called"
+ for (EventHandler handler : matchingHandlers) {
+ try {
+ handler.handleEvent(event);
+ } catch (Exception e) {
+ // OSGi spec: catch exceptions, log them, and continue
+ // If LogService is available, it should be used (we use SLF4J)
+ LOGGER.warn("Exception in event handler {} while processing event {}: {}",
+ handler.getClass().getName(), topic, e.getMessage(), e);
+ }
+ }
+ }
+
+ /**
+ * Checks if an event topic matches any of the topic filters.
+ *
+ * OSGi spec topic matching rules:
+ * - Topics are hierarchical, separated by '/' (e.g., "org/apache/unomi/definitions/conditionType/ADDED")
+ * - Wildcard "*" matches all topics
+ * - Wildcard "**" matches all subtopics (e.g., "org/apache/unomi/**" matches all topics starting with that prefix)
+ * - Exact match for specific topics
+ *
+ * @param topic the event topic
+ * @param topicFilters the topic filters to check against
+ * @return true if the topic matches any filter
+ */
+ private boolean matchesTopic(String topic, Set<String> topicFilters) {
+ if (topic == null) {
+ return false;
+ }
+
+ for (String filter : topicFilters) {
+ if ("*".equals(filter)) {
+ return true;
+ }
+
+ // Support wildcard pattern: "org/apache/unomi/**"
+ // Matches all topics starting with the prefix (including the prefix itself)
+ if (filter.endsWith("/**")) {
+ String prefix = filter.substring(0, filter.length() - 3);
+ if (topic.startsWith(prefix + "/") || topic.equals(prefix)) {
+ return true;
+ }
+ }
+
+ // Exact match
+ if (topic.equals(filter)) {
+ return true;
+ }
+
+ // Support single-level wildcard: "org/apache/unomi/*"
+ // Matches exactly one level (no nested slashes after the prefix)
+ if (filter.endsWith("/*")) {
+ String prefix = filter.substring(0, filter.length() - 2);
+ if (topic.startsWith(prefix + "/")) {
+ String remainder = topic.substring(prefix.length() + 1);
+ // Remainder should not contain '/' (single level only)
+ if (!remainder.contains("/")) {
+ return true;
+ }
+ }
+ }
+ }
+
+ return false;
+ }
+
+ /**
+ * Gets all events that were posted (asynchronously).
+ *
+ * @return a list of posted events
+ */
+ public List<Event> getPostedEvents() {
+ return new ArrayList<>(postedEvents);
+ }
+
+ /**
+ * Gets all events that were sent (synchronously).
+ *
+ * @return a list of sent events
+ */
+ public List<Event> getSentEvents() {
+ return new ArrayList<>(sentEvents);
+ }
+
+ /**
+ * Clears all stored events.
+ */
+ public void clearEvents() {
+ postedEvents.clear();
+ sentEvents.clear();
+ postedEventCount.set(0);
+ sentEventCount.set(0);
+ }
+
+ /**
+ * Gets the count of posted events.
+ *
+ * @return the number of posted events
+ */
+ public int getPostedEventCount() {
+ return postedEventCount.get();
+ }
+
+ /**
+ * Gets the count of sent events.
+ *
+ * @return the number of sent events
+ */
+ public int getSentEventCount() {
+ return sentEventCount.get();
+ }
+
+ /**
+ * Gets the number of registered handlers.
+ *
+ * @return the number of registered handlers
+ */
+ public int getHandlerCount() {
+ return handlers.size();
+ }
+
+ /**
+ * Waits for all pending asynchronous events to be processed.
+ * This is useful in tests to ensure events have been delivered before assertions.
+ *
+ * @param timeoutMs maximum time to wait in milliseconds
+ * @return true if all events were processed, false if timeout occurred
+ */
+ public boolean waitForEventProcessing(long timeoutMs) {
+ long deadline = System.currentTimeMillis() + timeoutMs;
+
+ // Wait for all handler queues to be empty
+ for (BlockingQueue<Event> queue : handlerQueues.values()) {
+ while (!queue.isEmpty() && System.currentTimeMillis() < deadline) {
+ try {
+ Thread.sleep(10);
Review Comment:
`waitForEventProcessing` only waits for handler queues to become empty. Because the worker thread removes the event from the queue *before* executing `handleEvent`, this method can return `true` while handlers are still processing, which can make tests flaky.
##########
services/src/test/java/org/apache/unomi/services/impl/rules/TestSetEventOccurrenceCountAction.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * 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.services.impl.rules;
+
+import org.apache.unomi.api.Event;
+import org.apache.unomi.api.actions.Action;
+import org.apache.unomi.api.actions.ActionExecutor;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.services.DefinitionsService;
+import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.persistence.spi.PersistenceService;
+
+import javax.xml.bind.DatatypeConverter;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.*;
+
+/**
+ * Test implementation of the SetEventOccurrenceCountAction.
+ * @TODO This is a duplicate of the SetEventOccurrenceCountAction in the unomi-plugins-base project to avoid introducing a depending to it but should be cleaned up.
+ */
Review Comment:
Javadoc has a typo/grammar issue: "introducing a depending to it" should be "introducing a dependency on it" (and `@TODO` isn't a standard Javadoc tag).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@unomi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] UNOMI-881: Add in-memory test harness and service unit tests (part 1/2) [unomi]
Posted by "sergehuber (via GitHub)" <gi...@apache.org>.
sergehuber merged PR #772:
URL: https://github.com/apache/unomi/pull/772
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@unomi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org