Skip to content
This repository has been archived by the owner on Apr 5, 2024. It is now read-only.

Replays are not purged on startup after deletion #115

Closed
eivindveg opened this issue Mar 31, 2016 · 8 comments
Closed

Replays are not purged on startup after deletion #115

eivindveg opened this issue Mar 31, 2016 · 8 comments
Labels
Milestone

Comments

@eivindveg
Copy link
Owner

ReplayFiles are not deleted from the database when not existing anymore.

The cause of this is believed to be 463f974 or 36ec68c

@eivindveg eivindveg added the bug label Mar 31, 2016
@eivindveg eivindveg added this to the 2.0.4 milestone Mar 31, 2016
@zhedar
Copy link
Collaborator

zhedar commented Mar 31, 2016

I can check on this tonight, when I'm home again.
It should happen in https://github.com/eivindveg/HotSUploader/blob/develop/src/main/java/ninja/eivind/hotsreplayuploader/repositories/OrmLiteFileRepository.java line 119:

 else if(!replayFile.getFile().exists())
     deleteReplay(replayFile);

@eivindveg
Copy link
Owner Author

Ah. That would only delete replays missing from an already known location. I uncovered this by moving my documents folder from my SSD to my HDD without symlinking(the location changed). Would it make more sense to invert the selection?

Ie instead of deleting the database entry for each file not in the database, we delete the database entry for each entry not in the file list?

EDIT: The basic idea will be the same, but the database list is assumed to be the more complete set.

@zhedar
Copy link
Collaborator

zhedar commented Mar 31, 2016

Well that should delete all replays, which aren't at the saved location anymore, so moving them should delete them as well. But I'm currently not sure, how exactly the path is saved (i.e. filename only or absolute path) in the database.

Maybe I'm missing something here, but don't we already do the 2nd and not the 1st? If the file doesn't exist in the file system, it will be removed from the db. Or what else did you mean with file list?

@eivindveg
Copy link
Owner Author

Currently it's "if the existing file is in the database and the file does not exist, delete it". I'm proposing "if the file from the database is not in the file list, delete it"

@eivindveg
Copy link
Owner Author

Actually, reviewing that first statement actually uncovers the bug; a missing file cannot be deleted as it is not part of the comparison. We need to expand the check to two different iterations: one for creating entries for new files, and one for deleting missing files.

@zhedar
Copy link
Collaborator

zhedar commented Mar 31, 2016

Currently it's "if the existing file is in the database and the file does not exist, delete it". I'm proposing "if the file from the database is not in the file list, delete it"

That could work, if we read the existing files first, maybe faster then looking it up again. But won't fix anything yet.

Actually, reviewing that first statement actually uncovers the bug; a missing file cannot be deleted as it is not part of the comparison.

But a missing file will still be in the list obtained from the db, which it should be removed from, if absent. That's how it should work.

@eivindveg
Copy link
Owner Author

I believe ReplayFile#fromDirectory, which is the iterated collection is local from disk though, which is why the current check cannot succeed.

@zhedar
Copy link
Collaborator

zhedar commented Mar 31, 2016

You're right.

public Void call() throws Exception {
    for(ReplayFile replayFile : replayFiles) {
         if(!fromDb.contains(replayFile))
              createReplay(replayFile);
         else if(!replayFile.getFile().exists())
              deleteReplay(replayFile);
    }

should really be split into two loops, the 2nd iterating over fromDb.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants