Pull Request #1202 Suggestions

Model 1

Improve performance by using case-insensitive map.

general

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

The current implementation iterates through all keys of `fkMap` for every call to `getFkDepth`, which can be inefficient for large schemas. It also doesn't handle the `checked` set in a case-insensitive way, potentially leading to infinite recursion. Consider using a case-insensitive map (like `TreeMap` with `String.CASE_INSENSITIVE_ORDER`) for `fkMap` to resolve these issues, or at least handle the `checked` set case-insensitively.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

// It's recommended to initialize fkMap with a case-insensitive map,
// e.g. fkMap = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
// With that change, the code can be simplified and made more efficient.
if(!fkMap.containsKey(tableName)) return -1;
if (checked.stream().anyMatch(tableName::equalsIgnoreCase)) return 0;
checked.add(tableName);
List<String> fks = fkMap.get(tableName);

Model 2

Fix potential infinite recursion

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

There's a potential infinite recursion issue since the `checked` set is using `tableName` (which might differ in case) but the filter uses case-insensitive comparison. This could lead to stack overflow when processing foreign key relationships with tables that differ only in case.

Existing Code

private int getFkDepth(String tableName, Set<String> checked){
    List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
    if(keys.isEmpty()) return -1;
    checked.add(tableName);
    List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());
    if (fks.isEmpty()) {
        return 0;
    }
    int sum = fks.size();

Improved Code

private int getFkDepth(String tableName, Set<String> checked){
    List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
    if(keys.isEmpty()) return -1;

    // Add all case variations to prevent infinite recursion
    for (String key : keys) {
        checked.add(key);
    }

    List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());
    if (fks.isEmpty()) {
        return 0;
    }
    int sum = fks.size();

Model 3

Handle null map values safely.

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

The `fkMap.get(s)` call might return `null` if a key `s` maps to a `null` list value, instead of a typical empty list for no foreign keys. Attempting `List::stream` on a `null` reference (which `flatMap(List::stream)` would do if `fkMap.get(s)` is `null`) will cause a `NullPointerException`. It is safer to add a filter step to remove any `null` lists before attempting to stream their elements.

Existing Code

List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

List<String> fks = keys.stream().map(s -> fkMap.get(s)).filter(list -> list != null).flatMap(List::stream).collect(Collectors.toList());

Handle null map keys safely.

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

If the `fkMap` could potentially contain `null` keys (which `HashMap`, for example, allows), iterating over `fkMap.keySet()` might yield a `null` string `s`. Calling `s.equalsIgnoreCase(tableName)` when `s` is `null` will result in a `NullPointerException`. Add a defensive check to ensure `s` is not `null` before calling `equalsIgnoreCase` on it.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());

Improved Code

List<String> keys = fkMap.keySet().stream().filter(s -> s != null && s.equalsIgnoreCase(tableName)).collect(Collectors.toList());

Model 4

Fix a potential infinite recursion.

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

The current case-insensitive lookup for table names is inefficient as it iterates over all keys in `fkMap` for every recursive call. More critically, the cycle detection is flawed. It adds only the input `tableName` to the `checked` set, but not other case-variant keys found for the same table. This can lead to a `StackOverflowError` in databases with cyclic dependencies and table names differing only in case.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;

// Use a case-insensitive set for cycle detection to handle table names that differ only by case.
Set<String> checkedNormalized = checked.stream().map(String::toLowerCase).collect(Collectors.toSet());
if (checkedNormalized.contains(tableName.toLowerCase())) {
    return 0; // Cycle detected
}

checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Model 5

Prevent infinite recursion with checked cycle.

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

Add a guard to prevent infinite recursion or stack overflow in case of cycles in the foreign key relationships by checking if `tableName` is already in `checked` before proceeding.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());
if (fks.isEmpty()) {
    return 0;
}
int sum = fks.size();

Improved Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
if (checked.contains(tableName)) return 0;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());
if (fks.isEmpty()) {
    return 0;
}
int sum = fks.size();

Model 6

Guard against null foreign-key lists

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

If any entry in `fkMap` unexpectedly stores a `null` list, the current stream pipeline will throw a `NullPointerException` when `flatMap` is applied. Filter out `null` values before flattening to make the method robust against such corrupt states.

Existing Code

List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

List<String> fks = keys.stream()
        .map(fkMap::get)
        .filter(list -> list != null)
        .flatMap(List::stream)
        .collect(Collectors.toList());

Normalize names in recursion guard

general

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

The algorithm now treats table names case-insensitively, yet `checked` stores them with original casing; this can cause infinite recursion when the same table is revisited with a different case. Store a normalized value (e.g., lower-case) instead.

Existing Code

checked.add(tableName);

Improved Code

checked.add(tableName.toLowerCase());

Model 7

Fix infinite recursion prevention

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

The cycle detection logic is broken because `checked.add(tableName)` uses the original parameter casing, but recursive calls may use different casing from the `keys` list. This could cause infinite recursion when tables have different case variations in the foreign key map.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;

// Add all matching keys to prevent infinite recursion
keys.forEach(checked::add);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Model 8

Simplify case-insensitive FK lookup

general

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

Use a single actual key for a case-insensitive lookup and then retrieve its FKs, ensuring your `checked` set uses the true map key. This simplifies recursion tracking and avoids handling multiple matches.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

String matchedKey = fkMap.keySet().stream()
        .filter(k -> k.equalsIgnoreCase(tableName))
        .findFirst()
        .orElse(null);
if (matchedKey == null) return -1;
checked.add(matchedKey);
List<String> fks = fkMap.getOrDefault(matchedKey, Collections.emptyList());

Avoid duplicate primary key

possible issue

File: client-java/controller/src/test/java/org/evomaster/client/java/controller/internal/db/sql/InitSqlScriptWithSmartDbCleanTest.java | Language: java

Change the hardcoded primary key to a unique value to avoid inserting a duplicate into `Xyz`. Using a new id (e.g. 3) prevents primary key constraint violations.

Existing Code

EMSqlScriptRunner.execCommand(getConnection(), "INSERT INTO Xyz (id, valueColumn, abc_id) VALUES (2, 0, 2);", true);

Improved Code

EMSqlScriptRunner.execCommand(getConnection(), "INSERT INTO Xyz (id, valueColumn, abc_id) VALUES (3, 0, 2);", true);

Model 9

Use consistent key for tracking

general

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

The case-insensitive lookup may lead to incorrect FK depth calculations when multiple table names differ only in case. Consider using the original `tableName` for `checked.add()` to maintain consistency with the lookup key.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(keys.get(0)); // Use the actual key found in fkMap
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Use single matched key only

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

When multiple keys match the case-insensitive filter, only the first match should be used to avoid duplicate foreign key entries. The current implementation collects all matching foreign keys which could lead to incorrect depth calculations.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

String matchedKey = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).findFirst().orElse(null);
if(matchedKey == null) return -1;
checked.add(matchedKey);
List<String> fks = fkMap.get(matchedKey);

Model 10

Fix infinite recursion risk

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

The code adds the original `tableName` to the `checked` set, but it should add the actual key found in `fkMap` (which may have different case). This can cause infinite recursion if table names with different cases reference each other.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
checked.add(tableName);
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
String actualKey = keys.get(0);
checked.add(actualKey);
List<String> fks = fkMap.get(actualKey);

Model 11

Fix case-insensitive map lookup logic.

possible issue

File: client-java/controller/src/main/java/org/evomaster/client/java/controller/internal/SutController.java | Language: java

The current implementation collects foreign keys from all map entries whose keys match the `tableName` case-insensitively. If `fkMap` can contain keys that only differ by case (e.g., "mytable", "MyTable"), this logic might incorrectly aggregate foreign keys. It's usually expected that a table name maps to a single set of foreign keys. Consider finding the first case-insensitive match or, preferably, use a case-insensitive map implementation when populating `fkMap`.

Existing Code

List<String> keys = fkMap.keySet().stream().filter(s-> s.equalsIgnoreCase(tableName)).collect(Collectors.toList());
if(keys.isEmpty()) return -1;
List<String> fks = keys.stream().map(s -> fkMap.get(s)).flatMap(List::stream).collect(Collectors.toList());

Improved Code

String matchingKey = fkMap.keySet().stream()
        .filter(s -> s.equalsIgnoreCase(tableName))
        .findFirst()
        .orElse(null);

if (matchingKey == null) return -1;

List<String> fks = fkMap.get(matchingKey);