-
Notifications
You must be signed in to change notification settings - Fork 99
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
Expose exceptions in DumpProcessingController #526
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -374,7 +374,8 @@ public Sites getSitesInformation() throws IOException { | |
* @see DumpProcessingController#processDump(MwDumpFile) | ||
* @see DumpProcessingController#getMostRecentDump(DumpContentType) | ||
*/ | ||
public void processAllRecentRevisionDumps() { | ||
public void processAllRecentRevisionDumps() | ||
throws IOException, FileAlreadyExistsException { | ||
WmfDumpFileManager wmfDumpFileManager = getWmfDumpFileManager(); | ||
if (wmfDumpFileManager == null) { | ||
return; | ||
|
@@ -398,7 +399,8 @@ public void processAllRecentRevisionDumps() { | |
* | ||
* @see DumpProcessingController#processAllRecentRevisionDumps() | ||
*/ | ||
public void processMostRecentMainDump() { | ||
public void processMostRecentMainDump() | ||
throws IOException, FileAlreadyExistsException { | ||
DumpContentType dumpContentType; | ||
if (this.preferCurrent) { | ||
dumpContentType = DumpContentType.CURRENT; | ||
|
@@ -419,7 +421,8 @@ public void processMostRecentMainDump() { | |
* | ||
* @see DumpProcessingController#processAllRecentRevisionDumps() | ||
*/ | ||
public void processMostRecentJsonDump() { | ||
public void processMostRecentJsonDump() | ||
throws IOException, FileAlreadyExistsException { | ||
processDump(getMostRecentDump(DumpContentType.JSON)); | ||
} | ||
|
||
|
@@ -434,7 +437,8 @@ public void processMostRecentJsonDump() { | |
* @param dumpFile | ||
* the dump to process | ||
*/ | ||
public void processDump(MwDumpFile dumpFile) { | ||
public void processDump(MwDumpFile dumpFile) | ||
throws IOException, FileAlreadyExistsException { | ||
if (dumpFile == null) { | ||
return; | ||
} | ||
|
@@ -493,18 +497,23 @@ public MwDumpFile getMostRecentDump(DumpContentType dumpContentType) { | |
* the dump file processor to use | ||
*/ | ||
void processDumpFile(MwDumpFile dumpFile, | ||
MwDumpFileProcessor dumpFileProcessor) { | ||
MwDumpFileProcessor dumpFileProcessor) | ||
throws IOException, FileAlreadyExistsException { | ||
try (InputStream inputStream = dumpFile.getDumpFileStream()) { | ||
dumpFileProcessor.processDumpFileContents(inputStream, dumpFile); | ||
} catch (FileAlreadyExistsException e) { | ||
logger.error("Dump file " | ||
String errorMessage = "Dump file " | ||
+ dumpFile.toString() | ||
+ " could not be processed since file " | ||
+ e.getFile() | ||
+ " already exists. Try deleting the file or dumpfile directory to attempt a new download."); | ||
+ " already exists. Try deleting the file or dumpfile directory to attempt a new download."; | ||
logger.error(errorMessage); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure the logging is still useful now that we raise the exception again. |
||
throw new FileAlreadyExistsException(errorMessage); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice to give to the constructor the file names to allow |
||
} catch (IOException e) { | ||
logger.error("Dump file " + dumpFile.toString() | ||
+ " could not be processed: " + e.toString()); | ||
String errorMessage = "Dump file " + dumpFile.toString() | ||
+ " could not be processed: " + e.toString(); | ||
logger.error(errorMessage); | ||
throw new IOException(errorMessage); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileAlreadyExistsException
is a sub class ofIOException
(same in other places)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps there is still a case for including both of them explicitly in the signature, because the caller might want to handle them differently?