OpenLiberty / liberty-tools-intellij

IntelliJ IDEA extension for Liberty
https://plugins.jetbrains.com/plugin/14856-open-liberty-tools
Eclipse Public License 2.0
11 stars 20 forks source link

Fixed using Deprecated Maven Methods which are Marked for Removal #828

Closed vaisakhkannan closed 6 days ago

vaisakhkannan commented 2 weeks ago

Fixes #823 Updated getMavenHome() method to getMavenHomeType() method and Updated resolveMavenHomeDirectory() method to getMavenHomeFile() method

vaisakhkannan commented 1 week ago

@aparnamichael , When I compared the implementation of the old deprecated method (resolveMavenHomeDirectory), I noticed it uses a staticOrBundled method. What this method actually does is..

  1. It attempts to cast the current object (this) to StaticResolvedMavenHomeType.
  2. If the cast is successful, it returns the casted object.
  3. If the cast fails (i.e., this is not an instance of StaticResolvedMavenHomeType), it returns 'BundledMaven3'. Screenshot 2024-06-20 at 1 56 23 PM

Using the same approach, I modified the code to return the default 'BundledMaven3.INSTANCE'

If we use null as the default return value, we will get an IllegalArgumentException if the casting fails in the future, as shown in the image. I encountered this issue during testing because I commented out the relevant code below.

https://github.com/OpenLiberty/liberty-tools-intellij/blob/54d1e896f432156a058642183e48b222a86b44c9/src/main/java/io/openliberty/tools/intellij/util/LibertyMavenUtil.java#L208C9-L211C10

Screenshot 2024-06-20 at 2 24 48 PM

I believe this solution is better now. Please review my changes.

mrglavas commented 1 week ago

I believe this solution is better now. Please review my changes.

Thanks for digging deeper to align the behaviour of the new API usage with the old one.