#1024: Move urls into url-updater module #1025

jan-vcapgemini · 2025-02-07T10:47:38Z

Implements:

adjusted github action workflow
updated wiremock from com.github.tomakehurst 2.35.2 to org.wiremock 3.11.0
added jackson-core dependency to ide-cli
added url to url tool test classes package name
added some tests to please coveralls
introduced maven dependencyManagement
removed unnecessary properties from url-updater module
moved dependency version variables to root pom.xml
moved getBaseUrl method to GithubUrlUpdater
added Override to sub classes using getBaseUrl
used getBasedUrl in each GithubUrlUpdater to construct download urls
changed java version from 17 to 21
changed java version in github action workflows from 17 to 21

adjusted github action workflow updated wiremock from com.github.tomakehurst 2.35.2 to org.wiremock 3.11.0 added jackson-core dependency to ide-cli added url to url tool test classes package name

coveralls · 2025-02-07T10:51:42Z

Pull Request Test Coverage Report for Build 13699101357

Details

0 of 0 changed or added relevant lines in 0 files are covered.
170 unchanged lines in 20 files lost coverage.
Overall coverage decreased (-1.2%) to 67.096%

Files with Coverage Reduction	New Missed Lines	%
com/devonfw/tools/ide/npm/NpmJsonDist.java	1	0.0%
com/devonfw/tools/ide/npm/NpmJsonObject.java	1	0.0%
com/devonfw/tools/ide/npm/NpmJsonVersion.java	1	0.0%
com/devonfw/tools/ide/os/SystemArchitecture.java	1	50.0%
com/devonfw/tools/ide/tool/androidstudio/AndroidJsonContent.java	1	0.0%
com/devonfw/tools/ide/tool/intellij/IntellijJsonDownloads.java	1	0.0%
com/devonfw/tools/ide/url/model/AbstractUrlArtifactWithParent.java	2	66.67%
com/devonfw/tools/ide/url/model/UrlArtifactWithParent.java	2	0.0%
com/devonfw/tools/ide/url/updater/GithubUrlUpdater.java	2	60.0%
com/devonfw/tools/ide/url/model/file/json/UrlStatus.java	4	50.0%

Totals
Change from base Build 13696612753:	-1.2%
Covered Lines:	7732
Relevant Lines:	11101

💛 - Coveralls

added testExtractTgzArchive and testFindExistingFileInFolder

added test and resources added getter of baseUrl to AwsUrlUpdater to be able to mock it

# Conflicts: # cli/src/test/resources/ide-projects/basic/project/home/environment.properties

hohwille

@jan-vcapgemini thanks for your PR. You not only did a refactoring, you also added a lot of new tests and coverage doing homework of other developers that forgot these tests. Really great job 👍
Sorry, but I left quite a lot of review comments for further improvements.
It would be awesome if you could have a look and improve. Thanks in advance!

hohwille · 2025-03-04T16:56:22Z

url-updater/pom.xml

+    <maven.compiler.source>21</maven.compiler.source>
+    <maven.compiler.target>21</maven.compiler.target>


See

IDEasy/pom.xml

Line 22 in ddb6d00

<java.version>17</java.version>

It is fine to upgrade our project from java 17 to java 21 but if we want to do that, we should do that centrally and not have some modules compile for Java17 and others for Java21 what IMHO does not make sense here.

Suggested change

<maven.compiler.source>21</maven.compiler.source>

<maven.compiler.target>21</maven.compiler.target>

Moved to root pom.xml now and adjusted to 21.
Also changed the java version of github actions workflows to java 21.

hohwille · 2025-03-04T16:57:15Z

url-updater/pom.xml

+  <properties>
+    <maven.compiler.source>21</maven.compiler.source>
+    <maven.compiler.target>21</maven.compiler.target>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>


Already defined in our root parent POM:
https://github.com/devonfw/maven-parent/blob/9d31509d5f25c96fa1ec8b4f8cd2c341349b4df2/pom.xml#L17-L18

Suggested change

<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

Removed now.

hohwille · 2025-03-04T17:00:42Z

cli/pom.xml

+      <groupId>org.wiremock</groupId>
+      <artifactId>wiremock</artifactId>
+      <version>3.11.0</version>


very nice that you update here.
With our new module it is more than time that we also go for maven dependencyMamangement.
Could you also do that in this PR so we do not introduce dependency versions redundantly in cli and url-updater?
Simply add a dependencyMamangement to our IDEasy parent POM and add all dependencies there with their versions. Then remove all these versions from other POMs. If we have multiple artifacts from the same groupId that share the same version, use a variable (like jackson.version) in our parent POM.

Suggested change

<groupId>org.wiremock</groupId>

<artifactId>wiremock</artifactId>

<version>3.11.0</version>

<groupId>org.wiremock</groupId>

<artifactId>wiremock</artifactId>

Added dependencyManagement to root pom.xml now.

hohwille · 2025-03-04T17:01:36Z

url-updater/pom.xml

+    <dependency>
+      <groupId>ch.qos.logback</groupId>
+      <artifactId>logback-classic</artifactId>
+      <version>1.5.16</version>


See above

Suggested change

<version>1.5.16</version>

Moved to root pom.xml now.

hohwille · 2025-03-04T17:01:48Z

url-updater/pom.xml

+    <dependency>
+      <groupId>org.wiremock</groupId>
+      <artifactId>wiremock</artifactId>
+      <version>3.11.0</version>


See above

Suggested change

<version>3.11.0</version>

Moved to root pom.xml now.

url-updater/src/test/java/com/devonfw/tools/ide/url/tool/UrlUpdaterTest.java

hohwille · 2025-03-04T17:20:27Z

...ater/src/main/java/com/devonfw/tools/ide/url/tool/androidstudio/AndroidStudioUrlUpdater.java

+import com.devonfw.tools.ide.tool.androidstudio.AndroidJsonDownload;
+import com.devonfw.tools.ide.tool.androidstudio.AndroidJsonItem;
+import com.devonfw.tools.ide.tool.androidstudio.AndroidJsonObject;


Shouldn't we also move these files here?
They are IMHO only used by the url-updaters and not by IDEasy and URL-Metadata.

Moved to url-updater.

hohwille · 2025-03-04T17:24:15Z

cli/pom.xml

+    <dependency>
+      <groupId>com.fasterxml.jackson.core</groupId>
+      <artifactId>jackson-core</artifactId>
+      <version>${jackson.version}</version>
+    </dependency>


why did you have to add this dependency additionally?
It is already a transitive dependency of all other jackson-* dependencies.

Suggested change

<dependency>

<groupId>com.fasterxml.jackson.core</groupId>

<artifactId>jackson-core</artifactId>

<version>${jackson.version}</version>

</dependency>

I've removed it now, did this also for the root pom.xml.

hohwille · 2025-03-04T17:34:46Z

.github/workflows/update-urls.yml

+          cd url-updater
          mvn -B -ntp -Dstyle.color=always install


this is not going to work since url-updater depends on cli so if you only build url-updater without building cli it will fail.
This currently does not fail because the artifacts will be downloaded by maven from nexus.
But when someone (our release process) sets a new version in the POM and then this action is going to be executed, it will fail because cli artifact is missing.

Suggested change

cd url-updater

mvn -B -ntp -Dstyle.color=always install

mvn -B -ntp -Dstyle.color=always -pl url-updater -am install

hohwille · 2025-03-04T17:37:07Z

cli/src/test/java/com/devonfw/tools/ide/context/AbstractIdeTestContext.java

+  public void setUserEnvironmentVariable(String userEnvironmentVariable) {
+
+  }


This does not seem to make sense to me. IMHO this is not used anywhere...

Suggested change

public void setUserEnvironmentVariable(String userEnvironmentVariable) {

}

Removed now.

hohwille · 2025-03-04T17:42:30Z

I am slightly confused. This PR was green. After updating with main it now failed with coverage decreased again.
In general this coveralls is a great tool but here it really is a big pain. You have done a great job and besides the refactoring added a lot of new tests but still coveralls is complaining. IMHO coveralls is buggy in aggregating coverage over modules.

introduced maven dependencyManagement removed unnecessary properties from url-updater module moved dependency version variables to root pom.xml

moved getBaseUrl method to GithubUrlUpdater added Override to sub classes using getBaseUrl used getBasedUrl in each GithubUrlUpdater to construct download urls

moved url specific classes from cli module to url-updater module

replaced import with fully qualified reference

moved url specific classes from cli module to url-updater module

adjusted github workflow

removed unnecessary method

replaced import with fully qualified reference

changed java version from 17 to 21

removed jackson-core dependency from cli

removed jackson-core dependency from root pom

…ithub.com/jan-vcapgemini/IDEasy into enhancement/1024-move-urls-to-url-updater

removed / from aws base url replaced graalvm base url with GITHUB_BASE_URL

devonfw#1024: Move urls into url-updater module

8a12beb

adjusted github action workflow updated wiremock from com.github.tomakehurst 2.35.2 to org.wiremock 3.11.0 added jackson-core dependency to ide-cli added url to url tool test classes package name

jan-vcapgemini self-assigned this Feb 7, 2025

jan-vcapgemini requested a review from hohwille February 7, 2025 10:48

hohwille self-assigned this Feb 10, 2025

hohwille added this to the release:2025.02.001 milestone Feb 10, 2025

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

7f72c6a

jan-vcapgemini added the urls ide-urls repo and related processes and features label Feb 10, 2025

jan-vcapgemini marked this pull request as draft February 11, 2025 08:07

hohwille added 2 commits February 14, 2025 11:25

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

3baaedb

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

ba26bd3

hohwille modified the milestones: release:2025.02.001, release:2025.02.002 Feb 17, 2025

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

075a79c

jan-vcapgemini marked this pull request as ready for review February 17, 2025 10:39

jan-vcapgemini and others added 10 commits February 17, 2025 17:36

devonfw#1024: added tests of WindowsHelperImpl

31d8719

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

1704672

devonfw#1024: added extract test to FileAccess tests

a13df27

devonfw#1022: added more tests

0ace365

added testExtractTgzArchive and testFindExistingFileInFolder

devonfw#1024: added AwsGithubUrlUpdaterTest

fa51e42

added test and resources added getter of baseUrl to AwsUrlUpdater to be able to mock it

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

3741c02

# Conflicts: # cli/src/test/resources/ide-projects/basic/project/home/environment.properties

devonfw#1024: added comments

502ab58

devonfw#1024: fixed remote url

42f8fce

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

38561fe

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

31dff79

hohwille modified the milestones: release:2025.02.002, release:2025.03.001 Feb 26, 2025

jan-vcapgemini and others added 2 commits March 3, 2025 19:13

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

bf7fe2a

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

a4fe444

hohwille approved these changes Mar 4, 2025

View reviewed changes

jan-vcapgemini and others added 19 commits March 5, 2025 12:05

devonfw#1024: implemented requested changes

768f13f

introduced maven dependencyManagement removed unnecessary properties from url-updater module moved dependency version variables to root pom.xml

devonfw#1024: implemented requested changes

f7c08e3

moved getBaseUrl method to GithubUrlUpdater added Override to sub classes using getBaseUrl used getBasedUrl in each GithubUrlUpdater to construct download urls

devonfw#1024: implemented requested changes

c8f81c5

moved url specific classes from cli module to url-updater module

devonfw#1024: implemented requested changes

5ff2362

replaced import with fully qualified reference

devonfw#1024: implemented requested changes

b003ef9

moved url specific classes from cli module to url-updater module

devonfw#1024: implemented requested changes

ba7b08b

adjusted github workflow

devonfw#1024: implemented requested changes

518f7c2

removed unnecessary method

devonfw#1024: implemented requested changes

c43f9c5

replaced import with fully qualified reference

devonfw#1024: fixed pom.xml

b49c1c0

devonfw#1024: adjusted java version

925bcd1

changed java version from 17 to 21

devonfw#1024: fixed pom.xml

8dbd60f

devonfw#1024: fixed pom.xml

702af2d

devonfw#1024: changed java version in github actions from 17 to 21

249dc19

devonfw#1024: fixed gui pom.xml

bc0f747

devonfw#1024: fixed url-updater pom.xml

3e7a1dd

devonfw#1024: implemented requested change

83484bc

removed jackson-core dependency from cli

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

dcf7baa

devonfw#1024: implemented requested change

99c1b9d

removed jackson-core dependency from root pom

Merge branch 'enhancement/1024-move-urls-to-url-updater' of https://g…

46cd6b7

…ithub.com/jan-vcapgemini/IDEasy into enhancement/1024-move-urls-to-url-updater

jan-vcapgemini requested a review from hohwille March 6, 2025 10:20

jan-vcapgemini and others added 2 commits March 6, 2025 11:37

devonfw#1024: adjusted base urls

5dc5414

removed / from aws base url replaced graalvm base url with GITHUB_BASE_URL

Merge branch 'main' into enhancement/1024-move-urls-to-url-updater

f0399ea

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#1024: Move urls into url-updater module #1025

#1024: Move urls into url-updater module #1025

jan-vcapgemini commented Feb 7, 2025 •

edited

Loading

coveralls commented Feb 7, 2025 •

edited

Loading

hohwille left a comment

hohwille Mar 4, 2025

jan-vcapgemini Mar 5, 2025 •

edited

Loading

hohwille Mar 4, 2025

jan-vcapgemini Mar 5, 2025

hohwille Mar 4, 2025

jan-vcapgemini Mar 5, 2025

hohwille Mar 4, 2025

jan-vcapgemini Mar 5, 2025

hohwille Mar 4, 2025

jan-vcapgemini Mar 5, 2025

hohwille Mar 4, 2025

jan-vcapgemini Mar 5, 2025

hohwille Mar 4, 2025

jan-vcapgemini Mar 6, 2025 •

edited

Loading

hohwille Mar 4, 2025

jan-vcapgemini Mar 5, 2025

hohwille Mar 4, 2025

jan-vcapgemini Mar 5, 2025

hohwille commented Mar 4, 2025

		<maven.compiler.source>21</maven.compiler.source>
		<maven.compiler.target>21</maven.compiler.target>

	cd url-updater
	mvn -B -ntp -Dstyle.color=always install
	mvn -B -ntp -Dstyle.color=always -pl url-updater -am install

		public void setUserEnvironmentVariable(String userEnvironmentVariable) {

		}

#1024: Move urls into url-updater module #1025

Are you sure you want to change the base?

#1024: Move urls into url-updater module #1025

Conversation

jan-vcapgemini commented Feb 7, 2025 • edited Loading

Implements:

coveralls commented Feb 7, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13699101357

Details

💛 - Coveralls

hohwille left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jan-vcapgemini Mar 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jan-vcapgemini Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hohwille commented Mar 4, 2025

jan-vcapgemini commented Feb 7, 2025 •

edited

Loading

coveralls commented Feb 7, 2025 •

edited

Loading

jan-vcapgemini Mar 5, 2025 •

edited

Loading

jan-vcapgemini Mar 6, 2025 •

edited

Loading