search

bitsofinfo / hazelcast-docker-swarm-discovery-spi

Docker Swarm based discovery strategy SPI for Hazelcast enabled applications
Apache License 2.0
39 stars 33 forks source link

Move strictDockerServiceNameComparison check #39

Closed bitsofinfo closed 5 years ago

bitsofinfo commented 5 years ago

@robinroos one other thing that needs fixing here is this check/line:

https://github.com/bitsofinfo/hazelcast-docker-swarm-discovery-spi/blob/39a02f3a84b6a17c9480343116da24257db846c1/src/main/java/org/bitsofinfo/hazelcast/discovery/docker/swarm/SwarmDiscoveryUtil.java#L480

This switch is comparing strictDockerServiceNameComparison against a generically typed ServiceFilter with no evidence its an implementation of NameBasedServiceFilter. So in the future if other impls are created this could inadvertently negate those which would not be the intent.

The check against strictDockerServiceNameComparison should be moved to here where the actual NameBasedServiceFilter is being passed/constructed. I'd prefer this block check the strictDockerServiceNameComparison flag then pass either NameBasedServiceFilter OR NullBasedServiceFitler based on the value of strictDockerServiceNameComparison

https://github.com/bitsofinfo/hazelcast-docker-swarm-discovery-spi/blob/39a02f3a84b6a17c9480343116da24257db846c1/src/main/java/org/bitsofinfo/hazelcast/discovery/docker/swarm/SwarmDiscoveryUtil.java#L352

robinroos commented 5 years ago

This switch is comparing strictDockerServiceNameComparison against a generically typed ServiceFilter ...

Agreed, the abstraction is a little "leaky". I will refactor that today.

robinroos commented 5 years ago

PR #43 addresses this.