Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I bumped into a race condition while using the existing
DiskShare#mkdir
What happened was that I had 3 threads executing the following client code:
This is a method I wrote myself in order to deal with the fact that
#mkdir
throws an exception when the directory already exists.But it was flawed, as the operation was not atomic.
A better way to deal with this problem could have been to silent the exception in case the status is STATUS_OBJECT_NAME_COLLISION:
That's ok but still it's not the best.
The best is to have
#mkdir
not throw in the first place, when the directory already exists.So here I'm proposing to add a
#mkdirs
method along with#mkdir
, with the same semantics as Files#createDirectories (I even copied its Javadoc :) )I didn't write tests for a couple of reasons:
#mkdirs
, together with any other method providing high level, client-facing functionality (e.g.SmbFiles#copy
, which could be very useful to users; it's unfortunate it is so "hidden"), in a Facade class which depended on a lower level, mockable class. So then it would be possible to write a simple test with mocks like this one:Also (but this should be another topic actually), I think it would be good if the path arguments we take from the user were of type
Path
.It would make our lives easier:
Path#resolve
), joining paths (it's so annoying having to dopath + "\\" file
all the time, isn't it? ;) )Plus, it would make sense from a domain point of view: we're really dealing with paths!
As an example, this is how my
#mkdirs
could look like: