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

ServiceConfigurationListener should override ProviderConfigurationListener #4175

Closed
2 tasks
uglycow opened this issue May 27, 2019 · 0 comments
Closed
2 tasks
Labels
type/bug Bugs to being fixed
Milestone

Comments

@uglycow
Copy link
Contributor

uglycow commented May 27, 2019

  • I have searched the issues of this repository and believe that this is not a duplicate.
  • I have checked the FAQ of this repository and believe that this is not a duplicate.

Environment

  • Dubbo version: 2.7.2-SNAPSHOT
  • Operating System version: xxx
  • Java version: xxx

Steps to reproduce this issue

�the override rule logic is different between Provider and Consumer.

Provider check ProviderConfigurationListener and then the ServiceConfigurationListener,

Consumer check ConsumerConfigurationListener and then the ReferenceConfigurationListener

i think we should keep them consistent and ServiceConfigurationListener should override ProviderConfigurationListener.

provider:
org.apache.dubbo.registry.integration.RegistryProtocol.OverrideListener#doOverrideIfNecessary

public synchronized void doOverrideIfNecessary() {
            final Invoker<?> invoker;
            if (originInvoker instanceof InvokerDelegate) {
                invoker = ((InvokerDelegate<?>) originInvoker).getInvoker();
            } else {
                invoker = originInvoker;
            }
            //The origin invoker
            URL originUrl = RegistryProtocol.this.getProviderUrl(invoker);
            String key = getCacheKey(originInvoker);
            ExporterChangeableWrapper<?> exporter = bounds.get(key);
            if (exporter == null) {
                logger.warn(new IllegalStateException("error state, exporter should not be null"));
                return;
            }
            //The current, may have been merged many times
            URL currentUrl = exporter.getInvoker().getUrl();
            //Merged with this configuration
            URL newUrl = getConfigedInvokerUrl(configurators, originUrl);
            newUrl = getConfigedInvokerUrl(serviceConfigurationListeners.get(originUrl.getServiceKey())
                    .getConfigurators(), newUrl);
            newUrl = getConfigedInvokerUrl(providerConfigurationListener.getConfigurators(), newUrl);
            if (!currentUrl.equals(newUrl)) {
                RegistryProtocol.this.reExport(originInvoker, newUrl);
                logger.info("exported provider url changed, origin url: " + originUrl +
                        ", old export url: " + currentUrl + ", new export url: " + newUrl);
            }
        }

consumer:
org.apache.dubbo.registry.integration.RegistryDirectory#overrideWithConfigurator

private URL overrideWithConfigurator(URL providerUrl) {
        // override url with configurator from "override://" URL for dubbo 2.6 and before
        providerUrl = overrideWithConfigurators(this.configurators, providerUrl);

        // override url with configurator from configurator from "app-name.configurators"
        providerUrl = overrideWithConfigurators(CONSUMER_CONFIGURATION_LISTENER.getConfigurators(), providerUrl);

        // override url with configurator from configurators from "service-name.configurators"
        if (serviceConfigurationListener != null) {
            providerUrl = overrideWithConfigurators(serviceConfigurationListener.getConfigurators(), providerUrl);
        }

        return providerUrl;
    }

Pls. provide [GitHub address] to reproduce this issue.

Expected Result

What do you expected from the above steps?

Actual Result

What actually happens?

If there is an exception, please attach the exception trace:

Just put your stack trace here!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Bugs to being fixed
Projects
None yet
Development

No branches or pull requests

2 participants