[MSHARED-193] API change: let MavenReportRenderer#render() throw an exception

[jira-importer]

Benson Margulies opened MSHARED-193 and commented

It seems unfortunate that org.apache.maven.reporting.MavenReportRenderer.render() does not throw MavenReportingException. Thus, even though the execute method for a report throws that exception, rendering problems cannot.

Obviously, a change to this would ramify. Would there be any chance of acceptance for a patch that added this 'throws'? Alternatively, how about an unchecked cousin of MavenReportingException?

---

Affects: maven-reporting-impl-2.1, maven-reporting-api-3.0

Issue Links:

• MSHARED-1032 API change: let MavenReport#canGenerateReport() throw an exception

Remote Links:

• GitHub Pull Request #18
  

2 votes, 4 watchers

[jira-importer]

Michael Osipov commented

This issue has been auto closed because it has been inactive for a long period of time. If you think this issue still applies, retest your problem with the most recent version of Maven and the affected component, reopen and post your results.

[jira-importer]

Michael Osipov commented

Hervé Boutemy, after touching all of our renders and looking again at this, I think this should be closed as wontfix and I will explain why:
A renderer (view in MVC) is the last layer, there is no logic, no data retrieval. Data (model) must be fetched and prepared by the controller, here the report and passed to the view (renderer) and the view's sole purpose is to print it out. Done. See here for a good example.

Let me know what you think!

[jira-importer]

Hervé Boutemy commented

A renderer (view in MVC) is the last layer, there is no logic, no data retrieval. Data (model) must be fetched and prepared by the controller, here the report and passed to the view (renderer) and the view's sole purpose is to print it out.

yes, I agree
but "it should" does not mean that there is never a small IO call, like reading a file that was prepared previously
then yes, in general IO should not happen, but it happens sometimes and is a pain when it arrives

that's why adding a MavenReportingException is IMHO a good thing (while adding the same to canGenerate() in MSHARED-1032), to make maven-reporting-api easier to use when something unexpected happens (and avoid stupid unchecked exceptions)

[jira-importer]

Michael Osipov commented

The IO you mention I would expect to happen in the report, don't you think? At least in all of our reporting plugins we prepare everything outside of the renderer.

[jira-importer]

Hervé Boutemy commented

if prepare stores result to file (after huge computation), renderer has to read it back
perhaps looking at concrete examples where renderers were forced to work around the miss would be useful, to avoid endless theoretical discussions

[jira-importer]

Michael Osipov commented

While this looks valid at first sight, it still does not qualify. We exactly have this case: Surefire, Failsafe. Complex tests are being run, results are stored in XML files, but not read by the renderer. The report class which invokes the render reads the file into a memory representation and passes onto the renderer.

[jira-importer]

Hervé Boutemy commented

yes, reading in memory is exactly what I'm talking about avoiding

[jira-importer]

Michael Osipov commented

How likely is this? I mean if you need to stream the data to process it, then how to visualize so much data in a report? Does this then make sense?
I mean, in the past 12+ years no one complained and commented on this issue...

[jira-importer]

Michael Osipov commented

Hervé Boutemy, I'd like to settle this. WDYT? Still required?

[jira-importer]

Hervé Boutemy commented

adding a checked exception does not cost anything, lets flexibility for users
refusing just force them to tweak when they need it: I don't see why

[jira-importer]

Michael Osipov commented

You are right. Since the one with #canGenerateReport() works as well, I can make it happen here too. I will evaluate and provide a PR.

[jira-importer]

Tamas Cservenak commented

And what will user (or whatever layer) do with this checked exception? Can it act on it, or just log and fail? Also, this issue is from 2011, so java5? When there was no runtime ioex existed.

[jira-importer]

Michael Osipov commented

Tamas Cservenak, all callers of #render() already throw this exception. There is almost no change from code.

[jira-importer]

Michael Osipov commented

Fixed with 5f1556d497f492285f884bde0ac47ccb6f1e1ed7.

Merged the change since it is an easy fix.