Overwriting a volume can leave a dangling volume file in the database

[ebrahimebrahim]

When using Databse.write_volume if we choose to overwrite an existing volume but use a volume file with a different filename, then the old volume file is left there dangling. (It exists only in the filesystem but it is still there in the database directory taking up space.)

[HussainAther]

Hey @ebrahimebrahim, I’m interested in fixing this! Could you clarify the expected behavior—should we delete the old volume immediately or mark it for cleanup?

[ebrahimebrahim]

This one is a weird edge case, a good way to explain it is perhaps to give you a chunk of code that fails because of it. So here is a unit test that you can add to test_databse to see the issue:

def test_issue_136(example_database:Database, tmp_path:Path):
    subject_id = "example_subject"
    volume_id = "a_volume_id"
    volume_name = "a_volume_name"
    volume_data_path = Path(tmp_path/'test_db_files/a_volume_filename.nii')
    volume_data_path.parent.mkdir(parents=True, exist_ok=True)
    volume_data_path.touch()
    volume_data_path2 = Path(tmp_path/'test_db_files/a_different_volume_filename.nii')
    volume_data_path2.parent.mkdir(parents=True, exist_ok=True)
    volume_data_path2.touch()

    # write volume with the data path volume_data_path
    example_database.write_volume(subject_id, volume_id, volume_name, volume_data_path)
    
    # check that the file is there in the DB with the expected name
    volume_filepath = example_database.get_volume_metadata_filepath(subject_id, volume_id)
    assert((volume_filepath.parent/volume_data_path.name).exists())

    # now overwrite using a different volume data path volume_data_path2, with a different filename
    example_database.write_volume(subject_id, volume_id, volume_name, volume_data_path2, on_conflict=OnConflictOpts.OVERWRITE)

    # check that the file is there in the DB with the expected name
    volume_filepath = example_database.get_volume_metadata_filepath(subject_id, volume_id)
    assert((volume_filepath.parent/volume_data_path2.name).exists())

    # check that the old file was removed
    assert(not (volume_filepath.parent/volume_data_path.name).exists())

Paste this into test_database.py and run it with pytest -k issue_136, or put it into a notebook. The last line, checking that the old file was removed, is what fails