You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/03/31 03:30:28 UTC

[GitHub] [james-project] Arsnael commented on a change in pull request #946: JAMES-3739 Allow IndexCreation factory to use custom analyzers and tokenizers

Arsnael commented on a change in pull request #946:
URL: https://github.com/apache/james-project/pull/946#discussion_r839136083



##########
File path: backends-common/elasticsearch-v7/src/main/java/org/apache/james/backends/es/v7/IndexCreationFactory.java
##########
@@ -33,65 +36,121 @@
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.indices.CreateIndexRequest;
 import org.elasticsearch.client.indices.GetIndexRequest;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.github.fge.lambdas.Throwing;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 public class IndexCreationFactory {
 
-    public static class AliasSpecificationStep {
-        private final int nbShards;
-        private final int nbReplica;
-        private final int waitForActiveShards;
-        private final IndexName indexName;
-        private final ImmutableList.Builder<AliasName> aliases;
+    public static class IndexCreationCustomElement {
+        public static IndexCreationCustomElement EMPTY = from("{}");
 
-        AliasSpecificationStep(int nbShards, int nbReplica, int waitForActiveShards, IndexName indexName) {
-            this.nbShards = nbShards;
-            this.nbReplica = nbReplica;
-            this.waitForActiveShards = waitForActiveShards;
-            this.indexName = indexName;
-            this.aliases = ImmutableList.builder();
+        public static IndexCreationCustomElement from(String value) {
+            try {
+                new ObjectMapper().readTree(value);
+            } catch (JsonProcessingException e) {
+                throw new IllegalArgumentException("value must be a valid json");
+            }
+            return new IndexCreationCustomElement(value);
         }
 
-        public AliasSpecificationStep addAlias(AliasName aliasName) {
-            Preconditions.checkNotNull(aliasName);
-            this.aliases.add(aliasName);
-            return this;
-        }
+        private final String payload;
 
-        public ReactorElasticSearchClient createIndexAndAliases(ReactorElasticSearchClient client) {
-            return new IndexCreationPerformer(nbShards, nbReplica, waitForActiveShards, indexName, aliases.build()).createIndexAndAliases(client, Optional.empty());
+        IndexCreationCustomElement(String payload) {
+            this.payload = payload;
         }
 
-        public ReactorElasticSearchClient createIndexAndAliases(ReactorElasticSearchClient client, XContentBuilder mappingContent) {
-            return new IndexCreationPerformer(nbShards, nbReplica, waitForActiveShards, indexName, aliases.build()).createIndexAndAliases(client, Optional.of(mappingContent));
+        public String getPayload() {
+            return payload;
         }
     }
 
     static class IndexCreationPerformer {
+        public static class Builder {
+            private final int nbShards;
+            private final int nbReplica;
+            private final int waitForActiveShards;
+            private final IndexName indexName;
+            private final ImmutableList.Builder<AliasName> aliases;
+            private Optional<IndexCreationCustomElement> customAnalyzers;
+            private Optional<IndexCreationCustomElement> customTokenizers;
+
+            public Builder(int nbShards, int nbReplica, int waitForActiveShards, IndexName indexName) {

Review comment:
       I find your builder a bit strange TBH. It does not follow the usual syntax of other builders in the code base

##########
File path: backends-common/elasticsearch-v7/src/main/java/org/apache/james/backends/es/v7/IndexCreationFactory.java
##########
@@ -33,65 +36,121 @@
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.indices.CreateIndexRequest;
 import org.elasticsearch.client.indices.GetIndexRequest;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.github.fge.lambdas.Throwing;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 
 public class IndexCreationFactory {
 
-    public static class AliasSpecificationStep {
-        private final int nbShards;
-        private final int nbReplica;
-        private final int waitForActiveShards;
-        private final IndexName indexName;
-        private final ImmutableList.Builder<AliasName> aliases;
+    public static class IndexCreationCustomElement {
+        public static IndexCreationCustomElement EMPTY = from("{}");
 
-        AliasSpecificationStep(int nbShards, int nbReplica, int waitForActiveShards, IndexName indexName) {
-            this.nbShards = nbShards;
-            this.nbReplica = nbReplica;
-            this.waitForActiveShards = waitForActiveShards;
-            this.indexName = indexName;
-            this.aliases = ImmutableList.builder();
+        public static IndexCreationCustomElement from(String value) {
+            try {
+                new ObjectMapper().readTree(value);
+            } catch (JsonProcessingException e) {
+                throw new IllegalArgumentException("value must be a valid json");
+            }
+            return new IndexCreationCustomElement(value);
         }
 
-        public AliasSpecificationStep addAlias(AliasName aliasName) {
-            Preconditions.checkNotNull(aliasName);
-            this.aliases.add(aliasName);
-            return this;
-        }
+        private final String payload;
 
-        public ReactorElasticSearchClient createIndexAndAliases(ReactorElasticSearchClient client) {
-            return new IndexCreationPerformer(nbShards, nbReplica, waitForActiveShards, indexName, aliases.build()).createIndexAndAliases(client, Optional.empty());
+        IndexCreationCustomElement(String payload) {
+            this.payload = payload;
         }
 
-        public ReactorElasticSearchClient createIndexAndAliases(ReactorElasticSearchClient client, XContentBuilder mappingContent) {
-            return new IndexCreationPerformer(nbShards, nbReplica, waitForActiveShards, indexName, aliases.build()).createIndexAndAliases(client, Optional.of(mappingContent));
+        public String getPayload() {
+            return payload;
         }
     }
 
     static class IndexCreationPerformer {
+        public static class Builder {
+            private final int nbShards;
+            private final int nbReplica;
+            private final int waitForActiveShards;
+            private final IndexName indexName;
+            private final ImmutableList.Builder<AliasName> aliases;
+            private Optional<IndexCreationCustomElement> customAnalyzers;
+            private Optional<IndexCreationCustomElement> customTokenizers;
+
+            public Builder(int nbShards, int nbReplica, int waitForActiveShards, IndexName indexName) {
+                this.nbShards = nbShards;
+                this.nbReplica = nbReplica;
+                this.waitForActiveShards = waitForActiveShards;
+                this.indexName = indexName;
+                this.aliases = ImmutableList.builder();
+                this.customAnalyzers = Optional.empty();
+                this.customTokenizers = Optional.empty();
+            }
+
+            public Builder addAlias(AliasName aliasName) {
+                Preconditions.checkNotNull(aliasName);
+                this.aliases.add(aliasName);
+                return this;
+            }
+
+            public Builder customAnalyzers(IndexCreationCustomElement customAnalyzers) {
+                Preconditions.checkNotNull(customAnalyzers);
+                this.customAnalyzers = Optional.of(customAnalyzers);
+                return this;
+            }
+
+            public Builder customTokenizers(IndexCreationCustomElement customTokenizers) {
+                Preconditions.checkNotNull(customTokenizers);
+                this.customTokenizers = Optional.of(customTokenizers);
+                return this;
+            }
+
+            public IndexCreationPerformer build() {
+                return new IndexCreationPerformer(nbShards, nbReplica, waitForActiveShards, indexName, aliases.build(), customAnalyzers, customTokenizers);
+            }
+
+            public ReactorElasticSearchClient createIndexAndAliases(ReactorElasticSearchClient client) {
+                return build().createIndexAndAliases(client, Optional.empty());
+            }
+
+            public ReactorElasticSearchClient createIndexAndAliases(ReactorElasticSearchClient client, XContentBuilder mappingContent) {
+                return build().createIndexAndAliases(client, Optional.of(mappingContent));
+            }
+        }
+
         private final int nbShards;
         private final int nbReplica;
         private final int waitForActiveShards;
         private final IndexName indexName;
         private final ImmutableList<AliasName> aliases;
+        private final Optional<IndexCreationCustomElement> customAnalyzers;
+        private final Optional<IndexCreationCustomElement> customTokenizers;
 
         public IndexCreationPerformer(int nbShards, int nbReplica, int waitForActiveShards, IndexName indexName, ImmutableList<AliasName> aliases) {
+            this(nbShards, nbReplica, waitForActiveShards, indexName, aliases, Optional.empty(), Optional.empty());
+        }
+
+        public IndexCreationPerformer(int nbShards, int nbReplica, int waitForActiveShards, IndexName indexName, ImmutableList<AliasName> aliases,

Review comment:
       Shouldn't the constructors be private and we should only expose a static method returning a new Builder instance, forcing its use? 




-- 
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: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org