TheElectronWill / night-config

Powerful java configuration library for toml, yaml, hocon, json and in-memory configurations. Serialization/deserialization framework.
GNU Lesser General Public License v3.0
242 stars 28 forks source link

IndexOutOfBoundsException when converting a list of unknown generic type #142

Closed mani1232 closed 1 year ago

mani1232 commented 1 year ago

image image image

TheElectronWill commented 1 year ago

To debug this properly, I would need:

It's not possible for me to debug your entire program, and it's even more difficult if all I have is a screenshot that I cannot copy-paste to run.

TheElectronWill commented 1 year ago

Possible root cause of the problem: ObjectConverter cannot handle a List<T> where T is unknown.

mani1232 commented 1 year ago

Json result

{
    "objects": [
        {
            "password": null,
            "passedCities": [],
            "objectId": "64a154ba9b75fa2dc300acc1",
            "username": null
        }, 
        {
            "password": null,
            "passedCities": [],
            "objectId": "64a154ba9b75fa2dc300acc2",
            "username": null
        }
    ]
}

Json api class

public class JsonDBAPI<T extends ObjectsDefault> implements DataBaseAPI<T> {
    private final ObjectConverter objectConverter;
    private final FileConfig fileConfig;
    Config config = new Config();

    public JsonDBAPI(DataBase<T> tDataBase) {
        objectConverter = new ObjectConverter();
        fileConfig = FileConfig.builder(tDataBase.getDbUrlOrPath()).onFileNotFound(FileNotFoundAction.CREATE_EMPTY).parsingMode(ParsingMode.ADD).autosave().build();
        fileConfig.load();
        Utils.scheduleWithFixedDelay(() -> objectConverter.toConfig(config, fileConfig), 150, 300, TimeUnit.SECONDS); // Save config every 5 min
        objectConverter.toObject(fileConfig, config);
    }

    @Override
    public T getObject(String fieldId, Object fieldValue) {
        return config.objects.stream().filter(t -> {
            try {
                return t.getClass().getDeclaredField(fieldId).get(t).equals(fieldValue);
            } catch (NoSuchFieldException | IllegalAccessException e) {
                e.printStackTrace();
                return false;
            }
        }).findFirst().orElse(null);
    }

    @Override
    public boolean createObject(T newObject) {
        System.out.println(config.objects.toString());
        config.objects.add(newObject);
        return true;
    }

    @Override
    public boolean replaceObject(String fieldId, Object fieldValue, T updateData) {
        Optional<T> objectToUpdate = config.objects.stream().filter(t -> {
            try {
                return t.getClass().getDeclaredField(fieldId).get(t).equals(fieldValue);
            } catch (NoSuchFieldException | IllegalAccessException e) {
                e.printStackTrace();
                return false;
            }
        }).findFirst();

        if (objectToUpdate.isPresent()) {
            int index = config.objects.indexOf(objectToUpdate.get());
            config.objects.set(index, updateData);
            return true;
        }

        return false;
    }

    @Override
    public boolean contains(String fieldId, Object fieldValue) {
        for (T t : config.objects) {
            try {
                if (t.getClass().getDeclaredField(fieldId).get(t).equals(fieldValue)) {
                    return true;
                }
            } catch (NoSuchFieldException | IllegalAccessException e) {
                e.printStackTrace();
                return false;
            }
        }
        return false;
    }

    @NoArgsConstructor
    @Getter
    @Setter
    public class Config {
        ArrayList<T> objects = new ArrayList<>();
    }
}

test json

dataBase = new DataBase<>("database.json", "local", CityUser.class);
dataBase.getDataBaseAPI().createObject(new CityUser());
dataBase.getDataBaseAPI().createObject(new CityUser());

select db type

@Getter
public class DataBase<T extends ObjectsDefault> {

    String dbUrlOrPath;
    DataBaseAPI<T> dataBaseAPI;

    public DataBase(String dbUrlOrPath, String type, Class<T> tClass) {
        this.dbUrlOrPath = dbUrlOrPath;
        if (type.equalsIgnoreCase("mongo")) this.dataBaseAPI = new MongoDBAPI<>(this, tClass);
        else this.dataBaseAPI = new JsonDBAPI<>(this);
    }
}

User

@Getter
@Setter
@NoArgsConstructor
public class CityUser extends ObjectsDefault {

    {
        objectId = ObjectId.get().toString();
        passedCities = new ArrayList<>();
    }

    public String username;
    public String password;
    public ArrayList<String> passedCities;
}
TheElectronWill commented 1 year ago

But there's no main in here, how do I run that? I don't need the entire JsonApi class.

mani1232 commented 1 year ago

But there's no main in here, how do I run that? I don't need the entire JsonApi class.

image

test json run it

mani1232 commented 1 year ago

DataBaseAPI

public interface DataBaseAPI<T extends ObjectsDefault> {

    T getObject(String fieldId, Object fieldValue);
    boolean createObject(T newObject);
    boolean replaceObject(String fieldId, Object fieldValue, T updateData);
    boolean contains(String fieldId, Object fieldValue);

}

ObjectsDefault

import org.bson.codecs.pojo.annotations.BsonId;

@Getter
@Setter
@NoArgsConstructor
public abstract class ObjectsDefault {
    @BsonId
    public String objectId;
}
TheElectronWill commented 1 year ago

test json run it

I can't copy-paste an image :'(

It's not very "minimal" to create many files just to run one small test :S Also, some imports are missing. (and I'd prefer not to install bson)

mani1232 commented 1 year ago

One thing I can say for sure, it translates fine into config, but not from config

TheElectronWill commented 1 year ago

here's a minimal program that reproduces the problem:

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import com.electronwill.nightconfig.core.Config;

public class GenericParamTest {

    static class GenericClass<T> {
        List<T> list = new ArrayList<>();
    }

    public static void main(String[] args) {
        ObjectConverter converter = new ObjectConverter();

        Config conf = Config.inMemory();
        conf.set("list", Arrays.asList("a", "b"));

        // ERROR: IndexOutOfBoundsException
        converter.toObject(conf, GenericClass<String>::new);

        // OK: object -> config
        GenericClass<String> object = new GenericClass<>();
        object.list = Arrays.asList("a", "b");
        converter.toConfig(object, Config::inMemory);
    }

}

This is caused by the fact that ObjectConverter only has local knowledge about the types, so here it sees java.util.List<T> with no information about T. Thus, it cannot proceed.

The error message could be improved, but the conversion, I'm not sure. Solution: don't use a generic parameter like that with conversions, use a concrete type.

mani1232 commented 1 year ago
    public MongoDBAPI(DataBase<T> dataBase, Class<T> tClass) {
        CodecProvider pojoCodecProvider = PojoCodecProvider.builder().automatic(true).build();
        CodecRegistry pojoCodecRegistry = fromRegistries(getDefaultCodecRegistry(), fromProviders(pojoCodecProvider));

        this.mongoClient = MongoClients.create(dataBase.getDbUrlOrPath());
        this.collection = mongoClient.getDatabase("Users").withCodecRegistry(pojoCodecRegistry).getCollection("CityUser", tClass);
    }

I could pass the class type, just like it is done in the MongoDB api

mani1232 commented 1 year ago

Can you add a method that will take

Class<T>
TheElectronWill commented 1 year ago

Not in the same way, because ObjectConverter converts the whole object at once. But more something like:

ObjectConverter converter = new ObjectConverter();
converter.addTypeHint("list", List.class, String.class); // give the type List<String> to the value "list"
converter.convert(...);
mani1232 commented 1 year ago
    private List<T> convertFromConfig(List<Config> cfgSections, Class<T> tClass) {
        return cfgSections.stream().map(section -> {
            try {
                T aClass = tClass.getDeclaredConstructor().newInstance();
                Arrays.stream(tClass.getDeclaredFields()).forEach(field -> {
                    try {
                        Object value = section.get(field.getName());
                        Field targetField = aClass.getClass().getDeclaredField(field.getName());
                        targetField.setAccessible(true);
                        targetField.set(aClass, value);
                    } catch (NoSuchFieldException | IllegalAccessException e) {
                        throw new RuntimeException(e);
                    }
                });
                return aClass;
            } catch (InstantiationException | IllegalAccessException | InvocationTargetException |
                     NoSuchMethodException e) {
                throw new RuntimeException(e);
            }
        }).toList();
    }
TheElectronWill commented 1 year ago

What is this? :think:

Note that the problem reported by this issue is not about converting the list alone, but an object that contains a list.

Converting a List<String> is OK. Converting a List<T> in a generic class isn't OK, because we cannot know the T that has been used when instantiating the object.

mani1232 commented 1 year ago

What is this?

Solution to my problem