Skip to content

Commit

Permalink
cautiously removing code #2009 #1883
Browse files Browse the repository at this point in the history 
in small steps because of how much trouble this caused in the past
  • Loading branch information
@ptrthomas
ptrthomas committed Aug 2, 2022
1 parent bcf844a commit e042e62
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 98 deletions.
10 changes: 10 additions & 0 deletions karate-core/src/main/java/com/intuit/karate/JsonUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,16 @@ public static String toCsv(List<Map<String, Object>> list) {
return sw.toString();
}

public static Object shallowCopy(Object o) {
if (o instanceof List) {
return new ArrayList((List) o);
} else if (o instanceof Map) {
return new LinkedHashMap((Map) o);
} else {
return o;
}
}

public static Object deepCopy(Object o) {
// anti recursion / back-references
Set<Object> seen = Collections.newSetFromMap(new IdentityHashMap());
Expand Down
5 changes: 2 additions & 3 deletions karate-core/src/main/java/com/intuit/karate/core/ScenarioBridge.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ private static Object callSingleResult(ScenarioEngine engine, Object o) throws E
}
// if we don't clone, an attach operation would update the tree within the cached value
// causing future cache hit + attach attempts to fail !
o = engine.recurseAndAttachAndShallowClone(o);
o = JsonUtils.shallowCopy(o);
return JsValue.fromJava(o);
}

Expand Down Expand Up @@ -256,8 +256,7 @@ public Object callSingle(String fileName, Value arg) throws Exception {
engine.logger.warn("callSingleCache write failed, not json-like: {}", resultVar);
}
}
// functions have to be detached so that they can be re-hydrated in another js context
result = engine.recurseAndDetachAndShallowClone(resultVar.getValue());
result = resultVar.getValue();
}
CACHE.put(fileName, result);
engine.logger.info("<< lock released, cached callSingle: {}", fileName);
Expand Down
105 changes: 10 additions & 95 deletions karate-core/src/main/java/com/intuit/karate/core/ScenarioEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1065,81 +1065,10 @@ public void init() { // not in constructor because it has to be on Runnable.run(
protected Map<String, Variable> detachVariables() {
Set<Object> seen = Collections.newSetFromMap(new IdentityHashMap());
Map<String, Variable> detached = new HashMap(vars.size());
vars.forEach((k, v) -> {
Object o = recurseAndDetachAndShallowClone(k, v.getValue(), seen);
detached.put(k, new Variable(o));
});
vars.forEach((k, v) -> detached.put(k, v.copy(false))); // shallow clone
return detached;
}

// callSingle
protected Object recurseAndAttachAndShallowClone(Object o) {
return recurseAndAttachAndShallowClone(o, Collections.newSetFromMap(new IdentityHashMap()));
}

// callonce
protected Object recurseAndAttachAndShallowClone(Object o, Set<Object> seen) {
if (o instanceof List) {
o = new ArrayList((List) o);
} else if (o instanceof Map) {
o = new LinkedHashMap((Map) o);
}
return o;
}

protected Object recurseAndDetachAndShallowClone(Object o) {
return recurseAndDetachAndShallowClone("", o, Collections.newSetFromMap(new IdentityHashMap()));
}

// callonce, callSingle and detachVariables()
private Object recurseAndDetachAndShallowClone(String name, Object o, Set<Object> seen) {
if (o instanceof List) {
o = new ArrayList((List) o);
} else if (o instanceof Map) {
o = new LinkedHashMap((Map) o);
}
Object result = recurseAndDetach(name, o, seen);
return result == null ? o : result;
}

private Object recurseAndDetach(String name, Object o, Set<Object> seen) {
if (o instanceof Value) {
return null;
} else if (o instanceof List) {
List list = (List) o;
int count = list.size();
try {
for (int i = 0; i < count; i++) {
Object child = list.get(i);
Object childResult = recurseAndDetach(name + "[" + i + "]", child, seen);
if (childResult != null) {
list.set(i, childResult);
}
}
} catch (Exception e) {
logger.warn("detach - immutable list: {}", name);
}
return null;
} else if (o instanceof Map) {
if (seen.add(o)) {
Map<String, Object> map = (Map) o;
try {
map.forEach((k, v) -> {
Object childResult = recurseAndDetach(name + "." + k, v, seen);
if (childResult != null) {
map.put(k, childResult);
}
});
} catch (Exception e) {
logger.warn("detach - immutable map: {}", name);
}
}
return null;
} else {
return null;
}
}

protected <T> Map<String, T> getOrEvalAsMap(Variable var, Object... args) {
if (var.isJsOrJavaFunction()) {
Variable res = executeFunction(var, args);
Expand Down Expand Up @@ -1889,27 +1818,15 @@ private Variable callOnceResult(ScenarioCall.Result result, boolean sharedScope)
if (sharedScope) { // if shared scope
vars.clear(); // clean slate
if (result.vars != null) {
Set<Object> seen = Collections.newSetFromMap(new IdentityHashMap());
result.vars.forEach((k, v) -> {
// clone maps and lists so that subsequent steps don't modify data / references being passed around
Object o = recurseAndAttachAndShallowClone(v.getValue(), seen);
try {
vars.put(k, new Variable(o));
} catch (Exception e) {
logger.warn("[*** callonce result ***] ignoring non-json value: '{}' - {}", k, e.getMessage());
}
});
// shallow clone maps and lists so that subsequent steps don't modify data / references being passed around
result.vars.forEach((k, v) -> vars.put(k, v.copy(false)));
} else if (result.value != null) {
if (result.value.isMap()) {
((Map) result.value.getValue()).forEach((k, v) -> {
try {
vars.put((String) k, new Variable(v));
} catch (Exception e) {
logger.warn("[*** callonce result ***] ignoring non-json value from result.value: '{}' - {}", k, e.getMessage());
}
});
Map<String, Object> map = result.value.getValue();
// shallow clone newly added variables
map.forEach((k, v) -> vars.put(k, new Variable(JsonUtils.shallowCopy(v))));
} else {
logger.warn("[*** callonce result ***] ignoring non-map value from result.value: {}", result.value);
logger.warn("callonce: ignoring non-map value from result.value: {}", result.value);
}
}
init(); // this will attach and also insert magic variables
Expand All @@ -1920,9 +1837,8 @@ private Variable callOnceResult(ScenarioCall.Result result, boolean sharedScope)
// else the call() routine would try to do it again
// note that shared scope means a return value is meaningless
} else {
// deep-clone for the same reasons mentioned above
Object resultValue = recurseAndAttachAndShallowClone(result.value.getValue());
return new Variable(resultValue);
// shallow clone for the same reasons mentioned above
return result.value.copy(false);
}
}

Expand Down Expand Up @@ -1957,8 +1873,7 @@ private Variable callOnce(String cacheKey, Variable called, Variable arg, boolea
Map<String, Variable> clonedVars = called.isFeature() && sharedScope ? detachVariables() : null;
Config clonedConfig = new Config(config);
clonedConfig.detach();
Object resultObject = recurseAndDetachAndShallowClone(resultVariables.getValue());
result = new ScenarioCall.Result(new Variable(resultObject), clonedConfig, clonedVars);
result = new ScenarioCall.Result(resultVariables.copy(false), clonedConfig, clonedVars);
CACHE.put(cacheKey, result);
logger.info("<< lock released, cached callonce: {}", cacheKey);
// another routine will apply globally if needed
Expand Down

0 comments on commit e042e62

Please sign in to comment.