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

Proposed by Matthias Brantner
Status: Superseded
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
Markos Zaharioudakis Approve
Rodolfo Ochoa Approve
Hybridum Pending
Review via email: mp+88922@code.launchpad.net

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

This proposal has been superseded by a proposal from 2012-01-18.

Commit message

Implemented StaticContext::getNamespaceBindings to resolve bug #905035.

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.
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal
Revision history for this message
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!

Revision history for this message
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.

Revision history for this message
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.

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

How hard is to get a zorba iterator instead?

Revision history for this message
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).

Revision history for this message
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.

Revision history for this message
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
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal
Revision history for this message
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!

Revision history for this message
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.

Revision history for this message
Hybridum (hybridum) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
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?

Revision history for this message
Matthias Brantner (matthias-brantner) wrote :

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.

Revision history for this message
Rodolfo Ochoa (rodolfo-ochoa) wrote :

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

review: Approve
Revision history for this message
Markos Zaharioudakis (markos-za) wrote :

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.

Revision history for this message
Markos Zaharioudakis (markos-za) :
review: Approve
lp:~zorba-coders/zorba/bug905035 updated
10619. By Matthias Brantner

more efficient implementation of the getNamespaceBindings functions

Unmerged revisions

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:25 +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:25 +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:25 +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:25 +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:25 +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:25 +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:25 +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