Chore/upgrade spring to 5 3 by cbeach47 · Pull Request #216 · uPortal-Project/WebproxyPortlet

cbeach47 · 2023-09-12T14:49:22Z

Spring upgrade to 5.3.

Brought in Liferay portlet support
Fixed some deprecation warnings due to dependencies
- Removed the json mapper view. The JSON portlet type does not appear to leverage that mapper view.
Deprecated Gateway SSO portlet type
- Compiles, but did not test runtime functionality
Added lombok dependency and replace log instantiation with @Slf4j.
Ensured all log4j dependencies/references were removed
[EDIT]: Hook in uPortal parent pom and clean up deps.
- parent pom PR: Add common deps from Webproxy Spring5 upgrade. Add and adjust plugins uportal-portlet-parent#3

Testing note: Due to available time, not all classes with the logging switch were tested. However - some were tested, and the logging switch was a simple switch across the board without logic adjustments.

…on warning

…recation

ChristianMurphy

Thanks @cbeach47!
From skimming the code (I have not tested this locally) it looks good!

groybal · 2023-09-12T17:52:46Z

I see some issues with dependencies when I deploy the portlet to Tomcat. In WEB-INF/lib, I noticed:

I see both of these jars but I think we only want the slf4j one: commons-logging-1.2.jar and
jcl-over-slf4j-1.7.5.jar
there are two jaxb-core jars (jaxb-core-2.2.7.jar and jaxb-core-2.3.0.1.jar)
I see this jar,which I think is no longer needed: spring-webmvc-portlet-4.3.30.RELEASE.jar
I see mockito-core-4.9.0.jar, which is only needed for testing and therefore should not be here

groybal · 2023-09-12T17:43:32Z

pom.xml

-            <dependency>
-                <groupId>commons-logging</groupId>
-                <artifactId>commons-logging</artifactId>
-                <version>1.2</version>
-                <scope>provided</scope>
-            </dependency>


Why was this removed? When I deploy the portlet to Tomcat, I see both these jars in WEB-INF/lib, but I think we only want the slf4j one:

commons-logging-1.2.jar

jcl-over-slf4j-1.7.5.jar

For logging, the goal is logback (via the slf4j api). All other logging implementations (even transitive ones) should be removed. I'll look into how commons-logging is still getting deployed.

The JCL bridges (-over-) are now included in spring-jcl, so we don't have to explicitly declare them in the pom, however, I'll need to check if jcl-over-slf4j-1.7.5.jar is from spring-jcl, or a transitive dependency that should be excluded.

So, lines 201-206 actually prevent any dependency from pulling in commons-logging, which is why I think it should remain. Otherwise, you have to find which dependencies are pulling in commons-logging and add excludes.

As per our side discussion with @bjagg , I'm going in the direction of using the maven enforcer plugin, and then excluding the commons-logging jars as needed. @groybal - please let me know if you don't feel this is sufficient.

discussed in PR - change the parent pom to have a dependencies section with the excluded deps as 'provided'

pom.xml

…m and moving common deps and plugins to the parent

cbeach47 · 2023-09-25T16:23:50Z

@groybal -

I see both of these jars but I think we only want the slf4j one: commons-logging-1.2.jar and
jcl-over-slf4j-1.7.5.jar

I've now set up the maven enforcer to ban commons-logging, and adjusted the pom to not emit the commons-logging dep.

there are two jaxb-core jars (jaxb-core-2.2.7.jar and jaxb-core-2.3.0.1.jar)

Took a deeper look at the jaxb-core jars - These 'duplicate' jars were present prior to upgrade, and deals with CAS and personDir flows. In order to control scope, I am to delay this. Added #217 for a future effort.

I see this jar,which I think is no longer needed: spring-webmvc-portlet-4.3.30.RELEASE.jar

Good catch - I've removed it now.

I see mockito-core-4.9.0.jar, which is only needed for testing and therefore should not be here

Ah - the scope was misconfigured. This is now removed in the deployment.

cbeach47 · 2023-09-26T22:06:15Z

pom.xml

-            </build>
-        </profile>
-        <profile>
-              <id>doclint-java8-disable</id>


Since we only support Java 8+ , doclint is disabled at the maven-javadoc-plugin level in the parent pom.

cbeach47 · 2023-09-27T17:14:49Z

pom.xml

@@ -51,18 +52,8 @@
    <!-- Dependency Version Properties -->
    <properties>


Collapse dependencyManagement

cbeach47 · 2023-09-27T17:27:58Z

pom.xml

-            </plugin>
-            <plugin>
+                <!-- Overrides the parent pom due to the overlays -->
                <groupId>org.apache.maven.plugins</groupId>


Leave a note similar to SCP's plugin

cbeach47 · 2023-09-27T17:33:08Z

pom.xml

-            </plugin>
        </plugins>
    </build>
-


Good to remove - note, that this locally builds, and pluto-izes the war file. Add this to the release notes.

cbeach47 added 5 commits September 6, 2023 11:21

Chore: Spring 5.3 upgrade - initial effort

3f2c8bd

Chore: Spring 5.3 upgrade - streamline logging

8e76f03

Chore: Spring 5.3 upgrade - deprecate gateway_sso and fix a deprecati…

763db07

…on warning

Chore: Spring 5.3 upgrade - fix deprecation warnings

ef58af8

Remove unused json mapper view and update gateway sso docs due to dep…

5fd2c58

…recation

ChristianMurphy reviewed Sep 12, 2023

View reviewed changes

groybal reviewed Sep 12, 2023

View reviewed changes

cbeach47 added 3 commits September 20, 2023 10:36

Chore: Spring 5.3 upgrade - PR revisions including revising parent po…

3c626d9

…m and moving common deps and plugins to the parent

Chore: Spring 5.3 upgrade - migrate maven-release-plugin to parent pom

5002fc7

Chore: Spring 5.3 upgrade - revise plugins, move to parent or remove

7ed1fc4

Chore: Spring 5.3 upgrade - clean up

29d42d8

cbeach47 commented Sep 26, 2023

View reviewed changes

cbeach47 commented Sep 27, 2023

View reviewed changes

pom.xml

@@ -51,18 +52,8 @@



<properties>

Copy link

Author

cbeach47 Sep 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collapse dependencyManagement

cbeach47 commented Sep 27, 2023

View reviewed changes

		@@ -51,18 +52,8 @@
		<!-- Dependency Version Properties -->
		<properties>

Conversation

cbeach47 commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChristianMurphy left a comment

Choose a reason for hiding this comment

Uh oh!

groybal commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cbeach47 commented Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cbeach47 commented Sep 12, 2023 •

edited

Loading

groybal commented Sep 12, 2023 •

edited

Loading

cbeach47 commented Sep 25, 2023 •

edited

Loading