#1386: Get latest `eo-runtime` dependency version from Maven #1407

volodya-lombrozo · 2022-11-03T14:20:37Z

Get latest eo-runtime dependency version from Maven.

Closes: #1386

…n from Maven Cenral feat(objectionary#1386): get runtime depenedncy from maven feat(objectionary#1386): cache runtime maven dependency feat(objectionary#1386): fix all linter mistakes

volodya-lombrozo · 2022-11-04T10:12:12Z

@mximp Can you take a look, please?

mximp · 2022-11-07T07:51:00Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsWithRuntime.java

+    /**
+     * Hardcoded dependency.
+     */
+    static final Unchecked<Dependency> HARDCODED = DcsWithRuntime.dependency("0.28.10");


@volodya-lombrozo why do we hardcode the version? Can't we pass it via constructor?

Actually it's not used anywhere within the class - must be moved out.

@mximp done. I've remove the static field at all. I've used it in tests previously.

mximp · 2022-11-07T08:06:15Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsWithRuntime.java

-     * The dependencies source.
+     * Dependency downloaded by HTTP from Maven Central.
+     */
+    static final Unchecked<Dependency> FROM_MAVEN = fromMaven();


@volodya-lombrozo Can we make it private?

@mximp done.

mximp

@volodya-lombrozo please check comments above

mximp · 2022-11-07T08:36:09Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsWithRuntime.java

+     * @param delegate Dependencies delegate.
+     * @param source Runtime dependency source.
+     */
+    DcsWithRuntime(final Dependencies delegate, final Unchecked<Dependency> source) {


@volodya-lombrozo what is the point of supplying runtime dependency to add? I believe the whole idea of this class is to download it from Maven Central in case it's missing in delegate, am I right?
Maybe we can rename the class to DcsRuntimeCentral to make it clear and get rid of two-args constructor?

@mximp You miss one point here. The idea of the class is not to download dependency from Maven Central. The idea the class is to add eo-runtime dependency to the collection of other dependencies. Moreover, we don't have to add eo-runtime dependency if it is already present in the collection. By that reason, we can have different sources where we can get eo-runtime dependency:

hardcoded - we used hardcoded version previously 0.28.10.

maven central - we are using it now
Of course, we can remove the constructor and hardcode only maven central, but it could be useful for the future testing to have additional constructor in order to "mock" source, because getting of "eo-runtime" dependency by http is a quite expensive .

@volodya-lombrozo got it. I believe better name for hardcoded would be fixed or supplied. Please check renaming suggestions for better readability.

mximp · 2022-11-07T09:15:01Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsWithRuntime.java

+     * @param delegate Dependencies delegate.
+     * @param source Runtime dependency source.
+     */
+    DcsWithRuntime(final Dependencies delegate, final Unchecked<Dependency> source) {


@volodya-lombrozo got it. I believe better name for hardcoded would be fixed or supplied. Please check renaming suggestions for better readability.

mximp · 2022-11-07T09:15:23Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsWithRuntime.java

-     * The main constructor.
+     * Runtime dependency source.
+     */
+    private final Unchecked<Dependency> source;


@volodya-lombrozo let's rename to fixed

@mximp renamed to supplied.

mximp · 2022-11-07T09:16:58Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsWithRuntime.java

+     *
+     * @return Runtime dependency from Maven Central.
+     */
+    private static Unchecked<Dependency> fromMaven() {


@volodya-lombrozo let's rename to download

@mximp I don't think it's a good name for that static method according with that post.

@mximp renamed to mavenDependency

mximp · 2022-11-07T09:22:06Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsWithRuntime.java

-     * The dependencies source.
+     * Dependency downloaded by HTTP from Maven Central.
+     */
+    private static final Unchecked<Dependency> FROM_MAVEN = fromMaven();


@volodya-lombrozo let's rename to dynamic. Also static methods should be called via class name.
So this would look like this:
private static final Unchecked<Dependency> DYNAMIC = DcsWithRuntime.download()

@volodya-lombrozo Actually you can use Sticky here from Cactoos lib and get rid of static method.

@mximp I'm already using Sticky class:

private static Unchecked<Dependency> dependency(final String version) { return new Unchecked<>( new Sticky<>( () -> { final Dependency dependency = new Dependency(); dependency.setGroupId("org.eolang"); dependency.setArtifactId("eo-runtime"); dependency.setVersion(version); dependency.setClassifier(""); return dependency; } ) ); }

I can't declare field as Sticky, because in that case I have to deal with Exception handling.

@mximp also I can't get rid of the static method, because I wanna leave private static final Unchecked<Dependency> FROM_MAVEN as a static field in order to increase speed of the tests. (when we have a static field, the method will be called only ~2 times, if you make it as a simple object field - the download will be happened each time when you create a new DcsWithRuntime object > 10 times)

@volodya-lombrozo I just mean why do we need the method dependency. It just creates an object and used only in one place - we can inline it:

private static final Unchecked<Dependency> MAVEN_DEPENDENCY = new Unchecked<>( new Sticky<>(() -> { final Dependency dependency = new Dependency(); dependency.setGroupId("org.eolang"); dependency.setArtifactId("eo-runtime"); dependency.setVersion(version); dependency.setClassifier(""); return dependency; }) );

@volodya-lombrozo corrected inlining:

private static final Unchecked<Dependency> MAVEN_DEPENDENCY = new Unchecked<>( new Sticky<>(() -> { final Dependency dependency = new Dependency(); dependency.setGroupId("org.eolang"); dependency.setArtifactId("eo-runtime"); dependency.setVersion( new XMLDocument( new URL( String.format( "https://repo.maven.apache.org/maven2/%s/maven-metadata.xml", "org/eolang/eo-runtime" ) ) ).xpath("//latest/text()").get(0) ); dependency.setClassifier(""); return dependency; }) );

@mximp I got your idea. It has several advantages:

we don't create extra methods

we reduce total number of lines

On the other hand, that approach:

creates lots of code that is placed above fields declarations and constructors (it's confusing to see a lot of code even before declaring variables).

looks like a solid piece of code that do several things at once: (1) Creates dependency (2) Downloads latest version of library from maven. (3) Creates Sticky object. (some sort of violation of the principle of single responsibility)

I prefer using small methods and classes for achieving granularity and better code readability. But I can be mistaken, of course. Especially in "better code readability".

@volodya-lombrozo I believe the biggest advantage of the inlined version is that I can see the whole logic of defining MAVEN_DEPENDENCY in front of me: that it's sticky (i.e. cached), which URL used and that latest version is extracted. It's only several lines of code and I don't need to jump around the methods.

Regarding SRP, in this case I think we need to look at it a bit broader: we just extract version from Maven Central. I would probably agree that dealing with URL and parsing XML for a version might be a distinguishable bit of functionality but in this case making it to be a separate method doesn't provide any benefits: we just spread logic across multiple places.

@yegor256 what is the better way?

volodya-lombrozo · 2022-11-08T06:44:04Z

@yegor256 take a look, please.

yegor256 · 2022-11-08T11:20:35Z

@rultor merge

rultor · 2022-11-08T11:32:02Z

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2022-11-08T11:39:27Z

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 7min)

feat(objectionary#1386): download latest eo-runtime dependency versio…

e82009d

…n from Maven Cenral feat(objectionary#1386): get runtime depenedncy from maven feat(objectionary#1386): cache runtime maven dependency feat(objectionary#1386): fix all linter mistakes

mximp approved these changes Nov 7, 2022

View reviewed changes

mximp suggested changes Nov 7, 2022

View reviewed changes

volodya-lombrozo added 3 commits November 7, 2022 11:09

chore: Merge branch 'master' into 1386

47e80be

feat(objectionary#1386): make FROM_MAVEN field private

54537f9

feat(objectionary#1386): remove HARCODED dependency from DcsWithRuntime

c56b05f

mximp suggested changes Nov 7, 2022

View reviewed changes

volodya-lombrozo added 3 commits November 7, 2022 12:53

feat(objectionary#1386): source -> supplied

bc7e32f

feat(objectionary#1386): FROM_MAVEN -> MAVEN_DEPENDENCY

4b39035

feat(objectionary#1386): change JavaDoc descriptions

7bd2f4c

volodya-lombrozo requested a review from mximp November 7, 2022 09:59

mximp approved these changes Nov 7, 2022

View reviewed changes

rultor merged commit 2dd0579 into objectionary:master Nov 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#1386: Get latest `eo-runtime` dependency version from Maven #1407

#1386: Get latest `eo-runtime` dependency version from Maven #1407

volodya-lombrozo commented Nov 3, 2022

volodya-lombrozo commented Nov 4, 2022

mximp Nov 7, 2022

mximp Nov 7, 2022

volodya-lombrozo Nov 7, 2022

mximp Nov 7, 2022

volodya-lombrozo Nov 7, 2022

mximp left a comment

mximp Nov 7, 2022

volodya-lombrozo Nov 7, 2022

mximp Nov 7, 2022

mximp Nov 7, 2022

mximp Nov 7, 2022

volodya-lombrozo Nov 7, 2022

mximp Nov 7, 2022

volodya-lombrozo Nov 7, 2022

volodya-lombrozo Nov 7, 2022

mximp Nov 7, 2022

mximp Nov 7, 2022

volodya-lombrozo Nov 7, 2022

volodya-lombrozo Nov 7, 2022

mximp Nov 7, 2022

mximp Nov 7, 2022

volodya-lombrozo Nov 7, 2022 •

edited

Loading

mximp Nov 7, 2022

volodya-lombrozo Nov 7, 2022

volodya-lombrozo commented Nov 8, 2022

yegor256 commented Nov 8, 2022

rultor commented Nov 8, 2022

rultor commented Nov 8, 2022

#1386: Get latest eo-runtime dependency version from Maven #1407

#1386: Get latest eo-runtime dependency version from Maven #1407

Conversation

volodya-lombrozo commented Nov 3, 2022

volodya-lombrozo commented Nov 4, 2022

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

mximp left a 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

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

volodya-lombrozo Nov 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volodya-lombrozo commented Nov 8, 2022

yegor256 commented Nov 8, 2022

rultor commented Nov 8, 2022

rultor commented Nov 8, 2022

#1386: Get latest `eo-runtime` dependency version from Maven #1407

#1386: Get latest `eo-runtime` dependency version from Maven #1407

volodya-lombrozo Nov 7, 2022 •

edited

Loading