-2

I am attempting to create a config handler that will automatically parse the updated config or remove the config if deleted. The watcher itself is listed below. The issue I am having is with the files HashMap. In my constructor I check for already existing configs and add it to the hash map. This section is fine. However, when the timer executes and calls the run() method the statement: Long storedModified = files.get(f); is null causing every file to be an 'add' action.

public abstract class DirWatcher extends TimerTask {

    private final File folder;
    public HashMap<File, Long> files = new HashMap<>(); //Keeps track of modified time

    public DirWatcher(String path) {
        this.folder = new File(path);
        System.out.println("Watching files on path: " + path);
        //gets initial files
        File[] startingFiles = this.folder.listFiles(file -> file.getName().endsWith(".json"));

        if(startingFiles == null || startingFiles.length < 1) return;

        for (File file : startingFiles) {
            System.out.println("Starting: File is " + file.getName());
            files.put(file, file.lastModified());
        }
        System.out.println(files); //Has values

    }

    public final void run() {
        System.out.println(files); // = {} but should have those values from constructor
        HashSet<File> checkedFiles = new HashSet<>(); //This is to check for deleted files.
        for(File f : getConfigFiles()) { //Check the dir because there could be new files
            Long storedModified = files.get(f); //see if we currently track it.
            checkedFiles.add(f);
            if(storedModified == null) { //if not, it is a new file.
                files.put(f, f.lastModified());
                onUpdate(f, "add");
            }
            else if(storedModified != f.lastModified()) { //It is an update to the file
                files.put(f, f.lastModified()); //add it to tracker
                onUpdate(f, "modified");
            }
        }
        //Now check for deleted files. We can remove it from files hashmap and stop tracking.
        Set<File> ref = files.keySet();
        ref.removeAll(checkedFiles);
        for (File deletedFile : ref) {
            files.remove(deletedFile); //remove from tracking.
            onUpdate(deletedFile, "delete");

        }
    }
public File[] getConfigFiles() {
        return folder.listFiles(file -> file.getName().endsWith(".json"));
    }
}

Implementation: Note that files here actually prints the config files it has found so far.

public ConfigHandler(Instance instance) { //Instance just gets me the folder.
    this.configDir = instance.getDataFolder();
    this.path = this.configDir.getAbsolutePath();
    System.out.println(this.configDir);

    TimerTask configWatch = new DirWatcher(this.path) {
        @Override
        protected void onUpdate(File file, String action) {
            System.out.println("[FILE WATCHER] Found updated file with action:" + action + ". File is " + file.getName());
            System.out.println(files);
        }
    };
    Timer timer = new Timer();
    timer.schedule(configWatch, new Date(), 5000);

}

I have been logging values at various points but I still don't understand why this hashmap is empty when my run() method is called.


EDIT: moved checking for deleted files outside of the for loop & add getConfigFiles method.

samwit
  • 13
  • 2
  • Is that java in `Android project` or `Spring` or what? – Mohamad Ghaith Alzin Dec 12 '22 at 00:38
  • 1
    What does `getConfigFiles()` return? If that returns a different set of files from what was in `files` already in the constructor then all the calls to `files.get(f)` would return null - even if `files` is not empty. Can you make a MCVE that someone else could run? – Tyler V Dec 12 '22 at 00:38
  • 1
    Also, your logic for deleting the checked files looks wrong - it will delete a lot more than you think... It looks like after the first `run` it could have completely deleted the contents of `files` (on the first entry from `getConfigFiles()` it deletes *all other* entries from `files`. I think you meant to put that outside the loop... – Tyler V Dec 12 '22 at 00:50
  • @TylerV you're right. That shouldn't be in the for loop. I will edit and check again. – samwit Dec 12 '22 at 01:15
  • @MohamadGhaithAlzin This is actually a Spigot Plugin for minecraft. However, the only method from Spigot is how I get the Files from the folder (getDataFolder) in the implementation. – samwit Dec 12 '22 at 01:35
  • @TylerV getConfigFiles returns all the files from the directory I am watching. From the updated logic for deleting deleted files from being tracked it goes: - get all of the files from the current hash map. - any left over files are files that should be deleted because they no longer exist checkedFiles and can be deleted and removed from the hash map. Even then, the first run still has an empty hashmap when starting. – samwit Dec 12 '22 at 01:38
  • I don't see how that is possible with the code you posted. Can you annotate and include the complete log output? (e.g. annotate the call points `System.out.println("[CONSTRUCTOR]:" + files + " in " + this);` and `System.out.println("[RUN]: " + files + " in " + this);`? Also, `files` is public so potentially something else could be editing it that you haven't included here. Can you make it private to make sure these are the only methods that modify it? – Tyler V Dec 12 '22 at 02:09
  • @TylerV here are the logs. https://pastebin.com/CAHbCvWA - i've also changed the map back to private. I just used that so I could see what the value of files is in the ConfigHandler – samwit Dec 12 '22 at 02:25

1 Answers1

0

It maybe a multi-threading question.
The class Timer has a field called thread, and the code is private final TimerThread thread = new TimerThread(queue); If you are using the class java.util.Timer.

In your code, timer.schedule(configWatch, new Date(), 5000); means do first call immediately. This is a synchronization problem, you can try this: private HashMap<File, Long> files = new ConcurrentHashMap<>();
ConcurrentHashMap is a thread-safe Map.

This is another way to watch file changes: Watching a Directory for Changes in Java
It's also need another thread to do this job.

And, you can design a command to allow user reload config file manually.

Update

After I review your log, I found this:

[21:24:12 INFO]: [STDOUT] [RUN]: {C:\Users\user\Desktop\1.19 paper\plugins\HeadHunting\Untitled-1.json=1670799307531} in me.sam.Config.ConfigHandler$1@3f7c8e5b
[21:24:17 INFO]: [STDOUT] [RUN]: {} in me.sam.Config.ConfigHandler$1@3f7c8e5b

In first run, the files field has value, in second run, it's empty.
I think, the problem is here.

//Now check for deleted files. We can remove it from files hashmap and stop tracking.
        Set<File> ref = files.keySet();
        ref.removeAll(checkedFiles);
        for (File deletedFile : ref) {
            files.remove(deletedFile); //remove from tracking.
            onUpdate(deletedFile, "delete");

        }

Here is some code from jdk, java.util.HashMap

    public Set<K> keySet() {
        Set<K> ks = keySet;
        if (ks == null) {
            ks = new KeySet();
            keySet = ks;
        }
        return ks;
    }

    // and the KeySet.remove method code
    public final boolean remove(Object key) {
        return removeNode(hash(key), key, null, false, true) != null;
    }

I think, maybe you should do like this.

Set<File> ref = new HashSet<>(files.keySet());
ref.removeAll(checkedFiles);
// ...
hideDragon
  • 344
  • 2
  • 5
  • Thanks for the response, I tried changing the hash map to a `ConcurrentHashMap` but I still get the same issue. The fact the hash map has a value for the first run but nothing for any subsequent runs is what I don't understand. I did initially look into the Watcher but upon a file modify/creation there is more than one event being fired? The end goal here is to parse the JSON of the changed file. I would like to not parse it more times than I need to. Also, that is true. I was just trying to explore other options for reloading the file. – samwit Dec 12 '22 at 04:12
  • Yep, the `HashSet` did it. Thank you very much. – samwit Dec 12 '22 at 04:53