941 ide urls with java #1000

cinnamon-coder-hub · 2022-12-07T20:58:08Z

First draft with most of the main functionalities necessary regarding issue #941. This is no final version and further commits will be added soon to tackle the following remarks:

Will be done in the comming days: Adding the 'missing'-file functionality, as well as the 'manual'-file for blocking versions regarding automated updating of download-urls.
If wanted: I'll add a more detailed documentation to the methods and classes.
Exception handling will be changed, e.g. throws IOException instead of try-catch block, if no further feedback can be given to the user.
Some parts of the test are a bit unpleasant, as they print out debug-infos into the terminal. This will be changed, so that assume*-lines are used instead, if needed. Then the tests can be run in an automated way via github-actions.

hohwille

@cinnamon-coder-hub thanks. 👍
I left a little review comment for a small rework. Can you address this so I can merge and we can collaborate on this. If you have solved the IDE settings meanwhile you can also perform a CleanUp for the entire project in Eclipse to apply the code format. Thanks in advance.

urlUpdater/pom.xml

...pdater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlHasChildParentArtifact.java

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlHasChildArtifact.java

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlReader.java

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlFile.java

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlRepository.java

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlReader.java

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlEditor.java

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlArtifact.java

hohwille · 2023-01-19T11:10:56Z

It seems as if the unresolved conversation (regarding the pom file) can be resolved now, ...

Done

Still the "CI Build PR" test fails ...

You or someone accidentally deleted the submodule settings in this feature-branch / PR.
Go to files tab and you will see this:

Submodule [settings](https://github.com/devonfw/ide-settings) deleted from a9f6a7

Simply revert this change to fix the build.

It would be good to hear your ideas regarding the epic 941 ...

It is no problem if the epic is not entirely resolved with this PR. That is the nature of Epics as they are too big.
All I care for is that we always have consistent code on master branch with high quality. So if the work from @Amueller36 is integrated, warnings resolved and JavaDoc is present, we can merge.
Please expect further finding when I will do the final reviews.
I wont merge hacks like this:

String pathToRepo ="I:\\UrlRepoTest";

Main class should have a resonable name, Java-conventions have to be met (package names have to be all lower-case - see Updater with captial U), etc. So we still have a road to go before we can merge this.
Thanks already for the great progress and state @cinnamon-coder-hub and @Amueller36.

Implemented Missing Crawlers. Renamed package Updater to updater.

…-ide-urls-with-java

Updated Settings folder

sonatype-lift · 2023-01-19T11:42:56Z

url-updater/src/main/java/com/devonfw/tools/ide/url/updater/AbstractCrawler.java

+            HttpRequest request = HttpRequest.newBuilder()
+                    .uri(URI.create(downloadUrl))
+                    .method("HEAD", HttpRequest.BodyPublishers.noBody())
+                    .timeout(java.time.Duration.ofSeconds(5))


UnnecessarilyFullyQualified: This fully qualified name is unambiguous to the compiler if imported.

Suggested change

.timeout(java.time.Duration.ofSeconds(5))

.timeout(Duration.ofSeconds(5))

ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift · 2023-01-19T11:42:58Z

url-updater/src/main/java/com/devonfw/tools/ide/url/updater/JsonCrawler.java

+        Set<String> versions = new HashSet<>();
+        try {
+            String response = doGetResponseBody(url);
+            ObjectMapper mapper = new ObjectMapper().configure(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);


UnnecessarilyFullyQualified: This fully qualified name is unambiguous to the compiler if imported.

Suggested change

ObjectMapper mapper = new ObjectMapper().configure(com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);

ObjectMapper mapper = new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);

ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlVersion.java

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlArtifact.java

url-updater/src/main/java/com/devonfw/tools/ide/url/updater/java/JavaCrawler.java

sonatype-lift · 2023-01-19T11:43:11Z

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlVersion.java

+
+  public void createFile(String os) {
+
+    File f = new File(this.getPath() + File.separator + os + ".urls");


💬 3 similar findings have been found in this PR

PATH_TRAVERSAL_IN: This API (java/io/File.(Ljava/lang/String;)V) reads a file whose location might be specified by user input

🔎 Expand here to view all instances of this finding

File Path Line Number

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlVersion.java 132

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlVersion.java 154

url-updater/src/main/java/com/devonfw/tools/ide/url/folderhandling/UrlVersion.java 166

Visit the Lift Web Console to find more details in your report.

ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift · 2023-01-19T11:43:14Z

🛠 Lift Auto-fix

Some of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.¹

# Download the patch
curl https://lift.sonatype.com/api/patch/github.com/devonfw/ide/1000.diff -o lift-autofixes.diff

# Apply the patch with git
git apply lift-autofixes.diff

# Review the changes
git diff

Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command:

curl https://lift.sonatype.com/api/patch/github.com/devonfw/ide/1000.diff | git apply

Once you're satisfied commit and push your changes in your project.

You can preview the patch by opening the patch URL in the browser. ↩

Records caused the build pipeline to fail because needed target version is 16! So I refactored it to use POJO's

url-updater/src/main/java/com/devonfw/tools/ide/url/updater/java/JavaCrawler.java

url-updater/src/main/java/com/devonfw/tools/ide/url/UpdateInitiator.java

Amueller36 · 2023-01-19T13:46:16Z

It seems as if the unresolved conversation (regarding the pom file) can be resolved now, ...

Done

Still the "CI Build PR" test fails ...

You or someone accidentally deleted the submodule settings in this feature-branch / PR. Go to files tab and you will see this:
Submodule [settings](https://github.com/devonfw/ide-settings) deleted from a9f6a7
Simply revert this change to fix the build.

It would be good to hear your ideas regarding the epic 941 ...

It is no problem if the epic is not entirely resolved with this PR. That is the nature of Epics as they are too big. All I care for is that we always have consistent code on master branch with high quality. So if the work from @Amueller36 is integrated, warnings resolved and JavaDoc is present, we can merge. Please expect further finding when I will do the final reviews. I wont merge hacks like this:
String pathToRepo ="I:\\UrlRepoTest";
Main class should have a resonable name, Java-conventions have to be met (package names have to be all lower-case - see Updater with captial U), etc. So we still have a road to go before we can merge this. Thanks already for the great progress and state @cinnamon-coder-hub and @Amueller36.

The build went through now. I had to rewrite the records to pojos because the tests run on java 11. Records require java 16.
I am not sure yet what path I should insert in the Current main function then, should it be the relative path ("../../ide-urls")?
I cp the settings folder back in and commited it but I am not sure if that was correct because I see in other forks in github it is a link to the template and not a folder itself.

Changed to relative path. Removed Double Slf4J dependency

Added method for updating only existing versions.

url-updater/src/main/java/com/devonfw/tools/ide/url/UpdateInitiator.java

hohwille · 2023-02-13T15:44:31Z

This PR was IMHO replaced by PR #1040 that has already been merged. Hence, I am closing it.

cinnamon-coder-hub and others added 7 commits November 27, 2022 13:24

devonfw#941:Added (folders + code) frame for discussion

754b739

Merge branch 'devonfw:master' into 941-New-ide-urls-approach

07e4095

941: Removing eclipse generated pom file

a09b916

devonfw#941:Changed folder name + added UrlFile

44724b0

devonfw#941:Changed mainClass in pom file

e0cb148

devonfw#941: Another method added + test scenario

9102481

devonfw#941: First serious draft for discussion if needed

3b429c9

cinnamon-coder-hub added the draft needs clarification or details, not ready for implementation label Dec 7, 2022

cinnamon-coder-hub requested a review from hohwille December 7, 2022 20:58

github-actions bot added the bash related to bash shell or scripts label Dec 7, 2022

devonfw#941:+assertThat tests -debug code +rename folder

fe66928

hohwille requested changes Dec 12, 2022

View reviewed changes

urlUpdater/pom.xml Outdated Show resolved Hide resolved

hohwille mentioned this pull request Dec 12, 2022

#941:Added (folders + code) frame for discussion #994

Closed

This was linked to issues Dec 12, 2022

New approach for ide-mirrors #941

Closed

create domain model for ide-urls #1007

Closed

Merge branch 'master' into 941-ide-urls-with-java

901f514

hohwille added this to the release:2022.12.001 milestone Dec 12, 2022

devonfw#941: mv remaining files, add module url-updater

b4503e9