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

Proposed by Chris Hillery on 2011-12-31
Status: Merged
Approved by: Matthias Brantner on 2012-01-10
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 2011-12-31 Approve on 2012-01-10
Chris Hillery Approve on 2012-01-10
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.
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

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
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?

> 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)

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.

> > 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.

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
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!

Zorba Build Bot (zorba-buildbot) wrote :

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

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
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.

> > 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.

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
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!

Zorba Build Bot (zorba-buildbot) wrote :

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

review: Approve
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