refactor: Avoid calling overridable methods in constructors

[redoom]

Motivation

Avoid Calling Other Methods in Constructors https://github.com/apache/dubbo/issues/15600

The Problem

public abstract class AbstractServer {
    public AbstractServer(URL url, ChannelHandler handler) {
        super(url, handler);
        doOpen();
    }
    protected abstract void doOpen() throws Throwable;
}

public class NettyServer extends AbstractServer {
    private ServerBootstrap bootstrap;  // Still null when doOpen() called

    @Override
    protected void doOpen() {
        bootstrap.group(...);
    }
}

Solution

Two-phase initialization with static factory methods:

// ✅ After: Separate construction from initialization
protected AbstractServer(URL url, ChannelHandler handler) {
    super(url, handler);
    // Only field initialization
}

protected final void init() throws RemotingException {
    doOpen();  // object fully constructed
}

public static NettyServer create(URL url, ChannelHandler handler) {
    NettyServer server = new NettyServer(url, handler);
    server.init();
    return server;
}

private NettyServer(URL url, ChannelHandler handler) {
    super(url, ChannelHandlers.wrap(handler, url));
}

Changes

• Split initialization: construction (fields only) + init() (resources)
• Private constructors + static factory methods
• Final init() methods prevent subclass override

// ✅
NettyServer server = NettyServer.create(url, handler);

// ❌
// NettyServer server = new NettyServer(url, handler);

[codecov-commenter]

Codecov Report

:x: Patch coverage is 76.31579% with 36 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 60.43%. Comparing base (f284fab) to head (21efca6). :warning: Report is 1 commits behind head on 3.3.

Files with missing lines	Patch %	Lines
...c/cluster/router/affinity/AffinityStateRouter.java	37.50%	4 Missing and 1 partial :warning:
...gistry/client/metadata/store/MetaCacheManager.java	75.00%	2 Missing and 3 partials :warning:
...ache/dubbo/registry/multiple/MultipleRegistry.java	61.53%	1 Missing and 4 partials :warning:
...affinity/config/AffinityListenableStateRouter.java	66.66%	2 Missing and 2 partials :warning:
...e/dubbo/registry/integration/RegistryProtocol.java	60.00%	2 Missing and 2 partials :warning:
...ng/transport/netty/NettyPortUnificationServer.java	0.00%	3 Missing :warning:
...cluster/router/condition/ConditionStateRouter.java	75.00%	0 Missing and 2 partials :warning:
...pache/dubbo/remoting/transport/AbstractClient.java	60.00%	2 Missing :warning:
...bo/rpc/protocol/tri/service/TriBuiltinService.java	66.66%	1 Missing and 1 partial :warning:
...er/router/affinity/AffinityStateRouterFactory.java	0.00%	1 Missing :warning:
... and 3 more

Additional details and impacted files

@@             Coverage Diff              @@
##                3.3   #15767      +/-   ##
============================================
- Coverage     60.76%   60.43%   -0.34%     
+ Complexity    11712    10139    -1573     
============================================
  Files          1938     1938              
  Lines         88646    88727      +81     
  Branches      13379    13389      +10     
============================================
- Hits          53866    53620     -246     
- Misses        29257    29533     +276     
- Partials       5523     5574      +51     

Flag	Coverage Δ
integration-tests-java21	?
integration-tests-java8	?
samples-tests-java21	32.03% <39.47%> (-0.02%)	:arrow_down:
samples-tests-java8	?
unit-tests-java11	59.11% <73.68%> (+0.02%)	:arrow_up:
unit-tests-java17	58.57% <72.36%> (+0.02%)	:arrow_up:
unit-tests-java21	58.59% <72.36%> (+0.01%)	:arrow_up:
unit-tests-java25	58.50% <72.36%> (+<0.01%)	:arrow_up:
unit-tests-java8	59.08% <74.66%> (-0.01%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[RainYuY]

I think we shouldn’t initialize objects in their subclasses. The better way, like the code you modified, is to use final or private to restrict them so they can only be initialized within their own class.

[redoom]

I think we shouldn’t initialize objects in their subclasses. The better way, like the code you modified, is to use final or private to restrict them so they can only be initialized within their own class.

Got it