Github user pivotal-amurmann commented on a diff in the pull request:
https://github.com/apache/geode/pull/716#discussion_r135089666
--- Diff:
geode-protobuf/src/main/java/org/apache/geode/protocol/protobuf/operations/GetAvailableServersOperationHandler.java
---
@@ -50,51 +37,23 @@
@Override
public Result<ServerAPI.GetAvailableServersResponse> process(
SerializationService serializationService,
ServerAPI.GetAvailableServersRequest request,
- Cache cache) {
-
- InternalDistributedSystem distributedSystem =
- (InternalDistributedSystem) cache.getDistributedSystem();
- Properties properties = distributedSystem.getProperties();
- String locatorsString =
properties.getProperty(ConfigurationProperties.LOCATORS);
+ MessageExecutionContext executionContext) throws
InvalidExecutionContextException {
- HashSet<DistributionLocatorId> locators = new HashSet();
- StringTokenizer stringTokenizer = new StringTokenizer(locatorsString,
",");
- while (stringTokenizer.hasMoreTokens()) {
- String locator = stringTokenizer.nextToken();
- if (StringUtils.isNotEmpty(locator)) {
- locators.add(new DistributionLocatorId(locator));
- }
+ InternalLocator locator = executionContext.getLocator();
+ ArrayList serversFromSnapshot =
--- End diff --
I very much agree with Galen that this should be refactored. This is a big
demeter violation which is pointing at some worse code in ServerLocator which
currently know how to answered requests and how to get the information to
answer them. If that was split out into one class that can talk whatever
protocol it's talking and another class that can get information from the
locator this could get cleaned up quite a bit and also make unit tests much
easier. Since we are talking about switching to Netty that might be wasted
effort at this time. On the other hand we should extract the business logic
from the transport logic anyways when moving to Netty and doing this beforehand
might make that move easier.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
