SLIDER-778 Clean up AgentProviderService slightly
Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/2887c1d0 Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/2887c1d0 Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/2887c1d0 Branch: refs/heads/develop Commit: 2887c1d088172e59d1ffa3345146c84f2a2a395c Parents: cd09a64 Author: Steve Loughran <ste...@apache.org> Authored: Fri Feb 6 16:00:48 2015 +0000 Committer: Steve Loughran <ste...@apache.org> Committed: Fri Feb 6 16:00:48 2015 +0000 ---------------------------------------------------------------------- .../apache/slider/common/tools/SliderUtils.java | 19 ++++++++ .../providers/agent/AgentProviderService.java | 46 ++++++++++---------- 2 files changed, 43 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/2887c1d0/slider-core/src/main/java/org/apache/slider/common/tools/SliderUtils.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/common/tools/SliderUtils.java b/slider-core/src/main/java/org/apache/slider/common/tools/SliderUtils.java index 3182bb7..6c69ad4 100644 --- a/slider-core/src/main/java/org/apache/slider/common/tools/SliderUtils.java +++ b/slider-core/src/main/java/org/apache/slider/common/tools/SliderUtils.java @@ -154,6 +154,25 @@ public final class SliderUtils { return !isUnset(s); } + /** + * Probe for a list existing and not being empty + * @param l list + * @return true if the reference is valid and it contains entries + */ + + public static boolean isNotEmpty(List l) { + return l == null || l.isEmpty(); + } + + /** + * Probe for a map existing and not being empty + * @param m map + * @return true if the reference is valid and it contains map entries + */ + public static boolean isNotEmpty(Map m) { + return m == null || m.isEmpty(); + } + /* * Validates whether num is an integer * @param num http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/2887c1d0/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java b/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java index 310596c..f195432 100644 --- a/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java +++ b/slider-core/src/main/java/org/apache/slider/providers/agent/AgentProviderService.java @@ -616,7 +616,7 @@ public class AgentProviderService extends AbstractProviderService implements */ @Override public RegistrationResponse handleRegistration(Register registration) { - log.info("Handling registration: " + registration); + log.info("Handling registration: {}", registration); RegistrationResponse response = new RegistrationResponse(); String label = registration.getLabel(); State agentState = registration.getActualState(); @@ -650,7 +650,7 @@ public class AgentProviderService extends AbstractProviderService implements response.setLog("Label not recognized."); log.warn("Received registration request from unknown label {}", label); } - log.info("Registration response: " + response); + log.info("Registration response: {}", response); return response; } @@ -663,7 +663,7 @@ public class AgentProviderService extends AbstractProviderService implements */ @Override public HeartBeatResponse handleHeartBeat(HeartBeat heartBeat) { - log.debug("Handling heartbeat: " + heartBeat); + log.debug("Handling heartbeat: {}", heartBeat); HeartBeatResponse response = new HeartBeatResponse(); long id = heartBeat.getResponseId(); response.setResponseId(id + 1L); @@ -676,7 +676,8 @@ public class AgentProviderService extends AbstractProviderService implements CommandScript cmdScript = getScriptPathFromMetainfo(roleName); if (cmdScript == null || cmdScript.getScript() == null) { - log.error("role.script is unavailable for " + roleName + ". Commands will not be sent."); + log.error("role.script is unavailable for {}. Commands will not be sent.", + roleName); return response; } @@ -766,7 +767,8 @@ public class AgentProviderService extends AbstractProviderService implements } //If INSTALL_FAILED and no INSTALL is scheduled let the agent fail - if(componentStatus.getState() == State.INSTALL_FAILED && command == command.NOP) { + if(componentStatus.getState() == State.INSTALL_FAILED + && command == Command.NOP) { log.warn("Sending terminate signal to container that failed installation: {}", label); response.setTerminateAgent(true); } @@ -788,7 +790,7 @@ public class AgentProviderService extends AbstractProviderService implements try { instance = getAmState().getOwnedContainer(containerId); } catch (NoSuchNodeException e) { - log.warn("Failed to locate instance of container {}: {}", containerId, e); + log.warn("Failed to locate instance of container {}", containerId, e); instance = null; } for (Map.Entry<String, String> port : ports.entrySet()) { @@ -808,7 +810,7 @@ public class AgentProviderService extends AbstractProviderService implements // to allocate port or the component may need an array of ports instance.registerPortEndpoint(Integer.valueOf(portNo), portname); } catch (NumberFormatException e) { - log.warn("Failed to parse {}: {}", portNo, e); + log.warn("Failed to parse {}", portNo, e); } } } @@ -1001,8 +1003,7 @@ public class AgentProviderService extends AbstractProviderService implements protected Map<String, DefaultConfig> initializeDefaultConfigs(SliderFileSystem fileSystem, String appDef, Metainfo metainfo) throws IOException { Map<String, DefaultConfig> defaultConfigMap = new HashMap<String, DefaultConfig>(); - if (metainfo.getApplication().getConfigFiles() != null && - metainfo.getApplication().getConfigFiles().size() > 0) { + if (SliderUtils.isNotEmpty(metainfo.getApplication().getConfigFiles())) { for (ConfigFile configFile : metainfo.getApplication().getConfigFiles()) { DefaultConfig config = null; try { @@ -1029,7 +1030,7 @@ public class AgentProviderService extends AbstractProviderService implements } private String getClusterName() { - if (clusterName == null || clusterName.length() == 0) { + if (SliderUtils.isUnset(clusterName)) { clusterName = getAmState().getInternalsSnapshot().get(OptionKeys.APPLICATION_NAME); } return clusterName; @@ -1182,7 +1183,7 @@ public class AgentProviderService extends AbstractProviderService implements if (statuses != null && !statuses.isEmpty()) { log.info("Processing {} status reports.", statuses.size()); for (ComponentStatus status : statuses) { - log.info("Status report: " + status.toString()); + log.info("Status report: {}", status.toString()); if (status.getConfigs() != null) { Application application = getMetainfo().getApplication(); @@ -1212,19 +1213,19 @@ public class AgentProviderService extends AbstractProviderService implements } List<ExportGroup> appExportGroups = application.getExportGroups(); - boolean hasExportGroups = appExportGroups != null && !appExportGroups.isEmpty(); + boolean hasExportGroups = SliderUtils.isNotEmpty(appExportGroups); Set<String> appExports = new HashSet(); String appExportsStr = getApplicationComponent(componentName).getAppExports(); if (SliderUtils.isSet(appExportsStr)) { for (String appExport : appExportsStr.split(",")) { - if (appExport.trim().length() > 0) { + if (!appExport.trim().isEmpty()) { appExports.add(appExport.trim()); } } } - if (hasExportGroups && appExports.size() > 0) { + if (hasExportGroups && !appExports.isEmpty()) { String configKeyFormat = "${site.%s.%s}"; String hostKeyFormat = "${%s_HOST}"; @@ -1246,7 +1247,7 @@ public class AgentProviderService extends AbstractProviderService implements Set<String> modifiedGroups = new HashSet<String>(); for (ExportGroup exportGroup : appExportGroups) { List<Export> exports = exportGroup.getExports(); - if (exports != null && !exports.isEmpty()) { + if (SliderUtils.isNotEmpty(exports)) { String exportGroupName = exportGroup.getName(); ConcurrentHashMap<String, List<ExportEntry>> map = (ConcurrentHashMap<String, List<ExportEntry>>)getCurrentExports(exportGroupName); @@ -1306,7 +1307,7 @@ public class AgentProviderService extends AbstractProviderService implements Map<String, String> simpleEntries = new HashMap<String, String>(); for (Map.Entry<String, List<ExportEntry>> entry : entries.entrySet()) { List<ExportEntry> exports = entry.getValue(); - if(exports != null && exports.size() > 0) { + if(SliderUtils.isNotEmpty(exports)) { // there is no support for multiple exports per name - so extract only the first one simpleEntries.put(entry.getKey(), entry.getValue().get(0).getValue()); } @@ -1332,7 +1333,7 @@ public class AgentProviderService extends AbstractProviderService implements Application application = getMetainfo().getApplication(); for (Component component : application.getComponents()) { if (component.getName().equals(componentName)) { - if (component.getComponentExports().size() > 0) { + if (!component.getComponentExports().isEmpty()) { for (ComponentExport export : component.getComponentExports()) { String templateToExport = export.getValue(); @@ -1358,7 +1359,7 @@ public class AgentProviderService extends AbstractProviderService implements } } - if (toPublish.size() > 0) { + if (!toPublish.isEmpty()) { Map<String, String> perContainerData = null; if (!getComponentInstanceData().containsKey(containerId)) { perContainerData = new ConcurrentHashMap<String, String>(); @@ -1382,12 +1383,12 @@ public class AgentProviderService extends AbstractProviderService implements List<ExportGroup> appExportGroups = getMetainfo().getApplication().getExportGroups(); Component component = getMetainfo().getApplicationComponent(compName); if (component != null && SliderUtils.isSet(component.getCompExports()) - && appExportGroups != null && appExportGroups.size() > 0) { + && SliderUtils.isNotEmpty(appExportGroups)) { Set<String> compExports = new HashSet(); String compExportsStr = component.getCompExports(); for (String compExport : compExportsStr.split(",")) { - if (compExport.trim().length() > 0) { + if (!compExport.trim().isEmpty()) { compExports.add(compExport.trim()); } } @@ -1396,7 +1397,7 @@ public class AgentProviderService extends AbstractProviderService implements Set<String> modifiedGroups = new HashSet<String>(); for (ExportGroup exportGroup : appExportGroups) { List<Export> exports = exportGroup.getExports(); - if (exports != null && !exports.isEmpty()) { + if (SliderUtils.isNotEmpty(exports)) { String exportGroupName = exportGroup.getName(); ConcurrentHashMap<String, List<ExportEntry>> map = (ConcurrentHashMap<String, List<ExportEntry>>) getCurrentExports(exportGroupName); @@ -1431,7 +1432,8 @@ public class AgentProviderService extends AbstractProviderService implements if (existingList != null) { boolean updatedInPlace = false; for (ExportEntry entry : existingList) { - if (containerId.equalsIgnoreCase(entry.getContainerId())) { + if (containerId.toLowerCase(Locale.ENGLISH) + .equals(entry.getContainerId())) { entryToAdd.setValue(templateToExport); entryToAdd.setUpdatedTime(now.toString()); updatedInPlace = true;