Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber merged PR #715: URL: https://github.com/apache/unomi/pull/715 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
jsinovassin commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2569142243
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/dispatcher/ConditionQueryBuilderDispatcherSupport.java:
##
@@ -0,0 +1,93 @@
+/*
+ * 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.conditions.dispatcher;
+
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.apache.unomi.scripting.ScriptExecutor;
+import org.slf4j.Logger;
+
+import java.util.Map;
+import java.util.function.Predicate;
+
+/**
+ * Shared helper for condition query builder dispatchers (ES/OS). Centralizes
logic that is
+ * backend-agnostic: contextualization, legacy ID mapping with logging, and
queryBuilder key resolution.
+ * The legacy-to-new queryBuilder identifiers are centralized here in
+ * {@link #LEGACY_TO_NEW_QUERY_BUILDER_IDS} and are used by the no-arg
+ * {@link #resolveLegacyQueryBuilderId(String, String, org.slf4j.Logger)} and
+ * {@link #findQueryBuilderKey(String, String, java.util.function.Predicate,
org.slf4j.Logger)} methods.
+ * This helper intentionally avoids any dependency on backend-specific query
types.
+ */
+public class ConditionQueryBuilderDispatcherSupport {
+
+/**
+ * Backend-agnostic legacy-to-new mapping of queryBuilder identifiers.
+ */
+public static final Map LEGACY_TO_NEW_QUERY_BUILDER_IDS =
Map.ofEntries(
+Map.entry("idsConditionESQueryBuilder",
"idsConditionQueryBuilder"),
+Map.entry("geoLocationByPointSessionConditionESQueryBuilder",
"geoLocationByPointSessionConditionQueryBuilder"),
+Map.entry("pastEventConditionESQueryBuilder",
"pastEventConditionQueryBuilder"),
+Map.entry("booleanConditionESQueryBuilder",
"booleanConditionQueryBuilder"),
+Map.entry("notConditionESQueryBuilder",
"notConditionQueryBuilder"),
+Map.entry("matchAllConditionESQueryBuilder",
"matchAllConditionQueryBuilder"),
+Map.entry("propertyConditionESQueryBuilder",
"propertyConditionQueryBuilder"),
+Map.entry("sourceEventPropertyConditionESQueryBuilder",
"sourceEventPropertyConditionQueryBuilder"),
+Map.entry("nestedConditionESQueryBuilder",
"nestedConditionQueryBuilder")
+);
+
+/**
+ * Returns a contextualized copy of the provided condition if any dynamic
parameters are present,
+ * otherwise returns {@code null} to indicate that a default fallback
should be used by callers.
+ */
+public Condition contextualize(Condition condition, Map
context, ScriptExecutor scriptExecutor) {
+return ConditionContextHelper.getContextualCondition(condition,
context, scriptExecutor);
+}
+
+/**
+ * Resolves a legacy queryBuilder identifier to its new canonical
identifier and logs a deprecation warning.
+ * Returns {@code null} if the provided identifier is not legacy-mapped.
+ */
+public String resolveLegacyQueryBuilderId(String queryBuilderId, String
conditionTypeId, Logger logger) {
+if (!LEGACY_TO_NEW_QUERY_BUILDER_IDS.containsKey(queryBuilderId)) {
+return null;
+}
+String mappedId = LEGACY_TO_NEW_QUERY_BUILDER_IDS.get(queryBuilderId);
+logger.warn("DEPRECATED: Using legacy queryBuilderId '{}' for
condition type '{}'. Please update your condition definition to use the new
queryBuilderId '{}'. Legacy mappings are deprecated and may be removed in
future versions.",
+queryBuilderId, conditionTypeId, mappedId);
+return mappedId;
+}
+
+/**
+ * Resolves the final queryBuilder key to use, trying the provided key
first, then applying legacy mapping.
+ * The {@code hasBuilder} predicate is used to test the presence of a
builder for a given key.
+ */
+public String findQueryBuilderKey(String queryBuilderKey, String
conditionTypeId,
Review Comment:
You should be able to remove the Logger from this method by using the
abstract method
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/dispatcher/Con
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on PR #715: URL: https://github.com/apache/unomi/pull/715#issuecomment-3527001363 Ok checks are passing again -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on PR #715: URL: https://github.com/apache/unomi/pull/715#issuecomment-3526033275 Following the release I need to update the version numbers, I'll do this asap. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426653657
##
api/src/main/java/org/apache/unomi/api/services/ProfileService.java:
##
@@ -443,4 +443,10 @@ default Session loadSession(String sessionId) {
*/
@Deprecated
void purgeMonthlyItems(int existsNumberOfMonths);
+
+/**
+ * Delete a session using its identifier
+ * @param sessionIdentifier the unique identifier for the session
+ */
+void deleteSession(String sessionIdentifier);
Review Comment:
This has been removed from this 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428299963
##
itests/src/test/resources/org.apache.unomi.healthcheck.cfg:
##
@@ -22,10 +22,18 @@ esLogin = ${org.apache.unomi.elasticsearch.username:-}
esPassword = ${org.apache.unomi.elasticsearch.password:-}
httpClient.trustAllCertificates =
${org.apache.unomi.elasticsearch.sslTrustAllCertificates:-false}
+# OpenSearch configuration
+osAddresses = ${org.apache.unomi.opensearch.addresses:-localhost:9200}
+osSSLEnabled = ${org.apache.unomi.opensearch.sslEnable:-false}
+osLogin = ${org.apache.unomi.opensearch.username:-admin}
+osPassword = ${org.apache.unomi.opensearch.password:-}
+osMinimalClusterState =
${org.apache.unomi.opensearch.minimalClusterState:-YELLOW}
+httpClient.trustAllCertificates =
${org.apache.unomi.opensearch.sslTrustAllCertificates:-false}
Review Comment:
Fixed by using separate names for the trustall option
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715: URL: https://github.com/apache/unomi/pull/715#discussion_r2428520696 ## extensions/privacy-extension/rest/pom.xml: ## @@ -88,6 +88,7 @@ +
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715: URL: https://github.com/apache/unomi/pull/715#discussion_r2428450910 ## plugins/hover-event/src/main/java/org/apache/unomi/plugins/events/hover/querybuilders/HoverEventConditionESQueryBuilder.java: ## Review Comment: I will instead replace it with a parent condition, we don't need a special query builder for this event. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426643467
##
api/src/main/java/org/apache/unomi/api/services/EventService.java:
##
@@ -161,4 +161,10 @@ public interface EventService {
* @param profileId identifier of the profile that we want to remove it's
events
*/
void removeProfileEvents(String profileId);
+
+/**
+ * Delete an event by specifying its event identifier
+ * @param eventIdentifier the unique identifier for the event
+ */
+void deleteEvent(String eventIdentifier);
Review Comment:
This was removed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428359272
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/geo/DistanceUnit.java:
##
@@ -0,0 +1,163 @@
+/*
+ * 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.conditions.geo;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public enum DistanceUnit {
Review Comment:
The library would have to be evaluated and tested to be 100% compatible with
the implementations of ElasticSearch and OpenSearch. For this reason I suggest
we keep the currently tested implementations and change this maybe later. I'll
add a TODO for that.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426650405
##
tools/shell-commands/src/main/java/org/apache/unomi/shell/actions/Start.java:
##
@@ -29,8 +31,20 @@ public class Start implements Action {
@Reference
UnomiManagementService unomiManagementService;
+@Argument(name = "persistence", description = "Persistence implementation
to use (elasticsearch/opensearch)", valueToShowInHelp = "elasticsearch")
+private String selectedPersistenceImplementation = "elasticsearch";
+
Review Comment:
As I mentioned above, this is now no longer a persistence implementation but
a start configuration that is configurable. This makes more sense.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428305110
##
plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java:
##
@@ -91,14 +93,15 @@ private int compare(Object actualValue, String
expectedValue, Object expectedVal
} else if (expectedValueDateExpr != null) {
return
getDate(actualValue).compareTo(getDate(expectedValueDateExpr));
} else {
-return actualValue.toString().compareTo(expectedValue);
+// We use toLowerCase here to match the behavior of the analyzer
configuration in the persistence configuration
+return
actualValue.toString().toLowerCase().compareTo(expectedValue);
Review Comment:
Thanks for catching this, I've changed it to foldToASCII.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426651920
##
tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/commands/ConditionRemove.java:
##
@@ -0,0 +1,45 @@
+/*
+ * 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.shell.commands;
+
+import org.apache.karaf.shell.api.action.Argument;
+import org.apache.karaf.shell.api.action.Command;
+import org.apache.karaf.shell.api.action.lifecycle.Reference;
+import org.apache.karaf.shell.api.action.lifecycle.Service;
+import org.apache.unomi.api.services.DefinitionsService;
+
+@Command(scope = "unomi", name = "condition-remove", description = "This
command will remove an condition type.")
+@Service
+public class ConditionRemove extends RemoveCommandSupport {
Review Comment:
This has been removed from this PR.
##
tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/commands/EventRemove.java:
##
@@ -0,0 +1,45 @@
+/*
+ * 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.shell.commands;
+
+import org.apache.karaf.shell.api.action.Argument;
+import org.apache.karaf.shell.api.action.Command;
+import org.apache.karaf.shell.api.action.lifecycle.Reference;
+import org.apache.karaf.shell.api.action.lifecycle.Service;
+import org.apache.unomi.api.services.EventService;
+
+@Command(scope = "unomi", name = "event-remove", description = "This command
will remove an event.")
+@Service
+public class EventRemove extends RemoveCommandSupport {
Review Comment:
This has been removed from this 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428142612
##
persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java:
##
@@ -0,0 +1,2761 @@
+/*
+ * 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.opensearch;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.json.stream.JsonParser;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.http.HttpHost;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.http.util.EntityUtils;
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.query.DateRange;
+import org.apache.unomi.api.query.IpRange;
+import org.apache.unomi.api.query.NumericRange;
+import org.apache.unomi.metrics.MetricAdapter;
+import org.apache.unomi.metrics.MetricsService;
+import org.apache.unomi.persistence.spi.PersistenceService;
+import org.apache.unomi.persistence.spi.aggregate.DateRangeAggregate;
+import org.apache.unomi.persistence.spi.aggregate.IpRangeAggregate;
+import org.apache.unomi.persistence.spi.aggregate.*;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.apache.unomi.persistence.spi.conditions.ConditionEvaluator;
+import
org.apache.unomi.persistence.spi.conditions.ConditionEvaluatorDispatcher;
+import org.opensearch.client.*;
+import org.opensearch.client.json.JsonData;
+import org.opensearch.client.json.JsonpMapper;
+import org.opensearch.client.json.jackson.JacksonJsonpMapper;
+import org.opensearch.client.opensearch.OpenSearchClient;
+import org.opensearch.client.opensearch._types.*;
+import org.opensearch.client.opensearch._types.aggregations.*;
+import org.opensearch.client.opensearch._types.mapping.TypeMapping;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+import org.opensearch.client.opensearch.cluster.HealthRequest;
+import org.opensearch.client.opensearch.cluster.HealthResponse;
+import org.opensearch.client.opensearch.core.*;
+import org.opensearch.client.opensearch.core.bulk.BulkOperation;
+import org.opensearch.client.opensearch.core.bulk.UpdateOperation;
+import org.opensearch.client.opensearch.core.search.Hit;
+import org.opensearch.client.opensearch.core.search.HitsMetadata;
+import org.opensearch.client.opensearch.core.search.TotalHits;
+import org.opensearch.client.opensearch.core.search.TotalHitsRelation;
+import org.opensearch.client.opensearch.indices.*;
+import org.opensearch.client.opensearch.indices.get_alias.IndexAliases;
+import org.opensearch.client.opensearch.tasks.GetTasksResponse;
+import org.opensearch.client.transport.OpenSearchTransport;
+import org.opensearch.client.transport.rest_client.RestClientTransport;
+import org.osgi.framework.*;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
+import java.io.*;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+import java.security.cert.X509Certificate;
+import java.util.*;
+import java.util.stream.Collectors;
+
+@SuppressWarnings("rawtypes")
+public class OpenSearchPersistenceServiceImpl implements PersistenceService,
SynchronousBundleListener {
+
+public static final String SEQ_NO = "seq_no";
+public static final String PRIMARY_TERM = "primary_term";
+
+private static final Logger LOGGER =
LoggerFactory.getLogger(OpenSearchPersistenceServiceImpl.class.getName());
+private static final String ROLLOVER_LIFECYCLE_NAME =
"unomi-rollover-policy";
+
+priv
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426648820
##
persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/ElasticSearchPersistenceServiceImpl.java:
##
@@ -1307,7 +1317,7 @@ protected Boolean execute(Object... args) throws
Exception {
itemType = customItemType;
}
String documentId = getDocumentIDForItemType(itemId,
itemType);
-String index = getIndexNameForQuery(itemType);
+String index = getIndex(itemType);
Review Comment:
This was rollbacked
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428524970
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/CustomObjectMapper.java:
##
@@ -58,6 +58,7 @@ public CustomObjectMapper() {
public CustomObjectMapper(Map> deserializers) {
super();
super.registerModule(new JaxbAnnotationModule());
+super.registerModule(new
com.fasterxml.jackson.datatype.jsr310.JavaTimeModule());
Review Comment:
This is no longer relevant it was replaced by the JavaTimeModule. The
initial change was due to the fact that Jackson changed its packaging and
remove the functionality from the base package.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715: URL: https://github.com/apache/unomi/pull/715#discussion_r2428302931 ## kar/src/main/feature/feature.xml: ## @@ -16,12 +16,12 @@ ~ limitations under the License. --> -http://karaf.apache.org/xmlns/features/v1.3.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://karaf.apache.org/xmlns/features/v1.3.0 http://karaf.apache.org/xmlns/features/v1.3.0";> +http://karaf.apache.org/xmlns/features/v1.3.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://karaf.apache.org/xmlns/features/v1.3.0 http://karaf.apache.org/xmlns/features/v1.3.0";> Review Comment: This is unfortunately necessary. I've tried a lot of different ways to solve this problem but the resolver keeps getting in the way if I try to deploy both persistence implementations. I've however changed the packaging now to use start configurations that should be very flexible to build custom packages. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426640056
##
extensions/healthcheck/src/main/java/org/apache/unomi/healthcheck/provider/ElasticSearchHealthCheckProvider.java:
##
@@ -60,10 +57,21 @@ public class ElasticSearchHealthCheckProvider implements
HealthCheckProvider {
private CloseableHttpClient httpClient;
+@Reference(service = PersistenceService.class, cardinality =
ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC, bind = "bind",
unbind = "unbind")
+private volatile PersistenceService persistenceService;
+
Review Comment:
This was refactored to start the relevant healthcheck provider for either
elasticsearch or opensearch based on the configuration.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715: URL: https://github.com/apache/unomi/pull/715#discussion_r2428329868 ## plugins/hover-event/src/main/java/org/apache/unomi/plugins/events/hover/querybuilders/HoverEventConditionESQueryBuilder.java: ## Review Comment: Actually no I think it is nice to have extensions that could package their own query builders, but it's true we are lacking one for Opensearch. I will add it. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426648162
##
docker/README.md:
##
@@ -48,21 +48,66 @@ For ElasticSearch:
docker pull docker.elastic.co/elasticsearch/elasticsearch:7.4.2
docker network create unomi
docker run --name elasticsearch --net unomi -p 9200:9200 -p 9300:9300 -e
"discovery.type=single-node" -e cluster.name=contextElasticSearch
docker.elastic.co/elasticsearch/elasticsearch:7.4.2
+
+For OpenSearch:
+
+docker pull opensearchproject/opensearch:2.18.0
+docker network create unomi
+export OPENSEARCH_ADMIN_PASSWORD=enter_your_custom_admin_password_here
+docker run --name opensearch --net unomi -p 9200:9200 -p 9300:9300 -e
"discovery.type=single-node" -e
OPENSEARCH_INITIAL_ADMIN_PASSWORD=${OPENSEARCH_ADMIN_PASSWORD}
opensearchproject/opensearch:2.18.0
-For Unomi:
+For Unomi (with ElasticSearch):
+
+docker pull apache/unomi:2.7.0-SNAPSHOT
+docker run --name unomi --net unomi -p 8181:8181 -p 9443:9443 -p 8102:8102
\
+-e UNOMI_ELASTICSEARCH_ADDRESSES=elasticsearch:9200 \
+apache/unomi:2.7.0-SNAPSHOT
-docker pull apache/unomi:2.0.0-SNAPSHOT
-docker run --name unomi --net unomi -p 8181:8181 -p 9443:9443 -p 8102:8102
-e UNOMI_ELASTICSEARCH_ADDRESSES=elasticsearch:9200 apache/unomi:2.0.0-SNAPSHOT
+For Unomi (with OpenSearch):
-## Using a host OS ElasticSearch installation (only supported on macOS &
Windows)
+docker pull apache/unomi:2.7.0-SNAPSHOT
+docker run --name unomi --net unomi -p 8181:8181 -p 9443:9443 -p 8102:8102
\
+-e UNOMI_AUTO_START=opensearch \
+-e UNOMI_OPENSEARCH_ADDRESSES=opensearch:9200 \
+-e UNOMI_OPENSEARCH_PASSWORD=${OPENSEARCH_ADMIN_PASSWORD} \
+apache/unomi:2.7.0-SNAPSHOT
+
+## Using a host OS Search Engine installation (only supported on macOS &
Windows)
+
+For ElasticSearch:
-docker run --name unomi -p 8181:8181 -p 9443:9443 -p 8102:8102 -e
UNOMI_ELASTICSEARCH_ADDRESSES=host.docker.internal:9200
apache/unomi:2.0.0-SNAPSHOT
+docker run --name unomi -p 8181:8181 -p 9443:9443 -p 8102:8102 \
+-e UNOMI_ELASTICSEARCH_ADDRESSES=host.docker.internal:9200 \
+apache/unomi:2.7.0-SNAPSHOT
+
+For OpenSearch:
+
+docker run --name unomi -p 8181:8181 -p 9443:9443 -p 8102:8102 \
+-e UNOMI_AUTO_START=opensearch \
+-e UNOMI_OPENSEARCH_ADDRESSES=host.docker.internal:9200 \
+-e UNOMI_OPENSEARCH_PASSWORD=${OPENSEARCH_ADMIN_PASSWORD} \
+apache/unomi:2.7.0-SNAPSHOT
Note: Linux doesn't support the host.docker.internal DNS lookup method yet, it
should be available in an upcoming version of Docker. See
https://github.com/docker/for-linux/issues/264
+## Environment Variables
+
+### Common Variables
+- `UNOMI_AUTO_START`: Specifies the search engine type (`elasticsearch` or
`opensearch`, defaults to `elasticsearch`)
Review Comment:
This was replaced because it is now named a startConfiguration. So basically
we can select start configurations in autostart or start command. Start
configurations are designed to be extendable and customizable in the
org.apache.unomi.start.cfg so that people may for example add their own
features to be started or build additional customizations. For the moment this
is only used for opensearch and elasticsearch but could be useful in the future
for other Unomi extensions or repackagings.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428525692
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/PastEventConditionPersistenceQueryBuilder.java:
##
@@ -0,0 +1,31 @@
+/*
+ * 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.conditions;
+
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.services.DefinitionsService;
+import org.apache.unomi.scripting.ScriptExecutor;
+
+import java.util.Map;
+
+public interface PastEventConditionPersistenceQueryBuilder {
Review Comment:
Will add it. Thanks for catching that.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428360669
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/geo/GeoDistance.java:
##
@@ -0,0 +1,102 @@
+/*
+ * 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.conditions.geo;
+
+public enum GeoDistance {
Review Comment:
Added a TODO on this class as well.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426939132
##
extensions/healthcheck/src/main/resources/org.apache.unomi.healthcheck.cfg:
##
@@ -20,12 +20,20 @@ esAddresses =
${org.apache.unomi.elasticsearch.addresses:-localhost:9200}
esSSLEnabled = ${org.apache.unomi.elasticsearch.sslEnable:-false}
esLogin = ${org.apache.unomi.elasticsearch.username:-}
esPassword = ${org.apache.unomi.elasticsearch.password:-}
-httpClient.trustAllCertificates =
${org.apache.unomi.elasticsearch.sslTrustAllCertificates:-false}
+esHttpClient.trustAllCertificates =
${org.apache.unomi.elasticsearch.sslTrustAllCertificates:-false}
+
+# OpenSearch configuration
+osAddresses = ${org.apache.unomi.opensearch.addresses:-localhost:9200}
+osSSLEnabled = ${org.apache.unomi.opensearch.sslEnable:-true}
+osLogin = ${org.apache.unomi.opensearch.username:-admin}
+osPassword = ${org.apache.unomi.opensearch.password:-}
+osHttpClient.trustAllCertificates =
${org.apache.unomi.opensearch.sslTrustAllCertificates:-true}
+osMinimalClusterState =
${org.apache.unomi.opensearch.minimalClusterState:-GREEN}
# Security configuration
authentication.realm = ${org.apache.unomi.security.realm:-karaf}
# Health check configuration
healthcheck.enabled = ${org.apache.unomi.healthcheck.enabled:-false}
-healthcheck.providers =
${org.apache.unomi.healthcheck.providers:-cluster,elasticsearch,unomi,persistence}
+healthcheck.providers =
${org.apache.unomi.healthcheck.providers:-cluster,elasticsearch,opensearch,unomi,persistence}
Review Comment:
This was refactored to only activated the health check provider for the
configured persistence implementation.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715: URL: https://github.com/apache/unomi/pull/715#discussion_r2428329868 ## plugins/hover-event/src/main/java/org/apache/unomi/plugins/events/hover/querybuilders/HoverEventConditionESQueryBuilder.java: ## Review Comment: Actually no I think it is nice to have extensions that could package their own query builders, but it's true we are lacking one for Opensearch. I will add it. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428517919
##
itests/src/test/java/org/apache/unomi/itests/BaseIT.java:
##
@@ -200,13 +218,28 @@ public void waitForStartup() throws InterruptedException {
httpClient = initHttpClient(getHttpClientCredentialProvider());
}
+private void waitForUnomiManagementService() throws InterruptedException {
+UnomiManagementService unomiManagementService =
getOsgiService(UnomiManagementService.class, 60);
+while (unomiManagementService == null) {
+LOGGER.info("Waiting for Unomi Management Service to be
available...");
+Thread.sleep(1000);
+unomiManagementService =
getOsgiService(UnomiManagementService.class, 60);
+}
Review Comment:
Fixed it by adding a retryCount
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715: URL: https://github.com/apache/unomi/pull/715#discussion_r2428285457 ## itests/README.md: ## @@ -260,3 +281,60 @@ And the final step is, zipping the new version of the snapshot repository and re > In case you are using docker, do zip in the container and use `docker cp` to > get the zip file from the docker container. Now you can modify the migration test class to test that your added data in 1.6.x is correctly migrated in 2.0.0 + +# Known issues + +In the OpenSearch test logs, you will see a lot of lines that look like this : + +opensearch> [2024-12-31T15:33:14,652][WARN ][o.o.w.QueryGroupTask ] [f3200971b164] QueryGroup _id can't be null, It should be set before accessing it. This is abnormal behaviour + +This is due to a bug in OpenSearch 2.18 but it has no impact on the actual functionality. You can track this bug here: + +https://github.com/opensearch-project/OpenSearch/issues/16874 + +## Karaf Tools + +The `kt.sh` script (short for "Karaf Tools") provides convenient utilities for working with Karaf logs and directories during integration testing. Since Karaf test directories are created with unique UUIDs for each test run, this script helps locate and work with the latest test instance. + +### Usage + +```bash +./kt.sh COMMAND [ARGS] +``` + +### Available Commands + +| Command | Alias | Description | +|-|---|---| +| `log` | `l` | View the latest Karaf log file using less| +| `tail` | `t` | Tail the current Karaf log file | +| `grep` | `g` | Grep the latest Karaf log file (requires pattern)| +| `dir` | `d` | Print the latest Karaf directory path| +| `pushd` | `p` | Change to the latest Karaf directory using pushd | +| `help` | `h` | Show help message| Review Comment: This was removed. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428141058
##
persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java:
##
@@ -0,0 +1,2761 @@
+/*
+ * 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.opensearch;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.json.stream.JsonParser;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.http.HttpHost;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.http.util.EntityUtils;
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.query.DateRange;
+import org.apache.unomi.api.query.IpRange;
+import org.apache.unomi.api.query.NumericRange;
+import org.apache.unomi.metrics.MetricAdapter;
+import org.apache.unomi.metrics.MetricsService;
+import org.apache.unomi.persistence.spi.PersistenceService;
+import org.apache.unomi.persistence.spi.aggregate.DateRangeAggregate;
+import org.apache.unomi.persistence.spi.aggregate.IpRangeAggregate;
+import org.apache.unomi.persistence.spi.aggregate.*;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.apache.unomi.persistence.spi.conditions.ConditionEvaluator;
+import
org.apache.unomi.persistence.spi.conditions.ConditionEvaluatorDispatcher;
+import org.opensearch.client.*;
+import org.opensearch.client.json.JsonData;
+import org.opensearch.client.json.JsonpMapper;
+import org.opensearch.client.json.jackson.JacksonJsonpMapper;
+import org.opensearch.client.opensearch.OpenSearchClient;
+import org.opensearch.client.opensearch._types.*;
+import org.opensearch.client.opensearch._types.aggregations.*;
+import org.opensearch.client.opensearch._types.mapping.TypeMapping;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+import org.opensearch.client.opensearch.cluster.HealthRequest;
+import org.opensearch.client.opensearch.cluster.HealthResponse;
+import org.opensearch.client.opensearch.core.*;
+import org.opensearch.client.opensearch.core.bulk.BulkOperation;
+import org.opensearch.client.opensearch.core.bulk.UpdateOperation;
+import org.opensearch.client.opensearch.core.search.Hit;
+import org.opensearch.client.opensearch.core.search.HitsMetadata;
+import org.opensearch.client.opensearch.core.search.TotalHits;
+import org.opensearch.client.opensearch.core.search.TotalHitsRelation;
+import org.opensearch.client.opensearch.indices.*;
+import org.opensearch.client.opensearch.indices.get_alias.IndexAliases;
+import org.opensearch.client.opensearch.tasks.GetTasksResponse;
+import org.opensearch.client.transport.OpenSearchTransport;
+import org.opensearch.client.transport.rest_client.RestClientTransport;
+import org.osgi.framework.*;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
+import java.io.*;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+import java.security.cert.X509Certificate;
+import java.util.*;
+import java.util.stream.Collectors;
+
+@SuppressWarnings("rawtypes")
+public class OpenSearchPersistenceServiceImpl implements PersistenceService,
SynchronousBundleListener {
+
+public static final String SEQ_NO = "seq_no";
+public static final String PRIMARY_TERM = "primary_term";
+
+private static final Logger LOGGER =
LoggerFactory.getLogger(OpenSearchPersistenceServiceImpl.class.getName());
+private static final String ROLLOVER_LIFECYCLE_NAME =
"unomi-rollover-policy";
+
+priv
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2427002078
##
manual/src/main/asciidoc/configuration.adoc:
##
@@ -68,20 +68,53 @@
org.apache.unomi.cluster.public.address=http://localhost:8181
org.apache.unomi.cluster.internal.address=https://localhost:9443
-If you need to specify an ElasticSearch cluster name, or a host and port that
are different than the default,
-it is recommended to do this BEFORE you start the server for the first time,
or you will loose all the data
+If you need to specify a search engine configuration that is different than
the default,
+it is recommended to do this BEFORE you start the server for the first time,
or you will lose all the data
you have stored previously.
-You can use the following properties for the ElasticSearch configuration
+Apache Unomi supports both ElasticSearch and OpenSearch as search engine
backends. Here are the configuration properties for each:
+
+For ElasticSearch:
[source]
org.apache.unomi.elasticsearch.cluster.name=contextElasticSearch
-# The elasticsearch.adresses may be a comma seperated list of host names and
ports such as
+# The elasticsearch.addresses may be a comma separated list of host names and
ports such as
# hostA:9200,hostB:9200
# Note: the port number must be repeated for each host.
org.apache.unomi.elasticsearch.addresses=localhost:9200
+For OpenSearch:
+[source]
+
+org.apache.unomi.opensearch.cluster.name=opensearch-cluster
+# The opensearch.addresses may be a comma separated list of host names and
ports such as
+# hostA:9200,hostB:9200
+# Note: the port number must be repeated for each host.
+org.apache.unomi.opensearch.addresses=localhost:9200
+
+# OpenSearch security settings (required by default since OpenSearch 2.0)
+org.apache.unomi.opensearch.ssl.enable=true
+org.apache.unomi.opensearch.username=admin
+org.apache.unomi.opensearch.password=${env:OPENSEARCH_INITIAL_ADMIN_PASSWORD:-admin}
+org.apache.unomi.opensearch.sslTrustAllCertificates=true
+
+
+To select which search engine to use, you can:
+1. Use the appropriate configuration properties above
+2. When building from source, use the appropriate Maven profile:
+ * For ElasticSearch (default): no special profile needed
+ * For OpenSearch: add `-Duse.opensearch=true` to your Maven command
Review Comment:
I've clarified this part.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on PR #715: URL: https://github.com/apache/unomi/pull/715#issuecomment-3407820589 **Changes since February on OpenSearch contribution** After a LOT of work, I have updated this PR. Hopefully, it will address all the feedback from the reviews and can be merged soon to avoid having to track changes on the master branch again. Here is a summary of the changes: - Merged with the latest changes from the master branch, including ES 9 support and Karaf 4.4.8 (tricky merge!) - Upgraded OpenSearch support to version 3 (was on 2.18 previously) - Removed all the non-OpenSearch related changes - Added missing Javadocs - Addressed all the points raised by the reviewers that were relevant - Renamed all the _queryBuilderIds_ and removed the *ESQueryBuilder postfix in the queryBuilderIds, replacing them with *QueryBuilder. For example, we had queryBuilderId=booleanConditionESQueryBuilder for both OpenSearch and ElasticSearch, so I simply renamed this to booleanConditionQueryBuilder. In order to avoid introducing a breaking change, I added a system to map old queryBuilderIds to new ones that issues a warning if a legacy ID is being used but still allows the old IDs to work transparently. Also documented the system in the migration guide. - Developed integration tests for the queryBuilderId mapping system - Updated the Healthcheck provider to now only report the persistence implementation that is being used - Documented the new start configuration system and explained how it can be used for custom deployments or distributions - Removed the Hover event query builder implementation and simply replaced it with a condition type with a parent condition. This way it can work on both ElasticSearch and OpenSearch. Also added a hover event JSON schema built into the plugin. All the tests are green on GitHub, and the code is ready for review (again)! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428599056
##
graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/condition/parsers/SegmentProfileEventsConditionParser.java:
##
@@ -185,9 +185,16 @@ private Map
createProfileEventPropertyField(final Condition cond
tuple.put("fieldName", "cdp_timestamp_gte");
}
-final OffsetDateTime fieldValue =
DateUtils.offsetDateTimeFromMap((Map)
condition.getParameter("propertyValueDate"));
-
-tuple.put("fieldValue", fieldValue != null ? fieldValue.toString()
: null);
+Object propertyValueDate =
condition.getParameter("propertyValueDate");
+if (propertyValueDate == null) {
+tuple.put("fieldValue", null);
+} else if (propertyValueDate instanceof Map){
+// This shouldn't be needed since Jackson was upgraded to >
2.13, but we keep it for backwards compatibility with older data sets
+final OffsetDateTime fieldValue =
DateUtils.offsetDateTimeFromMap((Map) propertyValueDate);
+tuple.put("fieldValue", fieldValue != null ?
fieldValue.toString() : null);
+} else {
+tuple.put("fieldValue", propertyValueDate.toString());
+}
Review Comment:
I've removed this fix. It was identified during manual testing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428598159
##
itests/src/test/java/org/apache/unomi/itests/JSONSchemaIT.java:
##
@@ -340,7 +339,14 @@ public void testFlattenedProperties() throws Exception {
condition.setParameter("propertyName",
"flattenedProperties.interests.cars");
condition.setParameter("comparisonOperator", "greaterThan");
condition.setParameter("propertyValueInteger", 2);
-assertNull(persistenceService.query(condition, null, Event.class, 0,
-1));
+// OpenSearch handles flattened fields differently than Elasticsearch
+if ("opensearch".equals(searchEngine)) {
+assertNotNull("OpenSearch should return results for flattened
properties",
+persistenceService.query(condition, null, Event.class, 0, -1));
+} else {
+assertNull("Elasticsearch should return null for flattened
properties",
+persistenceService.query(condition, null, Event.class, 0, -1));
+}
Review Comment:
It's just that one returns null and the other empty results. Shouldn't be a
big problem.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428594975
##
rest/src/main/java/org/apache/unomi/rest/service/impl/RestServiceUtilsImpl.java:
##
@@ -145,7 +145,13 @@ public EventsRequestContext initEventsRequest(String
scope, String sessionId, St
// Session user has been switched, profile id in
cookie is not up to date
// We must reload the profile with the session ID as
some properties could be missing from the session profile
// #personalIdentifier
-
eventsRequestContext.setProfile(profileService.load(sessionProfile.getItemId()));
+Profile sessionProfileWithId =
profileService.load(sessionProfile.getItemId());
+if (sessionProfileWithId != null) {
+
eventsRequestContext.setProfile(sessionProfileWithId);
+} else {
+LOGGER.warn("Couldn't find profile ID {}
referenced from session with ID {}, so we re-create it",
sessionProfile.getItemId(), sessionId);
+
eventsRequestContext.setProfile(createNewProfile(sessionProfile.getItemId(),
timestamp));
+}
Review Comment:
I have removed this change from this 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428527405
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/DateUtils.java:
##
@@ -0,0 +1,54 @@
+/*
+ * 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.conditions;
+
+import
org.apache.unomi.persistence.spi.conditions.datemath.DateMathParseException;
+import org.apache.unomi.persistence.spi.conditions.datemath.DateMathParser;
+import org.apache.unomi.persistence.spi.conditions.datemath.JavaDateFormatter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.Date;
+
+public class DateUtils {
Review Comment:
Will add it thanks.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428311658
##
itests/src/test/java/org/apache/unomi/itests/BaseIT.java:
##
@@ -223,12 +256,34 @@ protected void refreshPersistence(final Class... classes) throws
@Override
public MavenArtifactUrlReference getKarafDistribution() {
-return
CoreOptions.maven().groupId("org.apache.unomi").artifactId("unomi").versionAsInProject().type("tar.gz");
+return
maven().groupId("org.apache.unomi").artifactId("unomi").versionAsInProject().type("tar.gz");
}
@Configuration
public Option[] config() {
System.out.println(" Configuring container");
+
+searchEngine = System.getProperty(SEARCH_ENGINE_PROPERTY,
SEARCH_ENGINE_ELASTICSEARCH);
+System.out.println("Search Engine: " + searchEngine);
+
+// Define features option based on search engine
+Option featuresOption;
+if (SEARCH_ENGINE_ELASTICSEARCH.equals(searchEngine)) {
+featuresOption = features(maven().groupId("org.apache.unomi")
+
.artifactId("unomi-kar").versionAsInProject().type("xml").classifier("features"),
+"unomi-persistence-elasticsearch", "unomi-services",
+"unomi-router-karaf-feature", "unomi-groovy-actions",
+"unomi-web-applications", "unomi-rest-ui",
"unomi-healthcheck", "cdp-graphql-feature");
+} else if (SEARCH_ENGINE_OPENSEARCH.equals(searchEngine)) {
+featuresOption = features(maven().groupId("org.apache.unomi")
+
.artifactId("unomi-kar").versionAsInProject().type("xml").classifier("features"),
+"unomi-persistence-opensearch", "unomi-services",
+"unomi-router-karaf-feature", "unomi-groovy-actions",
+"unomi-web-applications", "unomi-rest-ui",
"unomi-healthcheck", "cdp-graphql-feature");
+} else {
+throw new IllegalArgumentException("Unknown search engine: " +
searchEngine);
+}
Review Comment:
I've changed the code since then, and despite having two similar lists, I
tried refactoring it and it made it harder to read and maintain.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428304154
##
package/src/main/resources/etc/custom.system.properties:
##
@@ -165,6 +165,80 @@
org.apache.unomi.elasticsearch.password=${env:UNOMI_ELASTICSEARCH_PASSWORD:-}
org.apache.unomi.elasticsearch.sslEnable=${env:UNOMI_ELASTICSEARCH_SSL_ENABLE:-false}
org.apache.unomi.elasticsearch.sslTrustAllCertificates=${env:UNOMI_ELASTICSEARCH_SSL_TRUST_ALL_CERTIFICATES:-false}
+###
+## OpenSearch settings
##
+###
+org.apache.unomi.opensearch.cluster.name=${env:UNOMI_OPENSEARCH_CLUSTERNAME:-opensearch-cluster}
+# The opensearch.addresses may be a comma seperated list of host names and
ports such as
+# hostA:9200,hostB:9200
+# Note: the port number must be repeated for each host.
+org.apache.unomi.opensearch.addresses=${env:UNOMI_OPENSEARCH_ADDRESSES:-localhost:9200}
+# refresh policy per item type in Json.
+# Valid values are WAIT_UNTIL/IMMEDIATE/NONE. The default refresh policy is
NONE.
+# Example: "{"event":"WAIT_UNTIL","rule":"NONE"}
+org.apache.unomi.opensearch.itemTypeToRefreshPolicy=${env:UNOMI_OPENSEARCH_REFRESH_POLICY_PER_ITEM_TYPE:-}
+org.apache.unomi.opensearch.fatalIllegalStateErrors=${env:UNOMI_OPENSEARCH_FATAL_STATE_ERRORS:-}
+org.apache.unomi.opensearch.index.prefix=${env:UNOMI_OPENSEARCH_INDEXPREFIX:-context}
+
+# These monthlyIndex properties are now deprecated, please use rollover
equivalent.
Review Comment:
Thanks for catching this. I fixed this, I've removed them.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428298779
##
itests/src/test/java/org/apache/unomi/itests/RuleServiceIT.java:
##
@@ -80,16 +80,27 @@ public void testRuleWithNullActions() throws
InterruptedException {
public void getAllRulesShouldReturnAllRulesAvailable() throws
InterruptedException {
String ruleIDBase = "moreThan50RuleTest";
int originalRulesNumber = rulesService.getAllRules().size();
+
+// Create a simple condition instead of null
+Condition defaultCondition = new
Condition(definitionsService.getConditionType("matchAllCondition"));
+
+// Create a default action
+Action defaultAction = new
Action(definitionsService.getActionType("setPropertyAction"));
+defaultAction.setParameter("propertyName", "testProperty");
+defaultAction.setParameter("propertyValue", "testValue");
+List actions = Collections.singletonList(defaultAction);
+
+
for (int i = 0; i < 60; i++) {
String ruleID = ruleIDBase + "_" + i;
Metadata metadata = new Metadata(ruleID);
metadata.setName(ruleID);
metadata.setDescription(ruleID);
metadata.setScope(TEST_SCOPE);
-Rule nullRule = new Rule(metadata);
-nullRule.setCondition(null);
-nullRule.setActions(null);
-createAndWaitForRule(nullRule);
+Rule rule = new Rule(metadata);
+rule.setCondition(defaultCondition); // Use default condition
instead of null
+rule.setActions(actions); // Empty list instead of null
Review Comment:
Fixed
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428262543
##
docker/src/main/docker/entrypoint.sh:
##
@@ -16,28 +16,110 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-# Wait for healthy ElasticSearch
-# next wait for ES status to turn to Green
-if [ "$UNOMI_ELASTICSEARCH_SSL_ENABLE" == 'true' ]; then
- schema='https'
+# Near the top of the file, add these default debug settings
+KARAF_DEBUG=${KARAF_DEBUG:-false}
+KARAF_DEBUG_PORT=${KARAF_DEBUG_PORT:-5005}
+KARAF_DEBUG_SUSPEND=${KARAF_DEBUG_SUSPEND:-n}
+
+# Before starting Karaf, add debug configuration
+if [ "$KARAF_DEBUG" = "true" ]; then
+echo "Enabling Karaf debug mode on port $KARAF_DEBUG_PORT
(suspend=$KARAF_DEBUG_SUSPEND)"
+export
JAVA_DEBUG_OPTS="-agentlib:jdwp=transport=dt_socket,server=y,suspend=$KARAF_DEBUG_SUSPEND,address=*:$KARAF_DEBUG_PORT"
+export KARAF_DEBUG=true
+fi
+
+# Determine search engine type from UNOMI_AUTO_START
+SEARCH_ENGINE="${UNOMI_AUTO_START:-elasticsearch}"
+export KARAF_OPTS="-Dunomi.autoStart=${UNOMI_AUTO_START}"
+
+if [ "$SEARCH_ENGINE" = "true" ]; then
+SEARCH_ENGINE="elasticsearch"
+fi
+echo "SEARCH_ENGINE: $SEARCH_ENGINE"
+echo "KARAF_OPTS: $KARAF_OPTS"
+
+# Function to check cluster health for a specific node
+check_node_health() {
+local node_url="$1"
+local curl_opts="$2"
+response=$(eval curl -v -fsSL ${curl_opts} "${node_url}" 2>&1)
+if [ $? -eq 0 ]; then
+echo "$response" | grep -o '"status"[ ]*:[ ]*"[^"]*"' | cut -d'"' -f4
+else
+echo ""
+fi
+}
+
+# Configure connection parameters based on search engine type
+if [ "$SEARCH_ENGINE" = "opensearch" ]; then
+# OpenSearch configuration
+if [ -z "$UNOMI_OPENSEARCH_PASSWORD" ]; then
+echo "Error: UNOMI_OPENSEARCH_PASSWORD must be set when using
OpenSearch"
+exit 1
+fi
+
+schema='https'
+auth_header="Authorization: Basic $(echo -n
"admin:${UNOMI_OPENSEARCH_PASSWORD}" | base64)"
+health_endpoint="_cluster/health"
+curl_opts="-k -H \"${auth_header}\" -H \"Content-Type: application/json\""
else
- schema='http'
+# Elasticsearch configuration
+if [ "$UNOMI_ELASTICSEARCH_SSL_ENABLE" = 'true' ]; then
+schema='https'
+else
+schema='http'
+fi
+
+if [ -v UNOMI_ELASTICSEARCH_USERNAME ] && [ -v
UNOMI_ELASTICSEARCH_PASSWORD ]; then
+auth_header="Authorization: Basic $(echo -n
"${UNOMI_ELASTICSEARCH_USERNAME}:${UNOMI_ELASTICSEARCH_PASSWORD}" | base64)"
+curl_opts="-H \"${auth_header}\""
+fi
+health_endpoint="_cluster/health"
fi
-if [ -v UNOMI_ELASTICSEARCH_USERNAME ] && [ -v UNOMI_ELASTICSEARCH_PASSWORD ];
then
-
elasticsearch_addresses="$schema://$UNOMI_ELASTICSEARCH_USERNAME:$UNOMI_ELASTICSEARCH_PASSWORD@$UNOMI_ELASTICSEARCH_ADDRESSES/_cat/health?h=status"
+# Build array of node URLs
+if [ "$SEARCH_ENGINE" = "opensearch" ]; then
Review Comment:
Thanks for catching this, I fixed it.
##
extensions/web-tracker/wab/yarn.lock:
##
@@ -4,27 +4,27 @@
"@ampproject/remapping@^2.1.0":
version "2.2.0"
- resolved
"https://registry.yarnpkg.com/@ampproject/remapping/-/remapping-2.2.0.tgz#56c133824780de3174aed5ab6834f3026790154d";
Review Comment:
Not sure no, probably something yarn did by itself.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428139919
##
persistence-opensearch/conditions/src/main/java/org/apache/unomi/persistence/opensearch/querybuilders/advanced/IdsConditionOSQueryBuilder.java:
##
@@ -0,0 +1,53 @@
+/*
+ * 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.opensearch.conditions;
+
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilder;
+import
org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilderDispatcher;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Map;
+
+public class IdsConditionOSQueryBuilder implements ConditionOSQueryBuilder {
+
+private int maximumIdsQueryCount = 5000;
+
+public void setMaximumIdsQueryCount(int maximumIdsQueryCount) {
+this.maximumIdsQueryCount = maximumIdsQueryCount;
+}
+
+@Override
+public Query buildQuery(Condition condition, Map context,
ConditionOSQueryBuilderDispatcher dispatcher) {
+Collection ids = (Collection)
condition.getParameter("ids");
+Boolean match = (Boolean) condition.getParameter("match");
+
+if (ids.size() > maximumIdsQueryCount) {
+// Avoid building too big ids query - throw exception instead
+throw new UnsupportedOperationException("Too many profiles");
Review Comment:
I've improved the exception message to include the limit, I also did the
change in the ElasticSearch implementation.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2428094420
##
services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java:
##
@@ -127,10 +116,17 @@ public String authenticateThirdPartyServer(String key,
String ip) {
for (Map.Entry entry :
thirdPartyServers.entrySet()) {
ThirdPartyServer server = entry.getValue();
if (server.getKey().equals(key)) {
-IPAddress ipAddress = new IPAddressString(ip).getAddress();
-for (IPAddress serverIpAddress : server.getIpAddresses()) {
-if (serverIpAddress.contains(ipAddress)) {
-return server.getId();
+// This is a fix to support proper IPv6 parsing
+if (ip != null) {
+if (ip.startsWith("[") && ip.endsWith("]")) {
+// This can happen with IPv6 addresses, we must
remove the markers since our IPAddress library doesn't support them.
+ip = ip.substring(1, ip.length() - 1);
+}
+IPAddress ipAddress = new
IPAddressString(ip).getAddress();
+for (IPAddress serverIpAddress :
server.getIpAddresses()) {
+if (serverIpAddress.contains(ipAddress)) {
+return server.getId();
+}
Review Comment:
I've removed it from this 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426984859
##
itests/src/test/java/org/apache/unomi/itests/migration/Migrate16xToCurrentVersionIT.java:
##
@@ -59,16 +79,31 @@ public void waitForStartup() throws InterruptedException {
// Restore the snapshot
HttpUtils.executePostRequest(httpClient,
"http://localhost:9400/_snapshot/snapshots_repository/snapshot_1.6.x/_restore?wait_for_completion=true";,
"{}", null);
+String snapshotStatus = HttpUtils.executeGetRequest(httpClient,
"http://localhost:9400/_snapshot/_status";, null);
+System.out.println(snapshotStatus);
+LOGGER.info(snapshotStatus);
+
Review Comment:
It is helpful if it fails yes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426977769
##
itests/src/test/java/org/apache/unomi/itests/ProgressSuite.java:
##
@@ -0,0 +1,81 @@
+/*
+ * 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.junit.Test;
+import org.junit.runner.Description;
+import org.junit.runner.notification.RunNotifier;
+import org.junit.runners.Suite;
+import org.junit.runners.model.InitializationError;
+
+import java.lang.reflect.Method;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class ProgressSuite extends Suite {
Review Comment:
I've added Javadoc documentation and as for the other class it would be nice
to keep this in this PR since it's helpful when running the integration tests
to understand their progress.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426972238
##
itests/src/test/java/org/apache/unomi/itests/ProgressListener.java:
##
@@ -0,0 +1,275 @@
+/*
+ * 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.junit.runner.Description;
+import org.junit.runner.Result;
+import org.junit.runner.notification.Failure;
+import org.junit.runner.notification.RunListener;
+
+import java.util.PriorityQueue;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class ProgressListener extends RunListener {
Review Comment:
This class help follow the execution progress of integration tests, adding a
progress bar that indicates how many tests have been completed, how much
remaining time until the tests completed, and the number of successful and
failed tests. It also does some reporting at the end of the tests on the
slowest test methods.
We could move it to another PR but it has been incredibly useful in checking
what is going on in the OpenSearch/ElasticSearch tests I'd like to keep this
one here :) I've removed all the other stuff that's not related to OpenSearch.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426960231
##
itests/src/test/java/org/apache/unomi/itests/BaseIT.java:
##
@@ -309,6 +375,9 @@ public Option[] config() {
karafOptions.add(editConfigurationFilePut("etc/org.ops4j.pax.logging.cfg",
"log4j2.logger.customLogging.level", customLoggingParts[1]));
}
+searchEngine = System.getProperty(SEARCH_ENGINE_PROPERTY,
SEARCH_ENGINE_ELASTICSEARCH);
+System.out.println("Search Engine: " + searchEngine);
Review Comment:
I've added outputing to both so that we get the output both in the console
and the logs.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426942596
##
itests/src/test/java/org/apache/unomi/itests/BaseIT.java:
##
@@ -164,10 +167,25 @@ public abstract class BaseIT extends KarafTestSupport {
public void waitForStartup() throws InterruptedException {
// disable retry
retry = new KarafTestSupport.Retry(false);
+searchEngine = System.getProperty(SEARCH_ENGINE_PROPERTY,
SEARCH_ENGINE_ELASTICSEARCH);
// Start Unomi if not already done
if (!unomiStarted) {
-executeCommand("unomi:start");
+// We must check that the Unomi Management Service is up and
running before launching the
+// command otherwise the start configuration will not be properly
populated.
+waitForUnomiManagementService();
+if (SEARCH_ENGINE_ELASTICSEARCH.equals(searchEngine)) {
+LOGGER.info("Starting Unomi with elasticsearch search
engine...");
+System.out.println(" Starting Unomi with elasticsearch
search engine...");
+executeCommand("unomi:start");
+} else if (SEARCH_ENGINE_OPENSEARCH.equals(searchEngine)){
+LOGGER.info("Starting Unomi with opensearch search engine...");
+System.out.println(" Starting Unomi with opensearch search
engine...");
+executeCommand("unomi:start " + SEARCH_ENGINE_OPENSEARCH);
Review Comment:
As mentioned before, this is now a start configuration system that is quite
generic and I think it makes more sense as it configured which features are
started. It is also documented.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r2426650982
##
tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/commands/ActionRemove.java:
##
@@ -0,0 +1,45 @@
+/*
+ * 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.shell.commands;
+
+import org.apache.karaf.shell.api.action.Argument;
+import org.apache.karaf.shell.api.action.Command;
+import org.apache.karaf.shell.api.action.lifecycle.Reference;
+import org.apache.karaf.shell.api.action.lifecycle.Service;
+import org.apache.unomi.api.services.DefinitionsService;
+
+@Command(scope = "unomi", name = "action-remove", description = "This command
will remove an action type.")
+@Service
+public class ActionRemove extends RemoveCommandSupport {
Review Comment:
This is removed from this branch now.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1947937002
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/datemath/JavaDateFormatter.java:
##
@@ -0,0 +1,351 @@
+/*
+ * 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.conditions.datemath;
+
+import java.time.*;
+import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeFormatterBuilder;
+import java.time.temporal.ChronoField;
+import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalQueries;
+import java.util.ArrayList;
+import java.util.List;
+
+public class JavaDateFormatter {
Review Comment:
I agree the context is missing. Basically the idea was to have a 100%
compatible implementation with the existing Elasticsearch ones. Introducing new
libraries here would have introduced new issues.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on PR #715: URL: https://github.com/apache/unomi/pull/715#issuecomment-2645919064 Hello @jkevan thank you for your review. I will look at your feedback but to answer some of your questions globally : - I am aware there are some things that don't seem directly related to the OpenSearch work, however ALL of them were necessary to get OpenSearch to work properly, and some where tools that were developped while working on the project to make sure everything worked. It made more sense to include them in the project since separating them would take more time. I do understand the fact that your preference is to have individual changes but on large modifications such as this this actually wastes time that would be better used at moving the project forward. - I will add some JavaDocs as I agree some are missing - About the re-implementation of some classes, these were necessary for the following reasons : stay 100% compatible with the existing implementation. Using other implementations such as the JSR 385 backport would have added possible changes in behavior. A lot of code had to be centralized and therefore new interface had to be created to make sure that the code was properly reusable between the two. - For the missing tests, actually I have been working on a new REST API test framework that includes testing of some of the new methods but I haven't been sharing that yet. The current integration tests are a nightmare to work with, especially on such a large change. - About the changes to features, this was not a change I really wanted to make at this point but was forced to do because there was really no clean way to fix the startup with conditional bundles depending on the persistence implementation. The unomi:start implementation was a mess from the start and was already duplicating a lot of the functionality of Karaf features and ideally should be completely removed to just use features. I actually tried to do that but I ran into BIG problems with the pax-web implementation that wouldn't restart properly. This is why I had to set all the bundles to start=false in the feature and then start them otherwise the system wouldn't start properly. I'm hoping this will be fixed when upgrading to a more recent Karaf but this was the only way I managed to get things to work properly for both Elasticsearch and OpenSearch. - As for the parameter on the unomi:start command this was actually very useful when developing as it made it very easy to start and stop Unomi without starting Karaf to switch between the ElasticSearch and OpenSearch implementations. As it is integrated with the autostart configuration it is also possible to set this up before the start and this has been implemented for the Docker images. - Did you watch the recording of the meeting I did to present the changes? A lot of the questions you had were presented in that meeting, which I specifically organized to explain the changes and give people opportunities to ask questions and give early feedback. You can find the video here : https://www.youtube.com/watch?v=uNbKW29FCnE . I will be organizing more of these as I am actively working on some more important contributions to the project and I have been sharing with the community at the monthly meetings as well. - Last but not least, as I have been working on other major things such as multi-tenancy and other cleanups such as the removal of the clustering code, I am beginning to think that maybe we should move these changes to be part of Unomi V3, which is a branch I have already started working on, and I have also shared my current work in the mailing lists and in multiple meetings. These big changes will slow down quickly however, as I am working on making Unomi much cheaper to deploy and operate. OpenSearch and multi-tenancy are the biggest requirements on this area, along with some cleanups that I would like to do. I am going to communicate some proposals on the subject, but I really need to get these things implemented for my own needs as well as some other users of Unomi. If you are interested in the multi-tenancy, please check my email in the mailing list about it : https://lists.apache.org/[email protected] -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1947936715
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/CustomObjectMapper.java:
##
@@ -58,6 +58,7 @@ public CustomObjectMapper() {
public CustomObjectMapper(Map> deserializers) {
super();
super.registerModule(new JaxbAnnotationModule());
+super.registerModule(new
com.fasterxml.jackson.datatype.jsr310.JavaTimeModule());
Review Comment:
Actually this was required due to the Jackson upgrade that doesn't include
this module by default anymore. I will add a comment to explain that.
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/ConditionEvaluatorDispatcher.java:
##
@@ -0,0 +1,32 @@
+/*
+ * 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.conditions;
+
+import org.apache.unomi.api.Item;
+import org.apache.unomi.api.conditions.Condition;
+
+import java.util.Map;
+
+public interface ConditionEvaluatorDispatcher {
Review Comment:
Indeed, will add it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1947936715
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/CustomObjectMapper.java:
##
@@ -58,6 +58,7 @@ public CustomObjectMapper() {
public CustomObjectMapper(Map> deserializers) {
super();
super.registerModule(new JaxbAnnotationModule());
+super.registerModule(new
com.fasterxml.jackson.datatype.jsr310.JavaTimeModule());
Review Comment:
Actually this was required when working with more recent JDKs.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1947936645
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/datemath/DateMathParser.java:
##
@@ -0,0 +1,270 @@
+/*
+ * 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.conditions.datemath;
+
+import java.time.*;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoField;
+import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalAdjusters;
+import java.time.temporal.TemporalQueries;
+import java.util.function.Function;
+import java.util.function.LongSupplier;
+
+public class DateMathParser {
Review Comment:
I couldn't find a 100% compatible DateMathParser that would work exactly the
same way as the Elasticsearch one, which is why this was done. But I agree the
Javadoc is missing I will add it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
jkevan commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1940871518
##
persistence-spi/src/main/java/org/apache/unomi/persistence/spi/conditions/datemath/JavaDateFormatter.java:
##
@@ -0,0 +1,351 @@
+/*
+ * 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.conditions.datemath;
+
+import java.time.*;
+import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeFormatterBuilder;
+import java.time.temporal.ChronoField;
+import java.time.temporal.TemporalAccessor;
+import java.time.temporal.TemporalQueries;
+import java.util.ArrayList;
+import java.util.List;
+
+public class JavaDateFormatter {
Review Comment:
What is the reason for re-implementing a Java date formatter, what was wrong
with the previous `JodaDateMathParser` ?
(if it was using the joda package from elasticsearch, it could be replace by
real java time API)
A bit of context with comments/java doc would be greatly appreciate the
necessity of this new implem.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
jkevan commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1939818900
##
package/src/main/resources/etc/custom.system.properties:
##
@@ -165,6 +165,80 @@
org.apache.unomi.elasticsearch.password=${env:UNOMI_ELASTICSEARCH_PASSWORD:-}
org.apache.unomi.elasticsearch.sslEnable=${env:UNOMI_ELASTICSEARCH_SSL_ENABLE:-false}
org.apache.unomi.elasticsearch.sslTrustAllCertificates=${env:UNOMI_ELASTICSEARCH_SSL_TRUST_ALL_CERTIFICATES:-false}
+###
+## OpenSearch settings
##
+###
+org.apache.unomi.opensearch.cluster.name=${env:UNOMI_OPENSEARCH_CLUSTERNAME:-opensearch-cluster}
+# The opensearch.addresses may be a comma seperated list of host names and
ports such as
+# hostA:9200,hostB:9200
+# Note: the port number must be repeated for each host.
+org.apache.unomi.opensearch.addresses=${env:UNOMI_OPENSEARCH_ADDRESSES:-localhost:9200}
+# refresh policy per item type in Json.
+# Valid values are WAIT_UNTIL/IMMEDIATE/NONE. The default refresh policy is
NONE.
+# Example: "{"event":"WAIT_UNTIL","rule":"NONE"}
+org.apache.unomi.opensearch.itemTypeToRefreshPolicy=${env:UNOMI_OPENSEARCH_REFRESH_POLICY_PER_ITEM_TYPE:-}
+org.apache.unomi.opensearch.fatalIllegalStateErrors=${env:UNOMI_OPENSEARCH_FATAL_STATE_ERRORS:-}
+org.apache.unomi.opensearch.index.prefix=${env:UNOMI_OPENSEARCH_INDEXPREFIX:-context}
+
+# These monthlyIndex properties are now deprecated, please use rollover
equivalent.
Review Comment:
Already deprecated props could probably not be re-add to the open search
implem ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
jkevan commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1939806425
##
itests/src/test/resources/org.apache.unomi.healthcheck.cfg:
##
@@ -22,10 +22,18 @@ esLogin = ${org.apache.unomi.elasticsearch.username:-}
esPassword = ${org.apache.unomi.elasticsearch.password:-}
httpClient.trustAllCertificates =
${org.apache.unomi.elasticsearch.sslTrustAllCertificates:-false}
+# OpenSearch configuration
+osAddresses = ${org.apache.unomi.opensearch.addresses:-localhost:9200}
+osSSLEnabled = ${org.apache.unomi.opensearch.sslEnable:-false}
+osLogin = ${org.apache.unomi.opensearch.username:-admin}
+osPassword = ${org.apache.unomi.opensearch.password:-}
+osMinimalClusterState =
${org.apache.unomi.opensearch.minimalClusterState:-YELLOW}
+httpClient.trustAllCertificates =
${org.apache.unomi.opensearch.sslTrustAllCertificates:-false}
Review Comment:
is it not conflicting with the es setting ?
it looks like it will override ES settings. may be safer to use an other
name for `httpClient.trustAllCertificates`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
jkevan commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1939679779
##
services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java:
##
@@ -127,10 +116,17 @@ public String authenticateThirdPartyServer(String key,
String ip) {
for (Map.Entry entry :
thirdPartyServers.entrySet()) {
ThirdPartyServer server = entry.getValue();
if (server.getKey().equals(key)) {
-IPAddress ipAddress = new IPAddressString(ip).getAddress();
-for (IPAddress serverIpAddress : server.getIpAddresses()) {
-if (serverIpAddress.contains(ipAddress)) {
-return server.getId();
+// This is a fix to support proper IPv6 parsing
+if (ip != null) {
+if (ip.startsWith("[") && ip.endsWith("]")) {
+// This can happen with IPv6 addresses, we must
remove the markers since our IPAddress library doesn't support them.
+ip = ip.substring(1, ip.length() - 1);
+}
+IPAddress ipAddress = new
IPAddressString(ip).getAddress();
+for (IPAddress serverIpAddress :
server.getIpAddresses()) {
+if (serverIpAddress.contains(ipAddress)) {
+return server.getId();
+}
Review Comment:
This fix looks ok, but what is the link with open search support ?
If it's fixing a bug or issue, would it be more appropriate to separate it
in a dedicated PR/tests + tickets.
(current PR is already quiet huge, making it difficult to review, I think
that additional fixes not related to open search implem should not be part of
the current PR.)
##
graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/condition/parsers/SegmentProfileEventsConditionParser.java:
##
@@ -185,9 +185,16 @@ private Map
createProfileEventPropertyField(final Condition cond
tuple.put("fieldName", "cdp_timestamp_gte");
}
-final OffsetDateTime fieldValue =
DateUtils.offsetDateTimeFromMap((Map)
condition.getParameter("propertyValueDate"));
-
-tuple.put("fieldValue", fieldValue != null ? fieldValue.toString()
: null);
+Object propertyValueDate =
condition.getParameter("propertyValueDate");
+if (propertyValueDate == null) {
+tuple.put("fieldValue", null);
+} else if (propertyValueDate instanceof Map){
+// This shouldn't be needed since Jackson was upgraded to >
2.13, but we keep it for backwards compatibility with older data sets
+final OffsetDateTime fieldValue =
DateUtils.offsetDateTimeFromMap((Map) propertyValueDate);
+tuple.put("fieldValue", fieldValue != null ?
fieldValue.toString() : null);
+} else {
+tuple.put("fieldValue", propertyValueDate.toString());
+}
Review Comment:
Is this related to OpenSearch implem support ?
or is it a separate additional fix ?
##
itests/src/test/java/org/apache/unomi/itests/ProgressListener.java:
##
@@ -0,0 +1,275 @@
+/*
+ * 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.junit.runner.Description;
+import org.junit.runner.Result;
+import org.junit.runner.notification.Failure;
+import org.junit.runner.notification.RunListener;
+
+import java.util.PriorityQueue;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class ProgressListener extends RunListener {
Review Comment:
What is the goal of this new class/runner ?
A bit of java doc would be welcome to understand it.
It's looks to be a nice addition to integration tests (even if I dont get
exactly what it does), but I dont see the relation to open search support.
Due to complexity of current PR that contains a lot of stuff I would greatly
appreciate separation between the open search support and the rest that is not
related.
##
api/src/main/java/org/apa
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1940893932
##
persistence-opensearch/core/src/main/java/org/apache/unomi/persistence/opensearch/OpenSearchPersistenceServiceImpl.java:
##
@@ -0,0 +1,2761 @@
+/*
+ * 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.opensearch;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.json.stream.JsonParser;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.http.HttpHost;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.http.util.EntityUtils;
+import org.apache.unomi.api.*;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.api.query.DateRange;
+import org.apache.unomi.api.query.IpRange;
+import org.apache.unomi.api.query.NumericRange;
+import org.apache.unomi.metrics.MetricAdapter;
+import org.apache.unomi.metrics.MetricsService;
+import org.apache.unomi.persistence.spi.PersistenceService;
+import org.apache.unomi.persistence.spi.aggregate.DateRangeAggregate;
+import org.apache.unomi.persistence.spi.aggregate.IpRangeAggregate;
+import org.apache.unomi.persistence.spi.aggregate.*;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.apache.unomi.persistence.spi.conditions.ConditionEvaluator;
+import
org.apache.unomi.persistence.spi.conditions.ConditionEvaluatorDispatcher;
+import org.opensearch.client.*;
+import org.opensearch.client.json.JsonData;
+import org.opensearch.client.json.JsonpMapper;
+import org.opensearch.client.json.jackson.JacksonJsonpMapper;
+import org.opensearch.client.opensearch.OpenSearchClient;
+import org.opensearch.client.opensearch._types.*;
+import org.opensearch.client.opensearch._types.aggregations.*;
+import org.opensearch.client.opensearch._types.mapping.TypeMapping;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+import org.opensearch.client.opensearch.cluster.HealthRequest;
+import org.opensearch.client.opensearch.cluster.HealthResponse;
+import org.opensearch.client.opensearch.core.*;
+import org.opensearch.client.opensearch.core.bulk.BulkOperation;
+import org.opensearch.client.opensearch.core.bulk.UpdateOperation;
+import org.opensearch.client.opensearch.core.search.Hit;
+import org.opensearch.client.opensearch.core.search.HitsMetadata;
+import org.opensearch.client.opensearch.core.search.TotalHits;
+import org.opensearch.client.opensearch.core.search.TotalHitsRelation;
+import org.opensearch.client.opensearch.indices.*;
+import org.opensearch.client.opensearch.indices.get_alias.IndexAliases;
+import org.opensearch.client.opensearch.tasks.GetTasksResponse;
+import org.opensearch.client.transport.OpenSearchTransport;
+import org.opensearch.client.transport.rest_client.RestClientTransport;
+import org.osgi.framework.*;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
+import java.io.*;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+import java.security.cert.X509Certificate;
+import java.util.*;
+import java.util.stream.Collectors;
+
+@SuppressWarnings("rawtypes")
+public class OpenSearchPersistenceServiceImpl implements PersistenceService,
SynchronousBundleListener {
+
+public static final String SEQ_NO = "seq_no";
+public static final String PRIMARY_TERM = "primary_term";
+
+private static final Logger LOGGER =
LoggerFactory.getLogger(OpenSearchPersistenceServiceImpl.class.getName());
+private static final String ROLLOVER_LIFECYCLE_NAME =
"unomi-rollover-policy";
+
+priv
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1940892929
##
persistence-opensearch/conditions/src/main/java/org/apache/unomi/persistence/opensearch/conditions/PropertyConditionOSQueryBuilder.java:
##
@@ -0,0 +1,255 @@
+/*
+ * 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.opensearch.conditions;
+
+import org.apache.commons.lang3.ObjectUtils;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilder;
+import
org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilderDispatcher;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormatter;
+import org.joda.time.format.ISODateTimeFormat;
+import org.opensearch.client.json.JsonData;
+import org.opensearch.client.opensearch._types.FieldValue;
+import org.opensearch.client.opensearch._types.GeoDistanceType;
+import org.opensearch.client.opensearch._types.query_dsl.BoolQuery;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+import org.opensearch.client.util.ObjectBuilder;
+
+import java.time.OffsetDateTime;
+import java.util.*;
+
+import static org.apache.unomi.persistence.spi.conditions.DateUtils.getDate;
+
+public class PropertyConditionOSQueryBuilder implements
ConditionOSQueryBuilder {
+
+DateTimeFormatter dateTimeFormatter;
+
+public PropertyConditionOSQueryBuilder() {
+dateTimeFormatter = ISODateTimeFormat.dateTime();
+}
+
+@Override
+public Query buildQuery(Condition condition, Map context,
ConditionOSQueryBuilderDispatcher dispatcher) {
+String comparisonOperator = (String)
condition.getParameter("comparisonOperator");
+String name = (String) condition.getParameter("propertyName");
+
+if (comparisonOperator == null || name == null) {
+throw new IllegalArgumentException("Impossible to build ES filter,
condition is not valid, comparisonOperator and propertyName properties should
be provided");
+}
+
+String expectedValue = ConditionContextHelper.foldToASCII((String)
condition.getParameter("propertyValue"));
+Object expectedValueInteger =
condition.getParameter("propertyValueInteger");
+Object expectedValueDouble =
condition.getParameter("propertyValueDouble");
+Object expectedValueDate =
convertDateToISO(condition.getParameter("propertyValueDate"));
+Object expectedValueDateExpr =
condition.getParameter("propertyValueDateExpr");
+
+Collection expectedValues =
ConditionContextHelper.foldToASCII((Collection)
condition.getParameter("propertyValues"));
+Collection expectedValuesInteger = (Collection)
condition.getParameter("propertyValuesInteger");
+Collection expectedValuesDouble = (Collection)
condition.getParameter("propertyValuesDouble");
+Collection expectedValuesDate = convertDatesToISO((Collection)
condition.getParameter("propertyValuesDate"));
+Collection expectedValuesDateExpr = (Collection)
condition.getParameter("propertyValuesDateExpr");
+
+Object value = ObjectUtils.firstNonNull(expectedValue,
expectedValueInteger, expectedValueDouble, expectedValueDate,
expectedValueDateExpr);
+@SuppressWarnings("unchecked")
+Collection values = ObjectUtils.firstNonNull(expectedValues,
expectedValuesInteger, expectedValuesDouble, expectedValuesDate,
expectedValuesDateExpr);
+
+switch (comparisonOperator) {
+case "equals":
+checkRequiredValue(value, name, comparisonOperator, false);
+return
Query.of(q->q.term(t->t.field(name).value(v->getValue(value;
+case "notEquals":
+checkRequiredValue(value, name, comparisonOperator, false);
+return
Query.of(q->q.bool(b->b.mustNot(m->m.term(t->t.field(name).value(v->getValue(value));
+case "greaterThan":
+checkRequiredValue(value, name, comparisonOperator, false);
+return
Query.of(q->q.range(r->r.field(name).gt(JsonData.of(value;
+case
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1940891973
##
persistence-opensearch/conditions/src/main/java/org/apache/unomi/persistence/opensearch/conditions/PropertyConditionOSQueryBuilder.java:
##
@@ -0,0 +1,255 @@
+/*
+ * 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.opensearch.conditions;
+
+import org.apache.commons.lang3.ObjectUtils;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilder;
+import
org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilderDispatcher;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormatter;
+import org.joda.time.format.ISODateTimeFormat;
+import org.opensearch.client.json.JsonData;
+import org.opensearch.client.opensearch._types.FieldValue;
+import org.opensearch.client.opensearch._types.GeoDistanceType;
+import org.opensearch.client.opensearch._types.query_dsl.BoolQuery;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+import org.opensearch.client.util.ObjectBuilder;
+
+import java.time.OffsetDateTime;
+import java.util.*;
+
+import static org.apache.unomi.persistence.spi.conditions.DateUtils.getDate;
+
+public class PropertyConditionOSQueryBuilder implements
ConditionOSQueryBuilder {
+
+DateTimeFormatter dateTimeFormatter;
+
+public PropertyConditionOSQueryBuilder() {
+dateTimeFormatter = ISODateTimeFormat.dateTime();
+}
+
+@Override
+public Query buildQuery(Condition condition, Map context,
ConditionOSQueryBuilderDispatcher dispatcher) {
+String comparisonOperator = (String)
condition.getParameter("comparisonOperator");
+String name = (String) condition.getParameter("propertyName");
+
+if (comparisonOperator == null || name == null) {
+throw new IllegalArgumentException("Impossible to build ES filter,
condition is not valid, comparisonOperator and propertyName properties should
be provided");
Review Comment:
```suggestion
throw new IllegalArgumentException("Impossible to build OS
filter, condition is not valid, comparisonOperator and propertyName properties
should be provided");
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on code in PR #715:
URL: https://github.com/apache/unomi/pull/715#discussion_r1940890837
##
persistence-opensearch/conditions/src/main/java/org/apache/unomi/persistence/opensearch/conditions/PropertyConditionOSQueryBuilder.java:
##
@@ -0,0 +1,255 @@
+/*
+ * 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.opensearch.conditions;
+
+import org.apache.commons.lang3.ObjectUtils;
+import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilder;
+import
org.apache.unomi.persistence.opensearch.ConditionOSQueryBuilderDispatcher;
+import org.apache.unomi.persistence.spi.conditions.ConditionContextHelper;
+import org.joda.time.DateTime;
+import org.joda.time.format.DateTimeFormatter;
+import org.joda.time.format.ISODateTimeFormat;
+import org.opensearch.client.json.JsonData;
+import org.opensearch.client.opensearch._types.FieldValue;
+import org.opensearch.client.opensearch._types.GeoDistanceType;
+import org.opensearch.client.opensearch._types.query_dsl.BoolQuery;
+import org.opensearch.client.opensearch._types.query_dsl.Query;
+import org.opensearch.client.util.ObjectBuilder;
+
+import java.time.OffsetDateTime;
+import java.util.*;
+
+import static org.apache.unomi.persistence.spi.conditions.DateUtils.getDate;
+
+public class PropertyConditionOSQueryBuilder implements
ConditionOSQueryBuilder {
+
+DateTimeFormatter dateTimeFormatter;
+
+public PropertyConditionOSQueryBuilder() {
+dateTimeFormatter = ISODateTimeFormat.dateTime();
+}
+
+@Override
+public Query buildQuery(Condition condition, Map context,
ConditionOSQueryBuilderDispatcher dispatcher) {
+String comparisonOperator = (String)
condition.getParameter("comparisonOperator");
+String name = (String) condition.getParameter("propertyName");
+
+if (comparisonOperator == null || name == null) {
+throw new IllegalArgumentException("Impossible to build ES filter,
condition is not valid, comparisonOperator and propertyName properties should
be provided");
+}
+
+String expectedValue = ConditionContextHelper.foldToASCII((String)
condition.getParameter("propertyValue"));
+Object expectedValueInteger =
condition.getParameter("propertyValueInteger");
+Object expectedValueDouble =
condition.getParameter("propertyValueDouble");
+Object expectedValueDate =
convertDateToISO(condition.getParameter("propertyValueDate"));
+Object expectedValueDateExpr =
condition.getParameter("propertyValueDateExpr");
+
+Collection expectedValues =
ConditionContextHelper.foldToASCII((Collection)
condition.getParameter("propertyValues"));
+Collection expectedValuesInteger = (Collection)
condition.getParameter("propertyValuesInteger");
+Collection expectedValuesDouble = (Collection)
condition.getParameter("propertyValuesDouble");
+Collection expectedValuesDate = convertDatesToISO((Collection)
condition.getParameter("propertyValuesDate"));
+Collection expectedValuesDateExpr = (Collection)
condition.getParameter("propertyValuesDateExpr");
+
+Object value = ObjectUtils.firstNonNull(expectedValue,
expectedValueInteger, expectedValueDouble, expectedValueDate,
expectedValueDateExpr);
+@SuppressWarnings("unchecked")
+Collection values = ObjectUtils.firstNonNull(expectedValues,
expectedValuesInteger, expectedValuesDouble, expectedValuesDate,
expectedValuesDateExpr);
+
+switch (comparisonOperator) {
+case "equals":
+checkRequiredValue(value, name, comparisonOperator, false);
+return
Query.of(q->q.term(t->t.field(name).value(v->getValue(value;
+case "notEquals":
+checkRequiredValue(value, name, comparisonOperator, false);
+return
Query.of(q->q.bool(b->b.mustNot(m->m.term(t->t.field(name).value(v->getValue(value));
+case "greaterThan":
+checkRequiredValue(value, name, comparisonOperator, false);
+return
Query.of(q->q.range(r->r.field(name).gt(JsonData.of(value;
+case
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
jayblanc commented on PR #715: URL: https://github.com/apache/unomi/pull/715#issuecomment-2626645054 I started a review -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
sergehuber commented on PR #715: URL: https://github.com/apache/unomi/pull/715#issuecomment-2606645556 Thank you @Fgerthoffert that is much appreciated. Also, if I can help maybe by directly briefing reviewers on the changes in the code that's something I can do. Best regards, Serge. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [UNOMI-828] Support for OpenSearch persistence [unomi]
Fgerthoffert commented on PR #715: URL: https://github.com/apache/unomi/pull/715#issuecomment-2606498454 Hello, Thanks a lot for the huge work on this PR, as mentioned during the demo, we plan to dedicate some time to review the pull request starting from January 31st (or earlier if we can). We are doing our best to chime asap to avoid the PR dragging for too long. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
