Skip to content

Align configuration properties and more#1785

Merged
cstamas merged 7 commits intoapache:masterfrom
cstamas:issue-1783
Feb 5, 2026
Merged

Align configuration properties and more#1785
cstamas merged 7 commits intoapache:masterfrom
cstamas:issue-1783

Conversation

@cstamas
Copy link
Copy Markdown
Member

@cstamas cstamas commented Feb 4, 2026

Initial goal is to make Resolver 1.x configuration keys transparently supported, but also reduce the copy pasta around "common" HTTP configuration as well. Transport still needs to take care about their own "native" params (supported only by them).

Changes:

  • removed "smart" from connector checksums, they are included checksums (aligned with enum)
  • removed "maven2" from newly (in 2.x) config property (class is named like it, but is totally internal). Property returned to "checksums".
  • made shared helper class for transports, that provides getter, validation and transformation for all "common" HTTP configurations, and reuse them.
  • checked other (important) properties, like split repo, and they already has "legacy" key support in place

Fixes #1783

Initial goal is to make Resolver 1.x configuration keys
transparently supported, but also reduce the copy pasta
around "common" HTTP configuration as well.

Fixes apache#1783
@cstamas cstamas added this to the 2.0.15 milestone Feb 4, 2026
@cstamas cstamas requested a review from kwin February 4, 2026 16:39
@cstamas cstamas self-assigned this Feb 4, 2026
@cstamas cstamas added the enhancement New feature or request label Feb 4, 2026
@cstamas cstamas marked this pull request as ready for review February 4, 2026 16:53
* @see RepositorySystemSession#getConfigProperties()
* @since 2.0.15
*/
public final class TransportUtils {
Copy link
Copy Markdown
Member

@kwin kwin Feb 5, 2026

Choose a reason for hiding this comment

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

Why not introducing AbstractHttpTransporter extends AbstractTransporter implements HttpTransporter and put those methods there and then inherit from it in all HTTP transporter impls?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All those methods seems HTTP specific anyhow.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

HttpTransport is in SPI, ConfigUtil is in utils. Spi and Utils does not depend on each other, only on API. This looks much simpler, but I have to admit I started on same trail you suggest, just to realize is impossible.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

      api
    /     \
  spi    util
    \     /
(any) transport

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not changing that, i.e. make SPI dependent on util as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nope, I don't want to touch dep layout in patch release.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, let us then just rename this to HttpTransport(er)Utils. I would not want to mix it with other more generic configuration.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok, but in that case I'd not only rename but also align packages with SPI (so move it deeper into http package just like HttpTransport is in SPI)

…ector/transport/http/HttpTransporterUtils.java

Co-authored-by: Konrad Windszus <konrad@windszus.net>
Copy link
Copy Markdown
Member

@kwin kwin left a comment

Choose a reason for hiding this comment

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

Thanks for the changes.

@cstamas cstamas merged commit 968692d into apache:master Feb 5, 2026
13 of 14 checks passed
@cstamas cstamas deleted the issue-1783 branch February 5, 2026 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Align configuration properties

2 participants