Community
Participate
Working Groups
Related to Bug 258166. When testing what would happen if I listed a composite repository as a destination for the mirror application I noticed that a composite artifact repository returns null when getOutputStream is called. This null OutputStream is then used by the mirror application and eventually resulting in a call to an Object.wait() call. I was thinking that given the API is defined as follows, returning null may be breaking the contract. Perhaps an I/O Exception would be better. The IArtifactRepository interface defines getOutputStream as: Open an output stream to which a client can write the data for the given artifact descriptor. And further states: @return the stream to which the artifact content can be written. @throws ProvisionException if the output stream could not be created. Reasons include: An I/O exception occurred An artifact already exists at that location
This method should either be abstract, or throw an exception if isModifiable returns true.
(In reply to comment #1) > This method should either be abstract, or throw an exception if isModifiable > returns true. > What would an appropriate exception be? The contract only defines ProvisionException as being throwable yet ProvisionException is defined as a recoverable exception. If we change the method to allow us to throw another type of exception we need to modify the catch blocks of every single call to getOutputStream.
I would suggest an unsupported method exception with a message saying that the subclasses must implement it.
Created attachment 120696 [details] Changes "return null;" to "throw new UnsupportedOperationException" I was thinking that since this method will only ever throw an exception, wouldn't it just make more sense to change it to an abstract method? It's provisional so no one but us is supposed to subclass it, so it shouldn't break too much.
Created attachment 120879 [details] Changes getOutputStream to abstracty method. Implements where appropriate. I much prefer this solution to my previous one. The operation isn't Unsupported, it's unimplemented. The error message will mean nothing to the user. Abstract methods and classes exist for this very purpose. AbstractArtifactRepository is provisional so this should have any affect on any users outside of the 2 internal classes I already modified in this patch.
Released second patch to HEAD which makes the method abstract on the abstract class. Closing.
