Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2419#discussion_r151630914 --- Diff: storm-client/src/jvm/org/apache/storm/topology/TopologyBuilder.java --- @@ -562,43 +560,54 @@ private void initCommon(String id, IComponent component, Number parallelism) thr } Map<String, Object> conf = component.getComponentConfiguration(); if(conf!=null) common.set_json_conf(JSONValue.toJSONString(conf)); - _commons.put(id, common); + commons.put(id, common); } protected class ConfigGetter<T extends ComponentConfigurationDeclarer> extends BaseConfigurationDeclarer<T> { - String _id; + String id; public ConfigGetter(String id) { - _id = id; + this.id = id; } @SuppressWarnings("unchecked") @Override public T addConfigurations(Map<String, Object> conf) { - if(conf!=null && conf.containsKey(Config.TOPOLOGY_KRYO_REGISTER)) { - throw new IllegalArgumentException("Cannot set serializations for a component using fluent API"); + if (conf != null) { + if (conf.containsKey(Config.TOPOLOGY_KRYO_REGISTER)) { + throw new IllegalArgumentException("Cannot set serializations for a component using fluent API"); + } + if (!conf.isEmpty()) { + String currConf = commons.get(id).get_json_conf(); + commons.get(id).set_json_conf(mergeIntoJson(parseJson(currConf), conf)); + } } - String currConf = _commons.get(_id).get_json_conf(); - _commons.get(_id).set_json_conf(mergeIntoJson(parseJson(currConf), conf)); return (T) this; } - @SuppressWarnings("unchecked") @Override - public T addResource(String resourceName, Number resourceValue) { - Map<String, Double> resourcesMap = (Map<String, Double>) getRASConfiguration().getOrDefault( - Config.TOPOLOGY_COMPONENT_RESOURCES_MAP, new HashMap()); - - resourcesMap.put(resourceName, resourceValue.doubleValue()); - - getRASConfiguration().put(Config.TOPOLOGY_COMPONENT_RESOURCES_MAP, resourcesMap); + public T addResources(Map<String, Double> resources) { + if (resources != null && !resources.isEmpty()) { + String currConf = commons.get(id).get_json_conf(); + Map<String, Object> conf = parseJson(currConf); + Map<String, Double> currentResources = + (Map<String, Double>) conf.computeIfAbsent(Config.TOPOLOGY_COMPONENT_RESOURCES_MAP, (k) -> new HashMap<>()); + currentResources.putAll(resources); + commons.get(id).set_json_conf(JSONValue.toJSONString(currConf)); --- End diff -- Isn't this writing the original currConf string back to commons instead of the modified conf map?
---