[GitHub] [camel-quarkus] ppalaga opened a new pull request #618: Fix #Build time FactoryFinders

classic Classic list List threaded Threaded
34 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga opened a new pull request #618: Fix #Build time FactoryFinders

GitBox
ppalaga opened a new pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367547095
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
 
 Review comment:
   we need to fine tune language and data-format as we can store into the registry only singleton services (this requires also some changes to the FastCamelContext as it assumes languages and dataformats are stored in the registry)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367553702
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
 
 Review comment:
   Yes we do have this problem as today but this is a good change to fix 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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367554684
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(entry.getKey(), entry.getValue().getProperty("class")))
+                    .forEach(services::produce);
+
+            /* Configurers need some name transformation */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/configurer")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(
 
 Review comment:
   As future evolution, may be better not to expose such instances to the user through the registry, maybe better to try to bind them to the components at build time

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
ppalaga commented on issue #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#issuecomment-575335515
 
 
   > It would be nice to have some build time properties to control factory files to be included/excluded like:
   >
   > ```
   > quarkus.camel.factories.discovery.include = **/*
   > quarkus.camel.factories.discovery.exclude = org/apache/camel/something/*,org/apache/camel/xyz/**/*
   > ```
   
   I wonder how the set of services defined by the extensions should be composed with these user facing selectors?
   
   A. (extension-includes + user-includes) - user-excludes
   
   or
   
   B. extension-includes + (user-includes - user-excludes)
   
   A. seems to make more sense. WDYT?
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367638712
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
 
 Review comment:
   > we need to fine tune language and data-format
   
   Not sure how should we tune?
   
   > as we can store into the registry only singleton services
   
   You seem to imply language and data-format are not singleton services, but could you plz. define "singleton services"?
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367644357
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(entry.getKey(), entry.getValue().getProperty("class")))
+                    .forEach(services::produce);
+
+            /* Configurers need some name transformation */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/configurer")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(
 
 Review comment:
   I wonder how can we cover the case you mentioned on the chat - manually bind a second instance of a component under a different name?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367686101
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
 
 Review comment:
   The "tuning" here is about to carefully select which language/dataformat can be bound to the registry.
   
   As example camel creates components only once so every time you use a schema, then you'll end up using the same component instance.
   
   For languages and dataformat that's not always true and this is because you can use the same dataformat multiple time in the same route with a different 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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on issue #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
lburgazzoli commented on issue #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#issuecomment-575381644
 
 
   > A. (extension-includes + user-includes) - user-excludes
   
   Yes something like that should work
   
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367689726
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(entry.getKey(), entry.getValue().getProperty("class")))
+                    .forEach(services::produce);
+
+            /* Configurers need some name transformation */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/configurer")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(
 
 Review comment:
   I mean that we should not even bind them to the registry but scan the registry for components and bind related configurers through a recorder during STATIC_INIT

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367831653
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(entry.getKey(), entry.getValue().getProperty("class")))
+                    .forEach(services::produce);
+
+            /* Configurers need some name transformation */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/configurer")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(
 
 Review comment:
   > the registry for components and bind related configurers through a recorder during STATIC_INIT
   
   Yes, that would be easy to do. But when a user adds a second instance of a component under a different name at runtime, our configurer setting code would not catch 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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #618: Fix #Build time FactoryFinders
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r367832781
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
 ##########
 @@ -128,9 +130,40 @@ void coreServiceFilter(BuildProducer<CamelServiceFilterBuildItem> filterBuildIte
         }
 
         @BuildStep
-        void serviceInfoTransformers(BuildProducer<CamelServiceInfoTransformerBuildItem> mapperBuildItems) {
-            mapperBuildItems.produce(
-                    new CamelServiceInfoTransformerBuildItem(CamelServiceInfoTransformers::configurer));
+        void coreServices(
+                ApplicationArchivesBuildItem archives,
+                BuildProducer<CamelServiceBuildItem> services) {
+
+            /* Registered only */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/component",
+                    "META-INF/services/org/apache/camel/language",
+                    "META-INF/services/org/apache/camel/dataformat")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(entry.getKey(), entry.getValue().getProperty("class")))
+                    .forEach(services::produce);
+
+            /* Configurers need some name transformation */
+            CamelSupport.propertyFiles(
+                    archives,
+                    "META-INF/services/org/apache/camel/configurer")
+                    .filter(entry -> entry.getValue().getProperty("class") != null)
+                    .map(entry -> CamelServiceBuildItem.registeredOnly(
 
 Review comment:
   yep but as the second instance is probably configured at runtime, the component will fallback to the "factory finder" way which is reasonable

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #618: Fix #617 Registerable and discoverable Camel services

GitBox
In reply to this post by GitBox
ppalaga commented on issue #618: Fix #617 Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/pull/618#issuecomment-575670595
 
 
   d82e823 implements the requested build time config.
   
   The includes/excludes need to be defined incl. the `META-INF/services/...` prefix. I hope that's not an issue.
   
   I vote for creating a new issue for the fine-tuning of the languages and data-formats.
   
   I'd also create a new issue for replacing the current usage of CamelServiceFilterBuildItem with the new CamelServicePatternBuildItem because those ServiceFilters are just excludes.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on issue #618: Fix #617 Registerable and discoverable Camel services

GitBox
In reply to this post by GitBox
ppalaga commented on issue #618: Fix #617 Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/pull/618#issuecomment-575674729
 
 
   4720897 rebased

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #618: Fix #617 Registerable and discoverable Camel services

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #618: Fix #617 Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r368011742
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/util/PathFilter.java
 ##########
 @@ -0,0 +1,152 @@
+/*
+ * 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.camel.quarkus.core.deployment.util;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Predicate;
+
+import org.apache.camel.util.AntPathMatcher;
+import org.apache.camel.util.ObjectHelper;
+import org.jboss.jandex.DotName;
+
+/**
+ * A utility able to filter resource paths using Ant-like includes and excludes.
+ */
+public class PathFilter {
+    private final AntPathMatcher matcher = new AntPathMatcher();
+    private final List<String> includePatterns;
+    private final List<String> excludePatterns;
+    private final Predicate<String> stringPredicate;
+
+    PathFilter(List<String> includePatterns, List<String> excludePatterns) {
+        this.includePatterns = includePatterns;
+        this.excludePatterns = excludePatterns;
+
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            this.stringPredicate = path -> true;
+        } else {
+            this.stringPredicate = path -> {
+                path = sanitize(path);
+                // same logic as  org.apache.camel.main.DefaultRoutesCollector so exclude
+                // take precedence over include
+                for (String part : excludePatterns) {
+                    if (matcher.match(part, path)) {
+                        return false;
+                    }
+                }
+                for (String part : includePatterns) {
+                    if (matcher.match(part, path)) {
+                        return true;
+                    }
+                }
+                return ObjectHelper.isEmpty(includePatterns);
+            };
+        }
+        ;
+    }
+
+    public Predicate<String> asStringPredicate() {
+        return stringPredicate;
+    }
+
+    public Predicate<DotName> asDotNamePredicate() {
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            return dotName -> true;
+        } else {
+            return dotName -> stringPredicate.test(dotName.toString().replace('.', '/'));
+        }
+    }
+
+    public Predicate<Path> asPathPredicate() {
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            return path -> true;
+        } else {
+            return path -> stringPredicate.test(sanitize(path.toString()));
+        }
+    }
+
+    static String sanitize(String path) {
+        path = path.trim();
+        return (!path.isEmpty() && path.charAt(0) == '/')
+                ? path.substring(1)
+                : path;
+    }
+
+    public static class Builder {
+        private List<String> includePatterns = new ArrayList<String>();
+        private List<String> excludePatterns = new ArrayList<String>();
+
+        public Builder patterns(boolean isInclude, Collection<String> patterns) {
+            if (isInclude) {
+                include(patterns);
+            } else {
+                exclude(patterns);
+            }
+            return this;
+        }
+
+        public Builder include(String pattern) {
+            includePatterns.add(sanitize(pattern));
+            return this;
+        }
+
+        public Builder include(Collection<String> patterns) {
+            patterns.stream().map(PathFilter::sanitize).forEach(includePatterns::add);
+            return this;
+        }
+
+        public Builder include(Optional<? extends Collection<String>> patterns) {
+            patterns.ifPresent(ps -> include(ps));
+            return this;
+        }
+
+        public Builder exclude(String pattern) {
+            excludePatterns.add(sanitize(pattern));
+            return this;
+        }
+
+        public Builder exclude(Collection<String> patterns) {
+            patterns.stream().map(PathFilter::sanitize).forEach(excludePatterns::add);
+            return this;
+        }
+
+        public Builder exclude(Optional<? extends Collection<String>> patterns) {
+            patterns.ifPresent(ps -> exclude(ps));
+            return this;
+        }
+
+        public Builder combine(Builder other) {
+            includePatterns.addAll(other.includePatterns);
+            excludePatterns.addAll(other.excludePatterns);
+            return this;
+        }
+
+        public PathFilter build() {
+            final List<String> incl = includePatterns;
+            includePatterns = null; // avoid leaking the collection trough reuse of the builder
 
 Review comment:
   not a huge issue in our case but my understanding is that invoking `build()` multiple time on the same `builder` should produce the same result

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on issue #618: Fix #617 Registerable and discoverable Camel services

GitBox
In reply to this post by GitBox
lburgazzoli commented on issue #618: Fix #617 Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/pull/618#issuecomment-575686937
 
 
   > I vote for creating a new issue for the fine-tuning of the languages and data-formats.
   
   if you have time, it would be nice to at least remove them from the registry and use the default camel discovery for languages and dataformats

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli edited a comment on issue #618: Fix #617 Registerable and discoverable Camel services

GitBox
In reply to this post by GitBox
lburgazzoli edited a comment on issue #618: Fix #617 Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/pull/618#issuecomment-575686937
 
 
   > I vote for creating a new issue for the fine-tuning of the languages and data-formats.
   
   if you have time, it would be nice to at least remove them from the registry and use the default camel discovery for languages and dataformats, we can fine tune them later one but at least we are on the safe side

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #618: Fix #617 Registerable and discoverable Camel services

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #618: Fix #617 Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r368031229
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/util/PathFilter.java
 ##########
 @@ -0,0 +1,152 @@
+/*
+ * 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.camel.quarkus.core.deployment.util;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Predicate;
+
+import org.apache.camel.util.AntPathMatcher;
+import org.apache.camel.util.ObjectHelper;
+import org.jboss.jandex.DotName;
+
+/**
+ * A utility able to filter resource paths using Ant-like includes and excludes.
+ */
+public class PathFilter {
+    private final AntPathMatcher matcher = new AntPathMatcher();
+    private final List<String> includePatterns;
+    private final List<String> excludePatterns;
+    private final Predicate<String> stringPredicate;
+
+    PathFilter(List<String> includePatterns, List<String> excludePatterns) {
+        this.includePatterns = includePatterns;
+        this.excludePatterns = excludePatterns;
+
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            this.stringPredicate = path -> true;
+        } else {
+            this.stringPredicate = path -> {
+                path = sanitize(path);
+                // same logic as  org.apache.camel.main.DefaultRoutesCollector so exclude
+                // take precedence over include
+                for (String part : excludePatterns) {
+                    if (matcher.match(part, path)) {
+                        return false;
+                    }
+                }
+                for (String part : includePatterns) {
+                    if (matcher.match(part, path)) {
+                        return true;
+                    }
+                }
+                return ObjectHelper.isEmpty(includePatterns);
+            };
+        }
+        ;
+    }
+
+    public Predicate<String> asStringPredicate() {
+        return stringPredicate;
+    }
+
+    public Predicate<DotName> asDotNamePredicate() {
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            return dotName -> true;
+        } else {
+            return dotName -> stringPredicate.test(dotName.toString().replace('.', '/'));
+        }
+    }
+
+    public Predicate<Path> asPathPredicate() {
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            return path -> true;
+        } else {
+            return path -> stringPredicate.test(sanitize(path.toString()));
+        }
+    }
+
+    static String sanitize(String path) {
+        path = path.trim();
+        return (!path.isEmpty() && path.charAt(0) == '/')
+                ? path.substring(1)
+                : path;
+    }
+
+    public static class Builder {
+        private List<String> includePatterns = new ArrayList<String>();
+        private List<String> excludePatterns = new ArrayList<String>();
+
+        public Builder patterns(boolean isInclude, Collection<String> patterns) {
+            if (isInclude) {
+                include(patterns);
+            } else {
+                exclude(patterns);
+            }
+            return this;
+        }
+
+        public Builder include(String pattern) {
+            includePatterns.add(sanitize(pattern));
+            return this;
+        }
+
+        public Builder include(Collection<String> patterns) {
+            patterns.stream().map(PathFilter::sanitize).forEach(includePatterns::add);
+            return this;
+        }
+
+        public Builder include(Optional<? extends Collection<String>> patterns) {
+            patterns.ifPresent(ps -> include(ps));
+            return this;
+        }
+
+        public Builder exclude(String pattern) {
+            excludePatterns.add(sanitize(pattern));
+            return this;
+        }
+
+        public Builder exclude(Collection<String> patterns) {
+            patterns.stream().map(PathFilter::sanitize).forEach(excludePatterns::add);
+            return this;
+        }
+
+        public Builder exclude(Optional<? extends Collection<String>> patterns) {
+            patterns.ifPresent(ps -> exclude(ps));
+            return this;
+        }
+
+        public Builder combine(Builder other) {
+            includePatterns.addAll(other.includePatterns);
+            excludePatterns.addAll(other.excludePatterns);
+            return this;
+        }
+
+        public PathFilter build() {
+            final List<String> incl = includePatterns;
+            includePatterns = null; // avoid leaking the collection trough reuse of the builder
 
 Review comment:
   > invoking `build()` multiple time on the same `builder` should produce the same result
   
   If the collection is not set to null here the following sequence would be possible:
   
   ```
   PathFilter.Builder b = new PathFilter.Builder();
   b.include("foo/bar");
   PathFilter pf1 = b.build();
   b.include("bar/baz");
   PathFilter pf2 = b.build();
   ```
   
   where pf1 and pf2 would have set the same list instance in their includePatterns fields. It find that really unwanted. That's a true reference leak that harms the immutability of PathFilter.
   
   I hold re-using builders for rather unusual and so I find breaking the re-use better than copying the list in the PathFilter constructor as a way to ensure immutability.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #618: Fix #617 Registerable and discoverable Camel services

GitBox
In reply to this post by GitBox
ppalaga commented on a change in pull request #618: Fix #617 Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r368031229
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/util/PathFilter.java
 ##########
 @@ -0,0 +1,152 @@
+/*
+ * 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.camel.quarkus.core.deployment.util;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Predicate;
+
+import org.apache.camel.util.AntPathMatcher;
+import org.apache.camel.util.ObjectHelper;
+import org.jboss.jandex.DotName;
+
+/**
+ * A utility able to filter resource paths using Ant-like includes and excludes.
+ */
+public class PathFilter {
+    private final AntPathMatcher matcher = new AntPathMatcher();
+    private final List<String> includePatterns;
+    private final List<String> excludePatterns;
+    private final Predicate<String> stringPredicate;
+
+    PathFilter(List<String> includePatterns, List<String> excludePatterns) {
+        this.includePatterns = includePatterns;
+        this.excludePatterns = excludePatterns;
+
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            this.stringPredicate = path -> true;
+        } else {
+            this.stringPredicate = path -> {
+                path = sanitize(path);
+                // same logic as  org.apache.camel.main.DefaultRoutesCollector so exclude
+                // take precedence over include
+                for (String part : excludePatterns) {
+                    if (matcher.match(part, path)) {
+                        return false;
+                    }
+                }
+                for (String part : includePatterns) {
+                    if (matcher.match(part, path)) {
+                        return true;
+                    }
+                }
+                return ObjectHelper.isEmpty(includePatterns);
+            };
+        }
+        ;
+    }
+
+    public Predicate<String> asStringPredicate() {
+        return stringPredicate;
+    }
+
+    public Predicate<DotName> asDotNamePredicate() {
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            return dotName -> true;
+        } else {
+            return dotName -> stringPredicate.test(dotName.toString().replace('.', '/'));
+        }
+    }
+
+    public Predicate<Path> asPathPredicate() {
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            return path -> true;
+        } else {
+            return path -> stringPredicate.test(sanitize(path.toString()));
+        }
+    }
+
+    static String sanitize(String path) {
+        path = path.trim();
+        return (!path.isEmpty() && path.charAt(0) == '/')
+                ? path.substring(1)
+                : path;
+    }
+
+    public static class Builder {
+        private List<String> includePatterns = new ArrayList<String>();
+        private List<String> excludePatterns = new ArrayList<String>();
+
+        public Builder patterns(boolean isInclude, Collection<String> patterns) {
+            if (isInclude) {
+                include(patterns);
+            } else {
+                exclude(patterns);
+            }
+            return this;
+        }
+
+        public Builder include(String pattern) {
+            includePatterns.add(sanitize(pattern));
+            return this;
+        }
+
+        public Builder include(Collection<String> patterns) {
+            patterns.stream().map(PathFilter::sanitize).forEach(includePatterns::add);
+            return this;
+        }
+
+        public Builder include(Optional<? extends Collection<String>> patterns) {
+            patterns.ifPresent(ps -> include(ps));
+            return this;
+        }
+
+        public Builder exclude(String pattern) {
+            excludePatterns.add(sanitize(pattern));
+            return this;
+        }
+
+        public Builder exclude(Collection<String> patterns) {
+            patterns.stream().map(PathFilter::sanitize).forEach(excludePatterns::add);
+            return this;
+        }
+
+        public Builder exclude(Optional<? extends Collection<String>> patterns) {
+            patterns.ifPresent(ps -> exclude(ps));
+            return this;
+        }
+
+        public Builder combine(Builder other) {
+            includePatterns.addAll(other.includePatterns);
+            excludePatterns.addAll(other.excludePatterns);
+            return this;
+        }
+
+        public PathFilter build() {
+            final List<String> incl = includePatterns;
+            includePatterns = null; // avoid leaking the collection trough reuse of the builder
 
 Review comment:
   > invoking `build()` multiple time on the same `builder` should produce the same result
   
   If the collection is not set to null here the following sequence would be possible:
   
   ```
   PathFilter.Builder b = new PathFilter.Builder();
   b.include("foo/bar");
   PathFilter pf1 = b.build();
   b.include("bar/baz");
   PathFilter pf2 = b.build();
   ```
   
   where pf1 and pf2 would have set the same list instance in their includePatterns fields. That would be really unwanted. That's a true reference leak that harms the immutability of PathFilter.
   
   I hold re-using builders for rather unusual and so I find breaking the re-use better than copying the list in the PathFilter constructor as a way to ensure immutability.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-quarkus] lburgazzoli commented on a change in pull request #618: Fix #617 Registerable and discoverable Camel services

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #618: Fix #617 Registerable and discoverable Camel services
URL: https://github.com/apache/camel-quarkus/pull/618#discussion_r368036556
 
 

 ##########
 File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/util/PathFilter.java
 ##########
 @@ -0,0 +1,152 @@
+/*
+ * 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.camel.quarkus.core.deployment.util;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Predicate;
+
+import org.apache.camel.util.AntPathMatcher;
+import org.apache.camel.util.ObjectHelper;
+import org.jboss.jandex.DotName;
+
+/**
+ * A utility able to filter resource paths using Ant-like includes and excludes.
+ */
+public class PathFilter {
+    private final AntPathMatcher matcher = new AntPathMatcher();
+    private final List<String> includePatterns;
+    private final List<String> excludePatterns;
+    private final Predicate<String> stringPredicate;
+
+    PathFilter(List<String> includePatterns, List<String> excludePatterns) {
+        this.includePatterns = includePatterns;
+        this.excludePatterns = excludePatterns;
+
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            this.stringPredicate = path -> true;
+        } else {
+            this.stringPredicate = path -> {
+                path = sanitize(path);
+                // same logic as  org.apache.camel.main.DefaultRoutesCollector so exclude
+                // take precedence over include
+                for (String part : excludePatterns) {
+                    if (matcher.match(part, path)) {
+                        return false;
+                    }
+                }
+                for (String part : includePatterns) {
+                    if (matcher.match(part, path)) {
+                        return true;
+                    }
+                }
+                return ObjectHelper.isEmpty(includePatterns);
+            };
+        }
+        ;
+    }
+
+    public Predicate<String> asStringPredicate() {
+        return stringPredicate;
+    }
+
+    public Predicate<DotName> asDotNamePredicate() {
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            return dotName -> true;
+        } else {
+            return dotName -> stringPredicate.test(dotName.toString().replace('.', '/'));
+        }
+    }
+
+    public Predicate<Path> asPathPredicate() {
+        if (ObjectHelper.isEmpty(excludePatterns) && ObjectHelper.isEmpty(includePatterns)) {
+            return path -> true;
+        } else {
+            return path -> stringPredicate.test(sanitize(path.toString()));
+        }
+    }
+
+    static String sanitize(String path) {
+        path = path.trim();
+        return (!path.isEmpty() && path.charAt(0) == '/')
+                ? path.substring(1)
+                : path;
+    }
+
+    public static class Builder {
+        private List<String> includePatterns = new ArrayList<String>();
+        private List<String> excludePatterns = new ArrayList<String>();
+
+        public Builder patterns(boolean isInclude, Collection<String> patterns) {
+            if (isInclude) {
+                include(patterns);
+            } else {
+                exclude(patterns);
+            }
+            return this;
+        }
+
+        public Builder include(String pattern) {
+            includePatterns.add(sanitize(pattern));
+            return this;
+        }
+
+        public Builder include(Collection<String> patterns) {
+            patterns.stream().map(PathFilter::sanitize).forEach(includePatterns::add);
+            return this;
+        }
+
+        public Builder include(Optional<? extends Collection<String>> patterns) {
+            patterns.ifPresent(ps -> include(ps));
+            return this;
+        }
+
+        public Builder exclude(String pattern) {
+            excludePatterns.add(sanitize(pattern));
+            return this;
+        }
+
+        public Builder exclude(Collection<String> patterns) {
+            patterns.stream().map(PathFilter::sanitize).forEach(excludePatterns::add);
+            return this;
+        }
+
+        public Builder exclude(Optional<? extends Collection<String>> patterns) {
+            patterns.ifPresent(ps -> exclude(ps));
+            return this;
+        }
+
+        public Builder combine(Builder other) {
+            includePatterns.addAll(other.includePatterns);
+            excludePatterns.addAll(other.excludePatterns);
+            return this;
+        }
+
+        public PathFilter build() {
+            final List<String> incl = includePatterns;
+            includePatterns = null; // avoid leaking the collection trough reuse of the builder
 
 Review comment:
   At least document it as I think no-one expect the builder to be re-set once you invoke `build()`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
12