You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by GitBox <gi...@apache.org> on 2021/04/05 10:59:56 UTC

[GitHub] [tomee] rzo1 commented on a change in pull request #779: Fix some examples using BOM

rzo1 commented on a change in pull request #779:
URL: https://github.com/apache/tomee/pull/779#discussion_r607018168



##########
File path: examples/polling-parent/pom.xml
##########
@@ -79,29 +79,17 @@
     <dependencies>
       <!-- API -->
       <dependency>
-        <groupId>org.apache.tomee</groupId>
-        <artifactId>javaee-api</artifactId>
-        <version>[8.0,)</version>
-        <scope>provided</scope>
-      </dependency>
-      <dependency>
-        <groupId>org.apache.tomee</groupId>
-        <artifactId>openejb-api</artifactId>
-        <version>${tomee.version}</version>
+        <groupId>org.apache.tomee.bom</groupId>
+        <artifactId>tomee-plus-api</artifactId>
+        <version>8.0.7-SNAPSHOT</version>
         <scope>provided</scope>
       </dependency>
 
       <!-- impl - for standard code scope test is fine -->
       <dependency>
-        <groupId>org.apache.tomee</groupId>
-        <artifactId>openejb-core</artifactId>
-        <version>${tomee.version}</version>
-        <scope>provided</scope>
-      </dependency>
-      <dependency>
-        <groupId>org.apache.tomee</groupId>
-        <artifactId>openejb-cxf-rs</artifactId>
-        <version>${tomee.version}</version>
+        <groupId>org.apache.tomee.bom</groupId>
+        <artifactId>tomee-plus-api</artifactId>

Review comment:
       Should probably be `tomee-plus` instead (otherwise duplicate). Would also go for scope `test` here (as `test` would be finde for standard code but we might need to override it in submodules)

##########
File path: examples/polling-parent/polling-core/pom.xml
##########
@@ -29,20 +29,18 @@
 
   <dependencies>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>javaee-api</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>openejb-api</artifactId>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus-api</artifactId>
+      <version>8.0.7-SNAPSHOT</version>
     </dependency>
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
     </dependency>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>openejb-core</artifactId>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus</artifactId>
+      <version>8.0.7-SNAPSHOT</version>

Review comment:
       No version needed. Tree gets it for free from the parent dep mgmt section.
   We have some dependencies towards `openjpa` here, so we would need `provided` here (only we have `test` in the parent pom)

##########
File path: examples/polling-parent/polling-client/pom.xml
##########
@@ -30,27 +30,9 @@
 
   <dependencies>
     <dependency>
-      <groupId>org.apache.cxf</groupId>
-      <artifactId>cxf-core</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.cxf</groupId>
-      <artifactId>cxf-rt-rs-client</artifactId>
-      <version>${cxf.version}</version>
-      <exclusions>
-        <exclusion>
-          <groupId>javax.annotation</groupId>
-          <artifactId>javax.annotation-api</artifactId>
-        </exclusion>
-      </exclusions>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.xbean</groupId>
-      <artifactId>xbean-finder-shaded</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.xbean</groupId>
-      <artifactId>xbean-reflect</artifactId>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus</artifactId>
+      <version>8.0.7-SNAPSHOT</version>

Review comment:
       No version needed. Tree gets it for free from the parent dep mgmt section.
   If you go for `test` in the parent, we need to set it to `provided` here as we have some dependencies.

##########
File path: examples/polling-parent/polling-web/pom.xml
##########
@@ -35,21 +35,14 @@
       <artifactId>junit</artifactId>
     </dependency>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>openejb-core</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>openejb-cxf-rs</artifactId>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus</artifactId>
+      <version>8.0.7-SNAPSHOT</version>

Review comment:
       No version needed. Tree gets it for free from the parent dep mgmt section.

##########
File path: examples/polling-parent/pom.xml
##########
@@ -79,29 +79,17 @@
     <dependencies>
       <!-- API -->
       <dependency>
-        <groupId>org.apache.tomee</groupId>
-        <artifactId>javaee-api</artifactId>
-        <version>[8.0,)</version>
-        <scope>provided</scope>
-      </dependency>
-      <dependency>
-        <groupId>org.apache.tomee</groupId>
-        <artifactId>openejb-api</artifactId>
-        <version>${tomee.version}</version>
+        <groupId>org.apache.tomee.bom</groupId>
+        <artifactId>tomee-plus-api</artifactId>
+        <version>8.0.7-SNAPSHOT</version>

Review comment:
       Maybe use `tomee.version` instead of the hard string as it is already defined in the `properties` section. We also can remove `cxf.version` and `xbean.version` from the `<properties` section of the parent pom. 
   
   In addition, we can remove
   
   ```
    <dependency>
           <groupId>org.apache.cxf</groupId>
           <artifactId>cxf-core</artifactId>
           <version>${cxf.version}</version>
         </dependency>
         <dependency>
           <groupId>org.apache.xbean</groupId>
           <artifactId>xbean-finder-shaded</artifactId>
           <version>${xbean.version}</version>
         </dependency>
         <dependency>
           <groupId>org.apache.xbean</groupId>
           <artifactId>xbean-reflect</artifactId>
           <version>${xbean.version}</version>
         </dependency>
   ```
   
   as it comes with the related BOM.

##########
File path: examples/polling-parent/polling-core/pom.xml
##########
@@ -29,20 +29,18 @@
 
   <dependencies>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>javaee-api</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>openejb-api</artifactId>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus-api</artifactId>
+      <version>8.0.7-SNAPSHOT</version>

Review comment:
       No version needed. Tree gets it for free from the parent dep mgmt section.

##########
File path: examples/applicationcomposer-jaxws-cdi/pom.xml
##########
@@ -64,10 +64,10 @@
   </repositories>
   <dependencies>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>javaee-api</artifactId>
-      <version>[8.0,)</version>
-      <scope>provided</scope>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus-api</artifactId>
+      <version>8.0.7-SNAPSHOT</version>
+      <scope>test</scope>

Review comment:
       Shouldn't it be `provided` ?

##########
File path: examples/polling-parent/polling-client/pom.xml
##########
@@ -62,8 +44,9 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>javaee-api</artifactId>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus-api</artifactId>
+      <version>8.0.7-SNAPSHOT</version>

Review comment:
       No version needed. Tree gets it for free from the parent dep mgmt section.

##########
File path: examples/polling-parent/polling-domain/pom.xml
##########
@@ -29,8 +29,9 @@
 
   <dependencies>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>javaee-api</artifactId>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus-api</artifactId>
+      <version>8.0.7-SNAPSHOT</version>

Review comment:
       No version needed. Tree gets it for free from the parent dep mgmt section.

##########
File path: examples/applicationcomposer-jaxws-cdi/pom.xml
##########
@@ -64,10 +64,10 @@
   </repositories>
   <dependencies>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>javaee-api</artifactId>
-      <version>[8.0,)</version>
-      <scope>provided</scope>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus-api</artifactId>
+      <version>8.0.7-SNAPSHOT</version>
+      <scope>test</scope>

Review comment:
       Otherwise:
   
   It seems, that the tests are failing as the `tomee-plus` boms brings in `tomee-security`, which expects some servlet registrations in `TomEESecurityServletAuthenticationMechanismMapper` and subsequently fails with a `NullPointerException`. 
   
   So either
   
   - (a) The test is flawed and requires some additional parameters / registrations
   - (b) `TomEESecurityServletAuthenticationMechanismMapper` requires a ` != null` check on `servletRegistrations` in `init(...)`
   - (c) Exclude `tomee-security` in the examples dependencies
   - (d) Something else (disable security in the example, other things, ...) ....
   
   Maybe @dblevins has some thoughts here?

##########
File path: examples/polling-parent/polling-web/pom.xml
##########
@@ -35,21 +35,14 @@
       <artifactId>junit</artifactId>
     </dependency>
     <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>openejb-core</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.tomee</groupId>
-      <artifactId>openejb-cxf-rs</artifactId>
+      <groupId>org.apache.tomee.bom</groupId>
+      <artifactId>tomee-plus</artifactId>
+      <version>8.0.7-SNAPSHOT</version>

Review comment:
       In addition: `provided` if `test` in parent pom.




-- 
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:
users@infra.apache.org