Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task.Folder can be null #871

Closed
PhyxionNL opened this issue Feb 16, 2021 · 7 comments
Closed

Task.Folder can be null #871

PhyxionNL opened this issue Feb 16, 2021 · 7 comments

Comments

@PhyxionNL
Copy link

Describe the bug

public TaskFolder Folder

This is marked as NotNull, but this is not correct. If you have a task and delete the folder it's contained in then Folder will return null.
See also:

TaskFolder f = null;

To Reproduce
Steps to reproduce the behavior:

  1. Create a task and keep an instance to it.
  2. Delete the task and its folder with Windows' task scheduler.
  3. Access Task.Folder.
@dahall
Copy link
Owner

dahall commented Feb 16, 2021

If you have a task instance, and delete the folder to which it is assigned, my understanding is that all of the contained tasks and subfolders are also deleted. I believe the error you are seeing is that the Task should no longer be valid and not just the Parent property.

In your described scenario, is the task instance still available from the system? This would be similar to getting a FileInfo instance for a file and then deleting the file from the system. You would still have a valid class instance but it does not represent a physical system instance.

@PhyxionNL
Copy link
Author

The Task is no longer valid as such, as it no longer exists, but Folder can return null then (which is incorrect with the attribute) . Alternatively, Folder could throw the original DirectoryNotFoundException. Throwing is probably better as some of the other properties also throw it, but would cause more things to be affected as GetFolder seems to be used in other places too.

@dahall
Copy link
Owner

dahall commented Feb 16, 2021

I think, given this scenario that I've never really accounted for, that I should review key functions and properties and ensure that they all throw an exception when trying to act on a folder or task that no longer exists. Doesn't that seem the appropriate behavior?

@PhyxionNL
Copy link
Author

I agree. Most properties of Task already throw a DirectoryNotFoundException if the folder is deleted. Some properties still work, such as the name, which is OK I guess.

@dahall
Copy link
Owner

dahall commented Feb 17, 2021

I'm considering putting in a simple method that I put in before any property is fetched that would check with TaskService to see if the task still exists and throw an exception if it was removed. Is that appropriate or too intrusive?

@dahall dahall closed this as completed in f0d92c2 Feb 17, 2021
@dahall
Copy link
Owner

dahall commented Feb 17, 2021

For now, I check for null and throw a DirectoryNotFoundException exception if returned from GetFolder. After more investigation, and your feedback, I may still implement the more elaborate check for deleted tasks and folders.

@PhyxionNL
Copy link
Author

I think that's a good enough change for now. I haven't tried all properties but the ones I'm using already throw a DirectoryNotFoundException.

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

No branches or pull requests

2 participants