Add additional SQLExceptions to the QueryException output

[Shredder121]

Fixes #1007

When JRE6 is used, it adds an extra Exception with additional information.
When JRE7 is used, it adds the extra SQLExceptions as suppressed exceptions.

[timowest]

Looks good. I restarted the build, cause it got stuck.

[Shredder121]

I also restarted the build, ah well.

[timowest]

@cowwoc Is this in your opinion also ok to merge?

[Shredder121]

The only thing that would really help, is a good test case.
This test case doesn't have that many nextExceptions, because it doesn't use a trigger that produces an exception, which is what @cowwoc was actually experiencing.

But in essence, the functionality is there, and since @cowwoc is probably using Java 7, he benefits fully from this pull request.

[cowwoc]

@timowest I believe this is okay to merge.

@Shredder121 My testcase, as it were, doesn't really exist anymore. I was running into such exceptions when moving code from stored procedures (DB layer) to the application layer. The code in question was removed.

You could actually do something very simple (I haven't tested this but in theory it should work):

SQLException e1 = new SQLException("Exception #1", "42001", 181);
SQLException e2 = new SQLException("Exception #2", "HY000", 1030);
e1.setNextException(e2);
SQLException sqlException = new SQLException("Batch operation failed");
sqlException.setNextException(e1);
QueryException qe = new QueryException(sqlException);

I would begin by examining what qe.printStackTrace() looks like (can we improve readability further?) and if that works I would assert that the exception contains two suppressed exceptions under JDK 7, or that there are two newline-separated sections in the exception message under JDK6.

[Shredder121]

Yes, I like your idea of the assertions, although probably we won't test with JDK6.
End users should build with skipTests=true, and because of that they won't see any output.

But I think I'll add your simple case, it constructs an Exception, with causes, so indeed, in theory it should work (although new QueryException()doesn't work, it would be SQLExceptionTranslator.translate()).

@timowest what's your take on (still) directing tests for JDK6? Is that useful?

[timowest]

@Shredder121 I believe the querydsl project can't be compiled anymore with JDK6, also because of the generics issue, but a separate project that contains a dependency to querydsl could be used to test querydsl with JDK 6-8. So the requirement is runtime compatibility with JDK 6-8.

[Shredder121]

I added combined Java 6 and 7 asserts, so it now passes correctly in both.

...well, I seem to also have mentioned a user in my commit..

[Shredder121]

I now added the simple exception creation case of @cowwoc, is it still necessary to check for 2 suppressed or newline separated exceptions as result?
printStackTrace()s are formatted with the system's line separator, so must the test suite account for both Java platform and system platform?
It could easily be implemented using a regex, but I am just wondering how everyone feels about this.

[cowwoc]

@Shredder121 If you didn't do this, how would you define success?

[timowest]

@Shredder121 A more specific assertion is in my opinion not necessary.

Is this good to merge, or is something still missing?

[Shredder121]

With the title:

Add additional SQLExceptions to the QueryException output.

and the linked issue:

#1007 QueryException truncates chained SQLExceptions

I think that the pull request does just as it says.
The assertion already confirms for suppressed exceptions for JDK7, or detailed information for JDK6, so in my opinion specific assertions are not necessary as well.

So I think that the pull request is now good to merge.

[timowest]

Fine tuning can be done via further pull requests.