Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.0] Judge whether success or not by user's perspective in interface-appName mapping. #8721

Merged
merged 2 commits into from
Sep 8, 2021

Conversation

plusmancn
Copy link
Contributor

@plusmancn plusmancn commented Sep 8, 2021

What is the purpose of the change

  • Judge whether success or not by user's perspective in interface-appName mapping.
  • By the way, format the function MetadataServiceNameMapping#map: change the indent spaces from 8 to 4.

Fixes #8722

Brief changelog

Every time the provider restarted after the first start, it would raise an error "Failed register interface application mapping......".
But from the user's perspective, it means successful when the oldConfigContent has contained the current appName. So we should not throw an Exception to the user, it will confuse the user.

Just change one line code:

// => org.apache.dubbo.registry.client.metadata.MetadataServiceNameMapping#map
public boolean map(URL url) {
    // ......
    if (StringUtils.isNotEmpty(oldConfigContent)) {
        boolean contains = StringUtils.isContains(oldConfigContent, appName);
        if (contains) {
            // From user's perspective, it means successful when the oldConfigContent has contained the current appName. So we should not throw an Exception to user, it will confuse user.
            succeeded = true;   // <= The only changes for this function.
            break;
        }
        newConfigContent = oldConfigContent + COMMA_SEPARATOR + appName;
    }
    // ......
}

What's more, the map function is only called by ServiceConfig#export,the changes are safe.

// => org.apache.dubbo.config.ServiceConfig#export
protected void exported() {
    exported = true;
    List<URL> exportedURLs = this.getExportedUrls();
    exportedURLs.forEach(url -> {
        if (url.getParameters().containsKey(SERVICE_NAME_MAPPING_KEY)) {
            ServiceNameMapping serviceNameMapping = ServiceNameMapping.getDefaultExtension(getScopeModel());
            try {
                // try to register
                boolean succeeded = serviceNameMapping.map(url);
                if (succeeded) {
                    logger.info("Successfully registered interface application mapping for service " + url.getServiceKey());
                }
            } catch (Exception e) {
                logger.error("Failed register interface application mapping for service " + url.getServiceKey(), e);
            }
        }
    });
    onExported();
}

@codecov-commenter
Copy link

codecov-commenter commented Sep 8, 2021

Codecov Report

Merging #8721 (b13d017) into 3.0 (1eee76e) will decrease coverage by 0.03%.
The diff coverage is 91.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.0    #8721      +/-   ##
============================================
- Coverage     63.70%   63.67%   -0.04%     
+ Complexity      313      311       -2     
============================================
  Files          1146     1146              
  Lines         48157    48147      -10     
  Branches       7252     7250       -2     
============================================
- Hits          30680    30657      -23     
- Misses        14113    14124      +11     
- Partials       3364     3366       +2     
Impacted Files Coverage Δ
...ry/client/metadata/MetadataServiceNameMapping.java 91.83% <91.66%> (+0.17%) ⬆️
...ubbo/registry/client/AbstractServiceDiscovery.java 87.50% <0.00%> (-12.50%) ⬇️
...he/dubbo/rpc/cluster/router/state/RouterCache.java 66.66% <0.00%> (-11.12%) ⬇️
...in/java/org/apache/dubbo/common/utils/JVMUtil.java 86.79% <0.00%> (-5.67%) ⬇️
.../rpc/cluster/router/tag/TagDynamicStateRouter.java 30.00% <0.00%> (-5.00%) ⬇️
...he/dubbo/remoting/transport/netty/NettyServer.java 70.17% <0.00%> (-3.51%) ⬇️
...e/dubbo/remoting/transport/netty/NettyChannel.java 52.27% <0.00%> (-3.41%) ⬇️
...org/apache/dubbo/rpc/protocol/AbstractInvoker.java 68.46% <0.00%> (-2.71%) ⬇️
...a/org/apache/dubbo/monitor/dubbo/DubboMonitor.java 86.66% <0.00%> (-1.91%) ⬇️
...pc/cluster/support/wrapper/MockClusterInvoker.java 62.90% <0.00%> (-1.16%) ⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1eee76e...b13d017. Read the comment docs.

Copy link
Contributor

@guohao guohao left a comment

Choose a reason for hiding this comment

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

PLZ do not format all code ... Just modify the small part

@EarthChen
Copy link
Member

#8722 #8721

@plusmancn
Copy link
Contributor Author

PLZ do not format all code ... Just modify the small part

@guohao The original style is unreadable, see follow:

image

@guohao guohao merged commit 6907dde into apache:3.0 Sep 8, 2021
@plusmancn plusmancn deleted the fix-appname-map-error branch September 8, 2021 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[3.0] the same service export multi protocol error
4 participants