dubbo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kezhenxu94 (GitHub) <git...@apache.org>
Subject [GitHub] [incubator-dubbo] kezhenxu94 opened issue #3106: Should org.apache.dubbo.registry.support.AbstractRegistry#getRegistered returns unmodifiable sets?
Date Mon, 31 Dec 2018 11:21:55 GMT
I notice that the method `org.apache.dubbo.registry.support.AbstractRegistry#getRegistered`
just returns the internal reference of `registered`, and everyone can modify this `Set` outside
the `Registry`, is this a good practice?

An example is that in `org.apache.dubbo.registry.support.AbstractRegistryTest#testRegister`,
there is something like:

```java
//test multiple urls
abstractRegistry.getRegistered().clear();
```

we clear the registered urls outside the `Registry`, maybe it's not a big deal because we
just do some logs and remove the url in `unregister` method:

```java
    @Override
    public void unregister(URL url) {
        if (url == null) {
            throw new IllegalArgumentException("unregister url == null");
        }
        if (logger.isInfoEnabled()) {
            logger.info("Unregister: " + url);
        }
        registered.remove(url);
    }
```

but still, it'll lose some logs and if we add more logics in the `unregister` method in the
future, it could be dangerous.

I suggest returning a unmodifiable set in the `getRegistered` method and unregister all urls
one by one or providing a method such as `unregisterAll`.

What do you think?

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3106 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


Mime
View raw message