DependencyResolver.resolve - removing dependents in check version block ?

[surajpuvvada]

In DependencyResolver.resolve() - I noticed this block of code (https://github.com/pf4j/pf4j/blob/master/pf4j/src/main/java/org/pf4j/DependencyResolver.java#L93) where dependents.remove(0) is called. Doesn't this remove the dependent plugin ? Is this correct behavior ?
For example: If I have A depend on B and I try to unload B it simply goes and unloads B without unloading A as getDependents() returns empty in AbstractDefaultManager.unloadPlugin() call.

// check dependencies versions
for (PluginDescriptor plugin : plugins) {
    String pluginId = plugin.getPluginId();
    String existingVersion = plugin.getVersion();

    List<String> dependents = getDependents(pluginId);
    while (!dependents.isEmpty()) {
        String dependentId = dependents.remove(0);
        PluginDescriptor dependent = pluginByIds.get(dependentId);
        String requiredVersion = getDependencyVersionSupport(dependent, pluginId);
        boolean ok = checkDependencyVersion(requiredVersion, existingVersion);
        if (!ok) {
            result.addWrongDependencyVersion(new WrongDependencyVersion(pluginId, dependentId, existingVersion, requiredVersion));
        }
    }
}

[decebals]

Doesn't this remove the dependent plugin ? Is this correct behavior ?

The intention is not to remove the dependents plugins. The idea is to iterate over all dependents plugins and to check if the required version for each one is the expected one or not.
All this logic operates with DAG's nodes. The intention is not to remove something, only to iterate over dependents.

Do you see a problem? If yes please add more details. If you can please add a (unit) test that replicate the problem.

[surajpuvvada]

Thanks for the reply.
I think the issue is the dependents list is a direct read through from the DAG i.e. not cloned. So dependents.remove(0) removes it.

When I changed the code to use an iterator instead - it seems to work correctly.

for (PluginDescriptor plugin : plugins) {
    String pluginId = plugin.getPluginId();
    String existingVersion = plugin.getVersion();

    List<String> dependents = getDependents(pluginId);
    Iterator<String> it = dependents.iterator();
    while(it.hasNext()){
        String dependentId = it.next();
        PluginDescriptor dependent = pluginByIds.get(dependentId);
        String requiredVersion = getDependencyVersionSupport(dependent, pluginId);
        boolean ok = checkDependencyVersion(requiredVersion, existingVersion);
        if (!ok) {
            result.addWrongDependencyVersion(new WrongDependencyVersion(pluginId, dependentId, existingVersion, requiredVersion));
            it.remove();
        }
    }
}

[decebals]

@surajpuvvada
Thanks! Yes, I think that is a problem with (im)mutability. I will fix the code asap.

[decebals]

@surajpuvvada
I added a fix and I created a new SNAPSHOT version. Can you test it and tell me if it's OK? Thanks!

[surajpuvvada]

@decebals
It works 👍 Thanks for the quick fix.