Merge lp:~zorba-coders/zorba/bug905035 into lp:zorba

Proposed by Matthias Brantner on 2012-01-18
Status: Merged
Approved by: Rodolfo Ochoa on 2012-01-20
Approved revision: 10619
Merged at revision: 10627
Proposed branch: lp:~zorba-coders/zorba/bug905035
Merge into: lp:zorba
Diff against target: 211 lines (+131/-1)
7 files modified
ChangeLog (+2/-0)
include/zorba/static_context.h (+11/-0)
src/api/item.cpp (+7/-1)
src/api/staticcontextimpl.cpp (+32/-0)
src/api/staticcontextimpl.h (+3/-0)
test/unit/CMakeLists.txt (+1/-0)
test/unit/static_context.cpp (+75/-0)
To merge this branch: bzr merge lp:~zorba-coders/zorba/bug905035
Reviewer Review Type Date Requested Status
Rodolfo Ochoa 2012-01-18 Approve on 2012-01-20
Markos Zaharioudakis 2012-01-18 Approve on 2012-01-18
Chris Hillery 2012-01-18 Pending
Hybridum 2012-01-18 Pending
Review via email: mp+89099@code.launchpad.net

This proposal supersedes a proposal from 2012-01-17.

Commit Message

Implemented StaticContext::getNamespaceBindings to resolve bug #905035. In the same commit, the function getNamespaceURIByPrefix was deprecated because it's superseded by the new function.

Description of the Change

Implemented StaticContext::getNamespaceBindings to resolve bug #905035. In the same commit, the function getNamespaceURIByPrefix was deprecated because it's superseded by the new function.

To post a comment you must log in.
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

Validation queue job bug905035-2012-01-11T20-48-04.305Z is finished. The final status was:

All tests succeeded!

Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

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

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

Would it be better to have StaticContext::getNamespaceBindings(), rather than getDeclaredPrefixes()? That would allow users to get both the prefixes and the URIs they are bound to in one step. It would also be parallel with the new Item::getNamespaceBindings() method (and could use the same NsBindings typedef), and parallel with the existing static_context::get_namespace_bindings() method that is used internally.

Rodolfo Ochoa (rodolfo-ochoa) wrote : Posted in a previous version of this proposal

How hard is to get a zorba iterator instead?

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

IMHO a Zorba iterator wouldn't be the appropriate way to return this information, since it would require wrapping each prefix into an Item (which the end-user would probably just call getStringValue() on anyway).

Rodolfo Ochoa (rodolfo-ochoa) wrote : Posted in a previous version of this proposal

For API purposes avoid STL usage like std::vector<string>, instead try to use Zorba Classes, like Iterator, I'm having difficulties trying to use STL on other languages.

Rodolfo Ochoa (rodolfo-ochoa) wrote : Posted in a previous version of this proposal

It's fine, just FYI, this wouldn't work on Ruby, any usage of vectors from STL are excluded from Ruby.

review: Approve
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

Validation queue job bug905035-2012-01-12T22-04-03.62Z is finished. The final status was:

All tests succeeded!

Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

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

Hybridum (hybridum) : Posted in a previous version of this proposal
review: Approve
Markos Zaharioudakis (markos-za) wrote : Posted in a previous version of this proposal

I think I agree with Chris' comment about having a getNamespaceBindings() method instead. Any reason why it was not done that way?

Matthias Brantner (matthias-brantner) wrote : Posted in a previous version of this proposal

I have adapted the fix as suggested. However, I'm not sure how to address Rodolfo's issues regarding other language bindings. This seems to be a bigger problem which is present in many places in our api.

Rodolfo Ochoa (rodolfo-ochoa) wrote : Posted in a previous version of this proposal

I think I can use it like it is now...
BTW: NSBindings class is no on zorba website's documentation

review: Approve
Markos Zaharioudakis (markos-za) wrote : Posted in a previous version of this proposal

Matthias, the getNamespaceBindings method is rather inefficient as it will copy the actual strings twice. And it should also pre-allocated the space for the returned vector. I don't think this is going to be a major bottleneck, so I have approved the proposal. But in the spirit of "best practices", it is better if it is fixed.

Markos Zaharioudakis (markos-za) : Posted in a previous version of this proposal
review: Approve

I agree with Markos's comment and have made the suggested improvements. Please vote again on the proposal.

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

Validation queue job bug905035-2012-01-21T00-06-05.414Z 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 'ChangeLog'
2--- ChangeLog 2012-01-18 12:18:59 +0000
3+++ ChangeLog 2012-01-18 18:13:27 +0000
4@@ -22,6 +22,8 @@
5 support.
6 * zerr is not predeclared anymore to be http://www.zorba-xquery.com/errors
7 * Added API method Item::getNamespaceBindings().
8+ * Added API method StaticContext::getNamespaceBindings() (see bug #905035)
9+ * Deprecated StaticContext:getNamespaceURIByPrefix()
10
11 version 2.1
12
13
14=== modified file 'include/zorba/static_context.h'
15--- include/zorba/static_context.h 2011-12-21 14:40:33 +0000
16+++ include/zorba/static_context.h 2012-01-18 18:13:27 +0000
17@@ -99,10 +99,21 @@
18 * could be found for the given prefix and an DiagnosticHandler has been
19 * registered.
20 * @throw ZorbaException if an error occured (e.g. no URI could be found for the given prefix).
21+ *
22+ * @deprecated This function is deprecated. Use getNamespaceBindings instead.
23 */
24 virtual String
25 getNamespaceURIByPrefix( const String& aPrefix ) const = 0;
26
27+ /**
28+ * \brief Get the list of all namespace bindings (prefix, uri)
29+ * declared in this and its parent static contexts.
30+ *
31+ * @param aBindings the bindings are added to this list
32+ */
33+ virtual void
34+ getNamespaceBindings( NsBindings& aBindings ) const = 0;
35+
36 /** \brief Set the default element and type namespace
37 * (see http://www.w3.org/TR/xquery/#static_context)
38 *
39
40=== modified file 'src/api/item.cpp'
41--- src/api/item.cpp 2012-01-11 17:30:25 +0000
42+++ src/api/item.cpp 2012-01-18 18:13:27 +0000
43@@ -389,12 +389,18 @@
44
45 store::NsBindings lStoreBindings;
46 m_item->getNamespaceBindings(lStoreBindings, aNsScoping);
47+ aBindings.reserve(aBindings.size() + lStoreBindings.size());
48+
49 store::NsBindings::iterator ite = lStoreBindings.begin();
50 store::NsBindings::iterator end = lStoreBindings.end();
51 for (; ite != end; ++ite) {
52 zstring& prefix = ite->first;
53 zstring& nsuri = ite->second;
54- aBindings.push_back(std::pair<String, String>(prefix.str(), nsuri.str()));
55+ aBindings.push_back(
56+ std::pair<String, String>(
57+ Unmarshaller::newString(prefix),
58+ Unmarshaller::newString(nsuri))
59+ );
60 }
61
62 ITEM_CATCH
63
64=== modified file 'src/api/staticcontextimpl.cpp'
65--- src/api/staticcontextimpl.cpp 2012-01-11 17:30:25 +0000
66+++ src/api/staticcontextimpl.cpp 2012-01-18 18:13:27 +0000
67@@ -215,6 +215,38 @@
68 return "";
69 }
70
71+/*******************************************************************************
72+
73+********************************************************************************/
74+void
75+StaticContextImpl::getNamespaceBindings( NsBindings& aBindings ) const
76+{
77+ try
78+ {
79+ store::NsBindings lBindings;
80+ theCtx->get_namespace_bindings(lBindings);
81+ aBindings.reserve(aBindings.size() + lBindings.size());
82+
83+ for (store::NsBindings::const_iterator lIter = lBindings.begin();
84+ lIter != lBindings.end(); ++lIter)
85+ {
86+ aBindings.push_back(
87+ std::pair<zorba::String, zorba::String>(
88+ Unmarshaller::newString(lIter->first),
89+ Unmarshaller::newString(lIter->second)
90+ )
91+ );
92+ }
93+ }
94+ catch (ZorbaException const& e)
95+ {
96+ ZorbaImpl::notifyError(theDiagnosticHandler, e);
97+ }
98+ catch (std::exception const& e)
99+ {
100+ ZorbaImpl::notifyError(theDiagnosticHandler, e.what());
101+ }
102+}
103
104 /*******************************************************************************
105
106
107=== modified file 'src/api/staticcontextimpl.h'
108--- src/api/staticcontextimpl.h 2011-12-21 14:40:33 +0000
109+++ src/api/staticcontextimpl.h 2012-01-18 18:13:27 +0000
110@@ -78,6 +78,9 @@
111
112 String getNamespaceURIByPrefix( const String& prefix ) const;
113
114+ void
115+ getNamespaceBindings( NsBindings& aBindings ) const;
116+
117 bool setDefaultElementAndTypeNamespace( const String& URI );
118
119 String getDefaultElementAndTypeNamespace( ) const;
120
121=== modified file 'test/unit/CMakeLists.txt'
122--- test/unit/CMakeLists.txt 2012-01-11 17:30:25 +0000
123+++ test/unit/CMakeLists.txt 2012-01-18 18:13:27 +0000
124@@ -94,6 +94,7 @@
125 xquery_functions.cpp
126 xmldatamanager.cpp
127 staticcollectionmanager.cpp
128+ static_context.cpp
129 )
130
131 IF (NOT ZORBA_NO_FULL_TEXT)
132
133=== added file 'test/unit/static_context.cpp'
134--- test/unit/static_context.cpp 1970-01-01 00:00:00 +0000
135+++ test/unit/static_context.cpp 2012-01-18 18:13:27 +0000
136@@ -0,0 +1,75 @@
137+/*
138+ * Copyright 2006-2012 The FLWOR Foundation.
139+ *
140+ * Licensed under the Apache License, Version 2.0 (the "License");
141+ * you may not use this file except in compliance with the License.
142+ * You may obtain a copy of the License at
143+ *
144+ * http://www.apache.org/licenses/LICENSE-2.0
145+ *
146+ * Unless required by applicable law or agreed to in writing, software
147+ * distributed under the License is distributed on an "AS IS" BASIS,
148+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
149+ * See the License for the specific language governing permissions and
150+ * limitations under the License.
151+ */
152+
153+#include <cassert>
154+#include <iostream>
155+#include <list>
156+#include <map>
157+
158+#include <sstream>
159+#include <zorba/store_manager.h>
160+#include <zorba/zorba.h>
161+#include <zorba/zorba_exception.h>
162+
163+using namespace std;
164+using namespace zorba;
165+using namespace zorba::locale;
166+
167+bool
168+sctx_test_1(Zorba* const zorba)
169+{
170+ StaticContext_t lSctx = zorba->createStaticContext();
171+
172+ Zorba_CompilerHints_t lHints;
173+
174+ std::stringstream lProlog;
175+ lProlog << "declare namespace foo = 'http://www.example.com';";
176+
177+ lSctx->loadProlog(lProlog.str(), lHints);
178+
179+ NsBindings lBindings;
180+ lSctx->getNamespaceBindings(lBindings);
181+
182+ bool lFooFound = false;
183+
184+ for (NsBindings::const_iterator lIter = lBindings.begin();
185+ lIter != lBindings.end(); ++lIter)
186+ {
187+ std::cout << "prefix: " << lIter->first << " bound to "
188+ << lIter->second << std::endl;
189+
190+ if (lIter->first.compare("foo") == 0)
191+ {
192+ lFooFound = true;
193+ }
194+ }
195+
196+ return lFooFound && lBindings.size() == 6;
197+}
198+
199+int static_context( int argc, char *argv[] ) {
200+ void *const zstore = StoreManager::getStore();
201+ Zorba *const zorba = Zorba::getInstance( zstore );
202+
203+ if (!sctx_test_1(zorba))
204+ return 1;
205+
206+ zorba->shutdown();
207+ StoreManager::shutdownStore( zstore );
208+ return 0;
209+}
210+/* vim:set et sw=2 ts=2: */
211+

Subscribers

People subscribed via source and target branches