Merge lp:~zorba-coders/zorba/get-namespace-bindings into lp:zorba

Proposed by Chris Hillery
Status: Merged
Approved by: Matthias Brantner
Approved revision: 10604
Merged at revision: 10611
Proposed branch: lp:~zorba-coders/zorba/get-namespace-bindings
Merge into: lp:zorba
Diff against target: 315 lines (+76/-21)
9 files modified
ChangeLog (+1/-0)
doc/cxx/examples/simple.cpp (+17/-10)
include/zorba/item.h (+29/-3)
include/zorba/item_factory.h (+2/-1)
modules/com/zorba-xquery/www/modules/http-client.xq.src/http_response_handler.cpp (+4/-4)
src/api/item.cpp (+20/-0)
src/api/itemfactoryimpl.cpp (+1/-1)
src/api/itemfactoryimpl.h (+1/-1)
src/util/http_util.cpp (+1/-1)
To merge this branch: bzr merge lp:~zorba-coders/zorba/get-namespace-bindings
Reviewer Review Type Date Requested Status
Matthias Brantner Approve
Chris Hillery Approve
Review via email: mp+87187@code.launchpad.net

Commit message

Expose Item::getNamespaceBindings() through the public API.

Description of the change

Exposes Item::getNamespaceBindings() through the public API. This is necessary for the CSX module.

I am not sure whether defining the NsBindings typedef and NsScoping enum on the Item class itself was a good idea or not.

To post a comment you must log in.
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

The attempt to merge lp:~zorba-coders/zorba/get-namespace-bindings into lp:zorba failed. Below is the output from the failed tests.

CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:273 (message):
  Validation queue job get-namespace-bindings-2011-12-31T09-19-06.423Z is
  finished. The final status was:

  1 tests did not succeed - changes not commited.

Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

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

Needs Fixing:
include/zorba/store_consts.h already provides an NsScoping enum.

Comment:
An alternative return value could be a map (which is nothing else than an efficient vector containing pairs).

review: Needs Fixing
Revision history for this message
Chris Hillery (ceejatec) wrote :

I've changed to use zorba::store::StoreConsts::NsScoping.

Regarding vector vs. map, after thinking about it a bit, I agree with you that map<> would be at least a bit better user experience. However, ItemFactory::createElementNode() takes a vector<pair<>> already, and that can't be changed. I think probably it's best that getNamespaceBindings() be consistent with that API.

In fact, probably createElementNode() should be changed to use the same typedef as getNamespaceBindings(). Should I move the typedef for NsBindings into store_consts.h?

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

> Regarding vector vs. map, after thinking about it a bit, I agree with you that
> map<> would be at least a bit better user experience. However,
> ItemFactory::createElementNode() takes a vector<pair<>> already, and that
> can't be changed. I think probably it's best that getNamespaceBindings() be
> consistent with that API.
Fair enough.

>
> In fact, probably createElementNode() should be changed to use the same
> typedef as getNamespaceBindings(). Should I move the typedef for NsBindings
> into store_consts.h?
No, I don't think so. Because the internal NsBindings uses zstring (not zorba::String)

Revision history for this message
Chris Hillery (ceejatec) wrote :

> No, I don't think so. Because the internal NsBindings uses zstring (not
> zorba::String)

I was talking about the public zorba::ItemFactory::createElementNode() method. It takes a std::vector<std::pair<zorba::String, zorba::String> > just like getNamespaceBindings() does.

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

> > No, I don't think so. Because the internal NsBindings uses zstring (not
> > zorba::String)
>
> I was talking about the public zorba::ItemFactory::createElementNode() method.
> It takes a std::vector<std::pair<zorba::String, zorba::String> > just like
> getNamespaceBindings() does.
Oh, sorry. Yes, that would make sense.

Revision history for this message
Chris Hillery (ceejatec) wrote :

Ok, I moved NsBindings to zorba:: scope (in item.h), and changed createElementNode() to also use it (and clients of createElementNode() to use it also).

I've changed the usages of createElementNode() in non-core modules as well, but they compile fine without changes (confirming backwards-compatibility). I'll propose them for merging individually once this change is on the trunk.

review: Approve
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job get-namespace-bindings-2012-01-06T02-19-10.63Z is finished. The final status was:

All tests succeeded!

Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

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

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

One more thing. Could we pass the NsBindings parameter as reference without breaking backwards compatibly?

Also, before merging, could you please mention the API extension in the ChangeLog?

review: Needs Fixing
Revision history for this message
Chris Hillery (ceejatec) wrote :

> One more thing. Could we pass the NsBindings parameter as reference without
> breaking backwards compatibly?

No, at least not for createElementNode(); it would break binary compatibility. If you don't mind that, I could make the change - it should still be source-compatible, at least.

> Also, before merging, could you please mention the API extension in the
> ChangeLog?

Oops, right - yes, I'll do that.

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

> > One more thing. Could we pass the NsBindings parameter as reference without
> > breaking backwards compatibly?
>
> No, at least not for createElementNode(); it would break binary compatibility.
> If you don't mind that, I could make the change - it should still be source-
> compatible, at least.
It seems like it not being a reference is not a good idea for performance reasons.
I think it's ok to break binary compatibility.

Revision history for this message
Chris Hillery (ceejatec) wrote :

I've added a ChangeLog comment.

I did NOT change createElementNode() to accept NsBindings by reference, because it turns out it's not even a source-compatible change. You can no longer do things like this to pass "no namespace bindings":

    theResponse = theFactory->createElementNode(lNullParent, lNodeName,
      theUntypedQName, true, false, NsBindings());

(which is done several times in the http-client module). Instead, you have to declare a local NsBindings instance and pass that by reference.

If we're going to go so far as to break source compatibility too, then I would actually change the API to accept a *pointer* to NsBindings, because passing NULL would be far more reasonable for the "no namespace bindings" case (which seems like it would be a pretty common case). If we don't want to break source compatibility, though, then we should just leave it as-is (in which case this branch can be merged in its current state).

review: Approve
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job get-namespace-bindings-2012-01-10T08-12-08.927Z is finished. The final status was:

All tests succeeded!

Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

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

Revision history for this message
Matthias Brantner (matthias-brantner) :
review: Approve
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job get-namespace-bindings-2012-01-10T16-00-10.868Z 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-05 13:20:30 +0000
3+++ ChangeLog 2012-01-10 08:05:28 +0000
4@@ -19,6 +19,7 @@
5 * Added split function to the string module that allows for streamable tokenization but doesn't have regular expression
6 support.
7 * zerr is not predeclared anymore to be http://www.zorba-xquery.com/errors
8+ * Added API method Item::getNamespaceBindings().
9
10 version 2.1
11
12
13=== modified file 'doc/cxx/examples/simple.cpp'
14--- doc/cxx/examples/simple.cpp 2011-06-14 14:21:49 +0000
15+++ doc/cxx/examples/simple.cpp 2012-01-10 08:05:28 +0000
16@@ -33,14 +33,14 @@
17
18 std::cout << lQuery << std::endl;
19
20- return true;
21+ return true;
22 }
23
24
25 bool
26 example_2(Zorba* aZorba)
27 {
28- XQuery_t lQuery = aZorba->compileQuery("1+2");
29+ XQuery_t lQuery = aZorba->compileQuery("1+2");
30
31 Iterator_t lIterator = lQuery->iterator();
32 lIterator->open();
33@@ -53,7 +53,7 @@
34
35 lIterator->close();
36
37- return true;
38+ return true;
39 }
40
41
42@@ -61,7 +61,7 @@
43 example_3(Zorba* aZorba)
44 {
45
46- XQuery_t lQuery = aZorba->compileQuery("1 div 0");
47+ XQuery_t lQuery = aZorba->compileQuery("1 div 0");
48 try {
49 std::cout << lQuery << std::endl;
50 } catch ( ZorbaException& e ) {
51@@ -69,7 +69,7 @@
52 return true;
53 }
54
55- return false;
56+ return false;
57 }
58
59
60@@ -125,9 +125,7 @@
61 bool
62 example_7()
63 {
64-
65 std::cout << Zorba::version() << std::endl;
66-
67 return true;
68 }
69
70@@ -246,7 +244,8 @@
71 bool
72 example_12(Zorba* aZorba)
73 {
74- XQuery_t lQuery = aZorba->compileQuery("<a><b attr='1'/><b attr='2'/></a>");
75+ XQuery_t lQuery = aZorba->compileQuery
76+ ("<a xmlns:foo='http://www.zorba-xquery.com/'><b attr='1' xmlns:bar='http://www.zorba-xquery.com/uri2'/><b attr='2'/></a>");
77
78 Iterator_t lIterator = lQuery->iterator();
79 lIterator->open();
80@@ -263,16 +262,24 @@
81 Item lNodeName;
82 lChild.getNodeName(lNodeName);
83 std::cout << "node name " << lNodeName.getStringValue() << std::endl;
84+
85 Iterator_t lAttrIter = lChild.getAttributes();
86-
87 lAttrIter->open();
88-
89 Item lAttr;
90 while (lAttrIter->next(lAttr))
91 {
92 std::cout << " attribute value " << lAttr.getStringValue() << std::endl;
93 }
94 lAttrIter->close();
95+
96+ NsBindings lBindings;
97+ lChild.getNamespaceBindings(lBindings,
98+ store::StoreConsts::ONLY_LOCAL_NAMESPACES);
99+ for (NsBindings::const_iterator ite = lBindings.begin();
100+ ite != lBindings.end(); ++ite) {
101+ std::cout << " namespace binding " << ite->first
102+ << "->" << ite->second << std::endl;
103+ }
104 }
105 lChildIter->close();
106 }
107
108=== modified file 'include/zorba/item.h'
109--- include/zorba/item.h 2011-06-14 17:26:33 +0000
110+++ include/zorba/item.h 2012-01-10 08:05:28 +0000
111@@ -19,6 +19,8 @@
112 #include <iostream>
113 #include <zorba/config.h>
114 #include <zorba/api_shared_types.h>
115+#include <zorba/store_consts.h>
116+#include <vector>
117
118 namespace zorba {
119
120@@ -31,6 +33,11 @@
121 void operator&(zorba::serialization::Archiver &ar, zorba::Item &obj);
122 }
123
124+/**
125+ * Used for Item::getNamespaceBindings() and ItemFactory::createElementNode().
126+ */
127+typedef std::vector<std::pair<String, String> > NsBindings;
128+
129 /** \brief The Zorba Item interface.
130 *
131 * This class is the Zorba representation of an Item as defined in the
132@@ -266,7 +273,7 @@
133 /** \brief Get an iterator for the children of this (node) Item.
134 *
135 * Note that this function is only available for node Items.
136- * The file \link simple.cpp \endlink contains some basic examples the demonstrate
137+ * The file \link simple.cpp \endlink contains some basic examples that demonstrate
138 * the use of this function.
139 *
140 * @return Iterator over the children of this node.
141@@ -278,7 +285,7 @@
142 /** \brief Get an iterator for the attributes of this (node) Item.
143 *
144 * Note that this function is only available for node Items.
145- * The file \link simple.cpp \endlink contains some basic examples the demonstrate
146+ * The file \link simple.cpp \endlink contains some basic examples that demonstrate
147 * the use of this function.
148 *
149 * @return Iterator over the attributes of this node.
150@@ -287,6 +294,25 @@
151 Iterator_t
152 getAttributes() const;
153
154+ /** \brief Get an iterator for the namespace bindings of this (element) Item.
155+ *
156+ * Note that this function is only available for element Items.
157+ * The file \link simple.cpp \endlink contains some basic examples that demonstrate
158+ * the use of this function.
159+ *
160+ * @param aBindings An STL list to receive the namespace bindings of this node (each
161+ * represented as a std::pair<zorba::String,zorba::String> where the
162+ * first string is the namespace prefix and the second is the namespace URI).
163+ * @param aScope An instance of NsScoping to declare which bindings to return:
164+ * those local to the element; those local to all parent elements; or all bindings
165+ * (the default).
166+ * @throw ZorbaException if an error occured, e.g. the Item is not of type element.
167+ */
168+ void
169+ getNamespaceBindings(NsBindings& aBindings,
170+ store::StoreConsts::NsScoping aNsScoping = store::StoreConsts::ALL_NAMESPACES)
171+ const;
172+
173 /** \brief Get parent of this (node) Item.
174 *
175 * Note that this function is only available for node Items.
176@@ -300,7 +326,7 @@
177 /** \brief Get the name of this (node) Item.
178 *
179 * Note that this function is only available for node Items.
180- * The file \link simple.cpp \endlink contains some basic examples the demonstrate
181+ * The file \link simple.cpp \endlink contains some basic examples that demonstrate
182 * the use of this function.
183 *
184 * @return bool if the name of the node was retrieved successfully
185
186=== modified file 'include/zorba/item_factory.h'
187--- include/zorba/item_factory.h 2011-12-21 14:40:33 +0000
188+++ include/zorba/item_factory.h 2012-01-10 08:05:28 +0000
189@@ -22,6 +22,7 @@
190
191 #include <zorba/config.h>
192 #include <zorba/api_shared_types.h>
193+#include <zorba/item.h>
194 #include <zorba/streams.h>
195
196 namespace zorba {
197@@ -607,7 +608,7 @@
198 Item aTypeName,
199 bool aHasTypedValue,
200 bool aHasEmptyValue,
201- std::vector<std::pair<String, String> > aNsBindings) = 0;
202+ NsBindings aNsBindings) = 0;
203
204 /**
205 * Create a new attribute node N and place it among the
206
207=== modified file 'modules/com/zorba-xquery/www/modules/http-client.xq.src/http_response_handler.cpp'
208--- modules/com/zorba-xquery/www/modules/http-client.xq.src/http_response_handler.cpp 2011-07-29 08:12:36 +0000
209+++ modules/com/zorba-xquery/www/modules/http-client.xq.src/http_response_handler.cpp 2012-01-10 08:05:28 +0000
210@@ -126,7 +126,7 @@
211 String lLocalName = "response";
212 Item lNodeName = theFactory->createQName(theNamespace, lLocalName);
213 theResponse = theFactory->createElementNode(lNullParent, lNodeName,
214- theUntypedQName, true, false, std::vector<std::pair<String, String> >());
215+ theUntypedQName, true, false, NsBindings());
216 theFactory->createAttributeNode(theResponse,
217 theFactory->createQName("", "status"), lNullType,
218 theFactory->createInteger(aStatus));
219@@ -165,7 +165,7 @@
220 Item lNullType;
221 Item lElem = theFactory->createElementNode(lParent,
222 theFactory->createQName(theNamespace, "header"), theUntypedQName, true,
223- true, std::vector<std::pair<String, String> >());
224+ true, NsBindings());
225 theFactory->createAttributeNode(lElem, theFactory->createQName("", "name"),
226 lNullType, theFactory->createString(aName));
227 theFactory->createAttributeNode(lElem, theFactory->createQName("", "value"),
228@@ -180,7 +180,7 @@
229 Item lNullType;
230 Item lElem = theFactory->createElementNode(lParent,
231 theFactory->createQName(theNamespace, "body"), theUntypedQName, true,
232- true, std::vector<std::pair<String, String> >());
233+ true, NsBindings());
234 theFactory->createAttributeNode(lElem,
235 theFactory->createQName("", "media-type"),
236 lNullType, theFactory->createString(aContentType));
237@@ -201,7 +201,7 @@
238 Item lNullType;
239 Item lElem = theFactory->createElementNode(theResponse,
240 theFactory->createQName(theNamespace, "body"), theUntypedQName, true,
241- true, std::vector<std::pair<String, String> >());
242+ true, NsBindings());
243 theFactory->createAttributeNode(lElem,
244 theFactory->createQName("", "content-type"),
245 lNullType, theFactory->createString(aContentType));
246
247=== modified file 'src/api/item.cpp'
248--- src/api/item.cpp 2011-06-14 17:26:33 +0000
249+++ src/api/item.cpp 2012-01-10 08:05:28 +0000
250@@ -380,6 +380,26 @@
251 return NULL;
252 }
253
254+void
255+Item::getNamespaceBindings(NsBindings& aBindings,
256+ store::StoreConsts::NsScoping aNsScoping) const
257+{
258+ ITEM_TRY
259+ SYNC_CODE(AutoLock lock(GENV_STORE.getGlobalLock(), Lock::READ);)
260+
261+ store::NsBindings lStoreBindings;
262+ m_item->getNamespaceBindings(lStoreBindings, aNsScoping);
263+ store::NsBindings::iterator ite = lStoreBindings.begin();
264+ store::NsBindings::iterator end = lStoreBindings.end();
265+ for (; ite != end; ++ite) {
266+ zstring& prefix = ite->first;
267+ zstring& nsuri = ite->second;
268+ aBindings.push_back(std::pair<String, String>(prefix.str(), nsuri.str()));
269+ }
270+
271+ ITEM_CATCH
272+}
273+
274 Item
275 Item::getParent() const
276 {
277
278=== modified file 'src/api/itemfactoryimpl.cpp'
279--- src/api/itemfactoryimpl.cpp 2011-12-21 14:40:33 +0000
280+++ src/api/itemfactoryimpl.cpp 2012-01-10 08:05:28 +0000
281@@ -696,7 +696,7 @@
282 Item aTypeName,
283 bool aHasTypedValue,
284 bool aHasEmptyValue,
285- std::vector<std::pair<String, String> > aNsBindings)
286+ NsBindings aNsBindings)
287 {
288 store::Item_t lItem;
289 store::Item_t lNodeName = Unmarshaller::getInternalItem(aNodeName);
290
291=== modified file 'src/api/itemfactoryimpl.h'
292--- src/api/itemfactoryimpl.h 2011-12-21 14:40:33 +0000
293+++ src/api/itemfactoryimpl.h 2012-01-10 08:05:28 +0000
294@@ -213,7 +213,7 @@
295 Item aTypeName,
296 bool aHasTypedValue,
297 bool aHasEmptyValue,
298- std::vector<std::pair<String, String> > aNsBindings);
299+ NsBindings aNsBindings);
300
301 virtual Item
302 createAttributeNode(Item aParent,
303
304=== modified file 'src/util/http_util.cpp'
305--- src/util/http_util.cpp 2011-12-21 14:40:33 +0000
306+++ src/util/http_util.cpp 2012-01-10 08:05:28 +0000
307@@ -47,7 +47,7 @@
308 ItemFactory* lFactory = lInstance->getItemFactory();
309 Item lNodeName = lFactory->createQName("http://expath.org/ns/http-client", "http", "request");
310 Item lEmptyItem;
311- std::vector<std::pair<String, String> > nsPairs;
312+ NsBindings nsPairs;
313 nsPairs.push_back(std::make_pair(String("xs"), String("http://www.w3.org/2001/XMLSchema")));
314 Item lRequestElement = lFactory->createElementNode(lEmptyItem, lNodeName,
315 lFactory->createQName("http://www.w3.org/2001/XMLSchema",

Subscribers

People subscribed via source and target branches