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

[Matthias Brantner]

Implemented StaticContext::getNamespaceBindings to resolve bug #905035.

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

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug905035-2012-01-11T20-48-04.305Z/log.html

[Zorba Build Bot (zorba-buildbot)]

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

All tests succeeded!

[Zorba Build Bot (zorba-buildbot)]

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

[Chris Hillery (ceejatec)]

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

How hard is to get a zorba iterator instead?

[Chris Hillery (ceejatec)]

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

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

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

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/bug905035-2012-01-12T22-04-03.62Z/log.html

[Zorba Build Bot (zorba-buildbot)]

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

All tests succeeded!

[Zorba Build Bot (zorba-buildbot)]

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

[Hybridum (hybridum)]

review: Approve

[Markos Zaharioudakis (markos-za)]

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

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

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

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

review: Approve

[lp:~zorba-coders/zorba/bug905035]

10619. By Matthias Brantner on 2012-01-18

more efficient implementation of the getNamespaceBindings functions

Unmerged revisions

=== modified file 'ChangeLog'
--- ChangeLog	2012-01-18 12:18:59 +0000
+++ ChangeLog	2012-01-18 18:13:25 +0000
@@ -22,6 +22,8 @@
     support.
   * zerr is not predeclared anymore to be http://www.zorba-xquery.com/errors
   * Added API method Item::getNamespaceBindings().
+  * Added API method StaticContext::getNamespaceBindings() (see bug #905035)
+  * Deprecated StaticContext:getNamespaceURIByPrefix()
 
 version 2.1
 
 
=== modified file 'include/zorba/static_context.h'
--- include/zorba/static_context.h	2011-12-21 14:40:33 +0000
+++ include/zorba/static_context.h	2012-01-18 18:13:25 +0000
@@ -99,10 +99,21 @@
    *         could be found for the given prefix and an DiagnosticHandler has been
    *         registered.
    * @throw ZorbaException if an error occured (e.g. no URI could be found for the given prefix).
+   *
+   * @deprecated This function is deprecated. Use getNamespaceBindings instead.
    */
   virtual String
   getNamespaceURIByPrefix( const String& aPrefix ) const = 0;
 
+  /**
+   * \brief Get the list of all namespace bindings (prefix, uri)
+   *   declared in this and its parent static contexts.
+   *
+   * @param aBindings the bindings are added to this list
+   */
+  virtual void
+  getNamespaceBindings( NsBindings& aBindings ) const = 0;
+
   /** \brief Set the default element and type namespace
    *         (see http://www.w3.org/TR/xquery/#static_context)
    *
 
=== modified file 'src/api/item.cpp'
--- src/api/item.cpp	2012-01-11 17:30:25 +0000
+++ src/api/item.cpp	2012-01-18 18:13:25 +0000
@@ -389,12 +389,18 @@
 
       store::NsBindings lStoreBindings;
       m_item->getNamespaceBindings(lStoreBindings, aNsScoping);
+      aBindings.reserve(aBindings.size() + lStoreBindings.size());
+
       store::NsBindings::iterator ite = lStoreBindings.begin();
       store::NsBindings::iterator end = lStoreBindings.end();
       for (; ite != end; ++ite) {
         zstring& prefix = ite->first;
         zstring& nsuri = ite->second;
-        aBindings.push_back(std::pair<String, String>(prefix.str(), nsuri.str()));
+        aBindings.push_back(
+            std::pair<String, String>(
+              Unmarshaller::newString(prefix),
+              Unmarshaller::newString(nsuri))
+          );
       }
 
   ITEM_CATCH
 
=== modified file 'src/api/staticcontextimpl.cpp'
--- src/api/staticcontextimpl.cpp	2012-01-11 17:30:25 +0000
+++ src/api/staticcontextimpl.cpp	2012-01-18 18:13:25 +0000
@@ -215,6 +215,38 @@
   return "";
 }
 
+/*******************************************************************************
+
+********************************************************************************/
+void
+StaticContextImpl::getNamespaceBindings( NsBindings& aBindings ) const
+{
+  try
+  {
+    store::NsBindings lBindings;
+    theCtx->get_namespace_bindings(lBindings);
+    aBindings.reserve(aBindings.size() + lBindings.size());
+
+    for (store::NsBindings::const_iterator lIter = lBindings.begin();
+         lIter != lBindings.end(); ++lIter)
+    {
+      aBindings.push_back(
+        std::pair<zorba::String, zorba::String>(
+          Unmarshaller::newString(lIter->first),
+          Unmarshaller::newString(lIter->second)
+        )
+      );
+    }
+  }
+  catch (ZorbaException const& e)
+  {
+    ZorbaImpl::notifyError(theDiagnosticHandler, e);
+  }
+  catch (std::exception const& e)
+  {
+    ZorbaImpl::notifyError(theDiagnosticHandler, e.what());
+  }
+}
 
 /*******************************************************************************
 
 
=== modified file 'src/api/staticcontextimpl.h'
--- src/api/staticcontextimpl.h	2011-12-21 14:40:33 +0000
+++ src/api/staticcontextimpl.h	2012-01-18 18:13:25 +0000
@@ -78,6 +78,9 @@
 
   String getNamespaceURIByPrefix( const String& prefix ) const;
 
+  void
+  getNamespaceBindings( NsBindings& aBindings ) const;
+
   bool setDefaultElementAndTypeNamespace( const String& URI );
 
   String getDefaultElementAndTypeNamespace( ) const;
 
=== modified file 'test/unit/CMakeLists.txt'
--- test/unit/CMakeLists.txt	2012-01-11 17:30:25 +0000
+++ test/unit/CMakeLists.txt	2012-01-18 18:13:25 +0000
@@ -94,6 +94,7 @@
   xquery_functions.cpp
   xmldatamanager.cpp
   staticcollectionmanager.cpp
+  static_context.cpp
 )
 
 IF (NOT ZORBA_NO_FULL_TEXT)
 
=== added file 'test/unit/static_context.cpp'
--- test/unit/static_context.cpp	1970-01-01 00:00:00 +0000
+++ test/unit/static_context.cpp	2012-01-18 18:13:25 +0000
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2006-2012 The FLWOR Foundation.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <cassert>
+#include <iostream>
+#include <list>
+#include <map>
+
+#include <sstream>
+#include <zorba/store_manager.h>
+#include <zorba/zorba.h>
+#include <zorba/zorba_exception.h>
+
+using namespace std;
+using namespace zorba;
+using namespace zorba::locale;
+
+bool
+sctx_test_1(Zorba* const zorba)
+{
+  StaticContext_t lSctx = zorba->createStaticContext();
+
+  Zorba_CompilerHints_t lHints;
+
+  std::stringstream lProlog;
+  lProlog << "declare namespace foo = 'http://www.example.com';";
+
+  lSctx->loadProlog(lProlog.str(), lHints);
+
+  NsBindings lBindings;
+  lSctx->getNamespaceBindings(lBindings);
+
+  bool lFooFound = false;
+
+  for (NsBindings::const_iterator lIter = lBindings.begin();
+       lIter != lBindings.end(); ++lIter)
+  {
+    std::cout << "prefix: " << lIter->first << " bound to "
+      << lIter->second << std::endl;
+
+    if (lIter->first.compare("foo") == 0)
+    {
+      lFooFound = true;
+    }
+  }
+
+  return lFooFound && lBindings.size() == 6;
+}
+
+int static_context( int argc, char *argv[] ) {
+  void *const zstore = StoreManager::getStore();
+  Zorba *const zorba = Zorba::getInstance( zstore );
+
+  if (!sctx_test_1(zorba))
+    return 1;
+
+  zorba->shutdown();
+  StoreManager::shutdownStore( zstore );
+  return 0;
+}
+/* vim:set et sw=2 ts=2: */
+