Merge lp:~zorba-coders/zorba/bug-1009366 into lp:zorba

Proposed by Till Westmann on 2012-06-14
Status: Merged
Approved by: Chris Hillery on 2012-06-14
Approved revision: 10878
Merged at revision: 10878
Proposed branch: lp:~zorba-coders/zorba/bug-1009366
Merge into: lp:zorba
Diff against target: 66 lines (+25/-9)
2 files modified
src/compiler/translator/translator.cpp (+18/-7)
test/api/userdefined_uri_resolution.cpp (+7/-2)
To merge this branch: bzr merge lp:~zorba-coders/zorba/bug-1009366
Reviewer Review Type Date Requested Status
Chris Hillery 2012-06-14 Approve on 2012-06-14
Markos Zaharioudakis 2012-06-14 Approve on 2012-06-14
Review via email: mp+110410@code.launchpad.net

This proposal supersedes a proposal from 2012-06-06.

Commit message

add location information to ZXQP0029_URI_ACCESS_DENIED

Description of the change

add location information to ZXQP0029_URI_ACCESS_DENIED

To post a comment you must log in.
Markos Zaharioudakis (markos-za) wrote : Posted in a previous version of this proposal

It is better (more robust) if the catch in translator.cpp catches ZorbaException instead of XQueryException.

Chris Hillery (ceejatec) wrote : Posted in a previous version of this proposal

Please add the "correct exception" message back in test_userdefined_uri_resolvers, so that it is clear when the test passes. Also, as it is, if the test fails you will report the exception twice; that's probably not necessary.

Other than that and Markos' comment, looks fine.

Till Westmann (tillw) wrote : Posted in a previous version of this proposal

I've changed the catch(...) in translator.cpp and I've put the "correct exception" message back into the test.
However, I don't see why the exception is reported twice, if the test fails.

review: Approve
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job bug-1009366-2012-06-14T20-56-11.007Z is finished. The final status was:

All tests succeeded!

Zorba Build Bot (zorba-buildbot) wrote :

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 2 Approve, 1 Pending.

Chris Hillery (ceejatec) wrote :

I think I mis-read the diff last time when I mentioned the double output of exceptions (I didn't see it as two separate catch() clauses). Latest changes look fine.

review: Approve
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job bug-1009366-2012-06-14T23-40-03.247Z is finished. The final status was:

All tests succeeded!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/compiler/translator/translator.cpp'
2--- src/compiler/translator/translator.cpp 2012-06-14 15:20:15 +0000
3+++ src/compiler/translator/translator.cpp 2012-06-14 20:36:35 +0000
4@@ -2967,13 +2967,24 @@
5 // rather than using compURI directly, because we want the version
6 // fragment to be passed to the mappers.
7 zstring lErrorMessage;
8- std::auto_ptr<internal::Resource> lResource =
9- theSctx->resolve_uri(compModVer.versioned_uri(),
10- internal::EntityData::MODULE,
11- lErrorMessage);
12-
13- internal::StreamResource* lStreamResource =
14- dynamic_cast<internal::StreamResource*> (lResource.get());
15+ std::auto_ptr<internal::Resource> lResource;
16+ internal::StreamResource* lStreamResource = NULL;
17+
18+ try
19+ {
20+ lResource =
21+ theSctx->resolve_uri(compModVer.versioned_uri(),
22+ internal::EntityData::MODULE,
23+ lErrorMessage);
24+
25+ lStreamResource =
26+ dynamic_cast<internal::StreamResource*> (lResource.get());
27+ }
28+ catch (ZorbaException& e)
29+ {
30+ set_source(e, loc);
31+ throw;
32+ }
33
34 if (lStreamResource != NULL)
35 {
36
37=== modified file 'test/api/userdefined_uri_resolution.cpp'
38--- test/api/userdefined_uri_resolution.cpp 2012-06-14 15:20:15 +0000
39+++ test/api/userdefined_uri_resolution.cpp 2012-06-14 20:36:35 +0000
40@@ -21,6 +21,7 @@
41 #include <zorba/zorba.h>
42 #include <zorba/store_manager.h>
43 #include <zorba/zorba_exception.h>
44+#include <zorba/xquery_exception.h>
45 #include <zorba/uri_resolvers.h>
46 #include <zorba/diagnostic_list.h>
47
48@@ -297,12 +298,16 @@
49 "'http://expath.org/ns/file'; "
50 "1 + 1", lContext);
51 std::cout << lQuery << std::endl;
52- } catch (ZorbaException& e) {
53+ } catch (XQueryException& e) {
54 std::cout << "Caught exception: " << e.what() << std::endl;
55- if (e.diagnostic() == zerr::ZXQP0029_URI_ACCESS_DENIED) {
56+ if (e.diagnostic() == zerr::ZXQP0029_URI_ACCESS_DENIED
57+ && e.has_source()
58+ && e.source_line() == 1) {
59 std::cout << "...the correct exception!" << std::endl;
60 return true;
61 }
62+ } catch (ZorbaException& e) {
63+ std::cout << "Caught unexpected exception: " << e.what() << std::endl;
64 }
65 return false;
66 }

Subscribers

People subscribed via source and target branches