ImplicitContext updates

7 files changed, 129 insertions, 224 deletions

diff --git a/cpp/CHANGES b/cpp/CHANGES
index 6ffc34420b1..08b0761a5ca 100644
--- a/cpp/CHANGES
+++ b/cpp/CHANGES
@@ -180,16 +180,13 @@ Changes since version 3.1.1
   context, Ice uses the "implicit context" combined with the context
   associated with the proxy (if there is one)
   You can retrieve and set this ImplicitContext using 
-  Communicator::getImplicitContext(). Four ImplicitContext implementations
+  Communicator::getImplicitContext(). Three ImplicitContext implementations
   are available. You select an implementation by setting the 
   Ice.ImplicitContext property to one of the following values:
     * None: (the default) No implicit context at all
     * PerThread: The implementation maintains a Context per thread
     * Shared: The implementation maintains a single Context shared 
-              by all threads, and serializes access to this Context
-    * SharedWithoutLocking: The implementation maintains a single 
-              Context shared by all threads, and does not serialize 
-              access to this Context
+              by all threads, and serializes access to this Context.
 
 - Removed defaultContext from InitializationData
 
diff --git a/cpp/slice/Ice/ImplicitContext.ice b/cpp/slice/Ice/ImplicitContext.ice
index de923f99048..11dd229548d 100644
--- a/cpp/slice/Ice/ImplicitContext.ice
+++ b/cpp/slice/Ice/ImplicitContext.ice
@@ -32,24 +32,21 @@ module Ice
  * <dd>The implementation maintains a [Context] per thread.</dd>
  * <dt><tt>Shared</tt></dt>
  * <dd>The implementation maintains a single [Context] shared 
- * by all threads, and serializes access to this [Context].</dd>
- * <dt><tt>SharedWithoutLocking</tt></dt>
- * <dd>
- * The implementation maintains a single 
- * [Context] shared by all threads, and does not serialize access to this [Context].
- * </dd>
+ * by all threads.</dd>
  * </dl><p>
- *
+ *  
+ * <tt>ImplicitContext</tt> also provides a number of operations to create, update or retrieve
+ * an entry in the underlying context without first retrieving a copy of the entire
+ * context. These operations correspond to a subset of the <tt>java.util.Map</tt> methods,
+ * with <tt>java.lang.Object</tt> replaced by <tt>string</tt> and null replaced by the empty-string.
+ * 
  **/
 
 local interface ImplicitContext
 {
     /**
-     * Get the underlying context. The operation returns a null proxy if no implicit
-     * context is established on the communicator (that is, if <tt>Ice.ImplicitContext</tt>
-     * is set to <tt>None</tt>).
-     *
-     * @return The underlying context.
+     * Get a copy of the underlying context.
+     * @return A copy of the underlying context.
      *
      **/
     ["cpp:const"] Context getContext();
@@ -63,47 +60,49 @@ local interface ImplicitContext
     void setContext(Context newContext);
 
     /**
-     * Get the value associated with the given key in the underlying context.
-     * Throws [NotSetException] when no value is associated with the given key.
+     * Check if this key has an associated value in the underlying context. 
      *
      * @param key The key.
      *
-     * @return The value associated with the key.
+     * @return True if the key has an associated value, False otherwise.
      *
      **/
-    ["cpp:const"] string get(string key);
-
+    ["cpp:const"] bool containsKey(string key);
+ 
     /**
      * Get the value associated with the given key in the underlying context.
+     * Returns an empty string if no value is associated with the key.
+     * [containsKey] allows you to distinguish between an empty-string value and 
+     * no value at all.
      *
      * @param key The key.
      *
-     * @param defaultValue The default value
-     *
-     * @return The value associated with the key, or defaultValue when no
-     * value is associated with the given key.
+     * @return The value associated with the key.
      *
      **/
-    ["cpp:const"] string getWithDefault(string key, string defaultValue);
+    ["cpp:const"] string get(string key);
 
     /**
-     * Set the value associated with the given key in the underlying context.
+     * Create or update a key/value entry in the underlying context.
      *
      * @param key The key.
      *
      * @param value The value.
      *
+     * @return The previous value associated with the key, if any.
+     *
      **/
-    void set(string key, string value);
+    string put(string key, string value);
 
     /**
-     * Remove the value associated with the given key in the underlying context.
-     * Throws [NotSetException] when no value is associated with the given key.
+     * Remove the entry for the given key in the underlying context.
      *
      * @param key The key.
      *
+     * @return The value associated with the key, if any.
+     *
      **/
-    void remove(string key);
+    string remove(string key);
 };
 };
 
diff --git a/cpp/slice/Ice/LocalException.ice b/cpp/slice/Ice/LocalException.ice
index e1ac62e5248..9d05cddb817 100644
--- a/cpp/slice/Ice/LocalException.ice
+++ b/cpp/slice/Ice/LocalException.ice
@@ -892,23 +892,6 @@ local exception FeatureNotSupportedException
 
 /**
  *
- * This exception is raised when attempting to read a context
- * entry that is not set.
- *
- **/
-local exception NotSetException
-{
-    /**
-     *
-     * The name of the key that is not set.
-     *
-     **/
-    string key;
-};
-
-
-/**
- *
  * This exception indicates a failure in a security subsystem,
  * such as the IceSSL plugin.
  *
diff --git a/cpp/src/Ice/Exception.cpp b/cpp/src/Ice/Exception.cpp
index 659875a3725..9c79f13c947 100644
--- a/cpp/src/Ice/Exception.cpp
+++ b/cpp/src/Ice/Exception.cpp
@@ -650,14 +650,6 @@ Ice::FeatureNotSupportedException::ice_print(ostream& out) const
 }
 
 void
-Ice::NotSetException::ice_print(ostream& out) const
-{
-    Exception::ice_print(out);
-    out << ":\nkey '" << key << "' is not set";
-}
-
-
-void
 Ice::SecurityException::ice_print(ostream& out) const
 {
     Exception::ice_print(out);
diff --git a/cpp/src/Ice/ImplicitContextI.cpp b/cpp/src/Ice/ImplicitContextI.cpp
index a2991b12e14..1a398c269d4 100644
--- a/cpp/src/Ice/ImplicitContextI.cpp
+++ b/cpp/src/Ice/ImplicitContextI.cpp
@@ -16,43 +16,23 @@ using namespace Ice;
 namespace
 {
 
-class SharedImplicitContextWithoutLocking : public ImplicitContextI
+class SharedImplicitContext : public ImplicitContextI
 {
 public:
     
     virtual Context getContext() const;
     virtual void setContext(const Context&);
 
+    virtual bool containsKey(const string&) const;
     virtual string get(const string&) const;
-    virtual string getWithDefault(const string&, const string&) const;
-    virtual void set(const string&, const string&);
-    virtual void remove(const string&);
-
-    virtual void write(const Context&, ::IceInternal::BasicStream*) const;
-    virtual void combine(const Context&, Context&) const;
-
-protected:
-
-    Context _context;
-};
-
-class SharedImplicitContext : public SharedImplicitContextWithoutLocking
-{
-public:
-    
-    virtual Context getContext() const;
-    virtual void setContext(const Context&);
-
-    virtual string get(const string&) const;
-    virtual string getWithDefault(const string&, const string&) const;
-    virtual void set(const string&, const string&);
-    virtual void remove(const string&);
+    virtual string put(const string&, const string&);
+    virtual string remove(const string&);
 
     virtual void write(const Context&, ::IceInternal::BasicStream*) const;
     virtual void combine(const Context&, Context&) const;
 
 private:
-
+    Context _context;
     IceUtil::Mutex _mutex;
 };
 
@@ -67,10 +47,10 @@ public:
     virtual Context getContext() const;
     virtual void setContext(const Context&);
 
+    virtual bool containsKey(const string&) const;
     virtual string get(const string&) const;
-    virtual string getWithDefault(const string&, const string&) const;
-    virtual void set(const string&, const string&);
-    virtual void remove(const string&);
+    virtual string put(const string&, const string&);
+    virtual string remove(const string&);
 
     virtual void write(const Context&, ::IceInternal::BasicStream*) const;
     virtual void combine(const Context&, Context&) const;
@@ -135,10 +115,6 @@ ImplicitContextI::create(const std::string& kind)
     {
         return new SharedImplicitContext;
     }
-    else if(kind == "SharedWithoutLocking")
-    {
-        return new SharedImplicitContextWithoutLocking;
-    }
     else if(kind == "PerThread")
     {
         return new PerThreadImplicitContext;
@@ -164,81 +140,100 @@ ImplicitContextI::cleanupThread()
 }
 #endif
 
+
 //
-// SharedImplicitContextWithoutLocking implementation
+// SharedImplicitContext implementation
 //
 
-inline Context
-SharedImplicitContextWithoutLocking::getContext() const
+Context
+SharedImplicitContext::getContext() const
 {
+    IceUtil::Mutex::Lock lock(_mutex);
     return _context;
 }
 
-inline void
-SharedImplicitContextWithoutLocking::setContext(const Context& newContext)
+void
+SharedImplicitContext::setContext(const Context& newContext)
 {
+    IceUtil::Mutex::Lock lock(_mutex);
     _context = newContext;
 }
 
-inline string 
-SharedImplicitContextWithoutLocking::get(const string& k) const
+bool 
+SharedImplicitContext::containsKey(const string& k) const
 {
+    IceUtil::Mutex::Lock lock(_mutex);
     Context::const_iterator p = _context.find(k);
-    if(p == _context.end())
-    {
-        throw NotSetException(__FILE__, __LINE__, k);
-    }
-    return p->second;
+    return p != _context.end();
 }
 
-inline string 
-SharedImplicitContextWithoutLocking::getWithDefault(const string& k, const string& d) const
+string 
+SharedImplicitContext::get(const string& k) const
 {
+    IceUtil::Mutex::Lock lock(_mutex);
     Context::const_iterator p = _context.find(k);
     if(p == _context.end())
     {
-        return d;
+        return "";
     }
     return p->second;
 }
 
-inline void 
-SharedImplicitContextWithoutLocking::set(const string& k, const string& v)
+
+string 
+SharedImplicitContext::put(const string& k, const string& v)
 {
-    _context[k] = v;
+    IceUtil::Mutex::Lock lock(_mutex);
+    string& val = _context[k];
+    
+    string oldVal = val;
+    val = v;
+    return oldVal;
 }
 
-inline void 
-SharedImplicitContextWithoutLocking::remove(const string& k)
+string
+SharedImplicitContext::remove(const string& k)
 {
-    if(_context.erase(k) == 0)
+    IceUtil::Mutex::Lock lock(_mutex);
+    Context::iterator p = _context.find(k);
+    if(p == _context.end())
     {
-        throw NotSetException(__FILE__, __LINE__, k);
+        return "";
+    }
+    else
+    {
+        string oldVal = p->second;
+        _context.erase(p);
+        return oldVal;
     }
 }
 
 void 
-SharedImplicitContextWithoutLocking::write(const Context& proxyCtx, ::IceInternal::BasicStream* s) const
+SharedImplicitContext::write(const Context& proxyCtx, ::IceInternal::BasicStream* s) const
 {
+    IceUtil::Mutex::Lock lock(_mutex);
     if(proxyCtx.size() == 0)
     {
         __write(s, _context, __U__Context());
     }
     else if(_context.size() == 0)
     {
+        lock.release();
         __write(s, proxyCtx, __U__Context());
     }
     else
     {
         Context combined = proxyCtx;
         combined.insert(_context.begin(), _context.end());
+        lock.release();
         __write(s, combined, __U__Context());
     }
 }
 
 void 
-SharedImplicitContextWithoutLocking::combine(const Context& proxyCtx, Context& ctx) const
+SharedImplicitContext::combine(const Context& proxyCtx, Context& ctx) const
 {
+    IceUtil::Mutex::Lock lock(_mutex);
     if(proxyCtx.size() == 0)
     {
         ctx = _context;
@@ -252,82 +247,6 @@ SharedImplicitContextWithoutLocking::combine(const Context& proxyCtx, Context& c
         ctx = proxyCtx;
         ctx.insert(_context.begin(), _context.end());
     }
-
-}
-
-//
-// SharedImplicitContext implementation
-//
-
-Context
-SharedImplicitContext::getContext() const
-{
-    IceUtil::Mutex::Lock lock(_mutex);
-    return SharedImplicitContextWithoutLocking::getContext();
-}
-
-void
-SharedImplicitContext::setContext(const Context& newContext)
-{
-    IceUtil::Mutex::Lock lock(_mutex);
-    SharedImplicitContextWithoutLocking::setContext(newContext);
-}
-
-string 
-SharedImplicitContext::get(const string& k) const
-{
-    IceUtil::Mutex::Lock lock(_mutex);
-    return SharedImplicitContextWithoutLocking::get(k);
-}
-
-string 
-SharedImplicitContext::getWithDefault(const string& k, const string& d) const
-{
-    IceUtil::Mutex::Lock lock(_mutex);
-    return SharedImplicitContextWithoutLocking::getWithDefault(k, d);
-}
-
-void 
-SharedImplicitContext::set(const string& k, const string& v)
-{
-    IceUtil::Mutex::Lock lock(_mutex);
-    SharedImplicitContextWithoutLocking::set(k, v);
-}
-
-void 
-SharedImplicitContext::remove(const string& k)
-{
-    IceUtil::Mutex::Lock lock(_mutex);
-    SharedImplicitContextWithoutLocking::remove(k);
-}
-
-void 
-SharedImplicitContext::write(const Context& proxyCtx, ::IceInternal::BasicStream* s) const
-{
-    IceUtil::Mutex::Lock lock(_mutex);
-    if(proxyCtx.size() == 0)
-    {
-        __write(s, _context, __U__Context());
-    }
-    else if(_context.size() == 0)
-    {
-        lock.release();
-        __write(s, proxyCtx, __U__Context());
-    }
-    else
-    {
-        Context combined = proxyCtx;
-        combined.insert(_context.begin(), _context.end());
-        lock.release();
-        __write(s, combined, __U__Context());
-    }
-}
-
-void 
-SharedImplicitContext::combine(const Context& proxyCtx, Context& ctx) const
-{
-    IceUtil::Mutex::Lock lock(_mutex);
-    SharedImplicitContextWithoutLocking::combine(proxyCtx, ctx);
 }
 
 //
@@ -579,64 +498,77 @@ PerThreadImplicitContext::setContext(const Context& newContext)
     }
 }
 
-string 
-PerThreadImplicitContext::get(const string& k) const
+bool 
+PerThreadImplicitContext::containsKey(const string& k) const
 {
-    Context* ctx = getThreadContext(false);
+    const Context* ctx = getThreadContext(false);
     if(ctx == 0)
     {
-        throw NotSetException(__FILE__, __LINE__, k);
+        return false;
     }
     Context::const_iterator p = ctx->find(k);
-    if(p == ctx->end())
-    {
-        throw NotSetException(__FILE__, __LINE__, k);
-    }
-    return p->second;
+    return p != ctx->end();
 }
 
 string 
-PerThreadImplicitContext::getWithDefault(const string& k, const string& d) const
+PerThreadImplicitContext::get(const string& k) const
 {
-    Context* ctx = getThreadContext(false);
+    const Context* ctx = getThreadContext(false);
     if(ctx == 0)
     {
-        return d;
+        return "";
     }
     Context::const_iterator p = ctx->find(k);
     if(p == ctx->end())
     {
-        return d;
+        return "";
     }
     return p->second;
 }
 
-void 
-PerThreadImplicitContext::set(const string& k, const string& v)
+string 
+PerThreadImplicitContext::put(const string& k, const string& v)
 {
-     Context* ctx = getThreadContext(true);
-     (*ctx)[k] = v;
+    Context* ctx = getThreadContext(true);
+
+    string& val = (*ctx)[k];
+    
+    string oldVal = val;
+    val = v;
+    return oldVal;
 }
 
-void 
+string
 PerThreadImplicitContext::remove(const string& k)
 {
      Context* ctx = getThreadContext(false);
-     if(ctx == 0 || ctx->erase(k) == 0)
+     if(ctx == 0)
      {
-         throw NotSetException(__FILE__, __LINE__, k);
+         return "";
      }
      
-     if(ctx->size() == 0)
+     Context::iterator p = ctx->find(k);
+     if(p == ctx->end())
      {
-         clearThreadContext();
+         return "";
      }
+     else
+     {
+         string oldVal = p->second;
+         ctx->erase(p);
+
+         if(ctx->size() == 0)
+         {
+             clearThreadContext();
+         }
+         return oldVal;
+    }
 }
 
 void 
 PerThreadImplicitContext::write(const Context& proxyCtx, ::IceInternal::BasicStream* s) const
 {
-    Context* threadCtx = getThreadContext(false);
+    const Context* threadCtx = getThreadContext(false);
 
     if(threadCtx == 0 || threadCtx->size() == 0)
     {
@@ -657,7 +589,7 @@ PerThreadImplicitContext::write(const Context& proxyCtx, ::IceInternal::BasicStr
 void 
 PerThreadImplicitContext::combine(const Context& proxyCtx, Context& ctx) const
 {
-    Context* threadCtx = getThreadContext(false);
+    const Context* threadCtx = getThreadContext(false);
 
     if(threadCtx == 0 || threadCtx->size() == 0)
     {
diff --git a/cpp/test/Ice/operations/Twoways.cpp b/cpp/test/Ice/operations/Twoways.cpp
index 27f7133b622..055098a2a5b 100644
--- a/cpp/test/Ice/operations/Twoways.cpp
+++ b/cpp/test/Ice/operations/Twoways.cpp
@@ -692,8 +692,8 @@ twoways(const Ice::CommunicatorPtr& communicator, const Test::MyClassPrx& p)
             // Test implicit context propagation
             //
             
-            string impls[] = {"Shared", "SharedWithoutLocking", "PerThread"};
-            for(int i = 0; i < 3; i++)
+            string impls[] = {"Shared", "PerThread"};
+            for(int i = 0; i < 2; i++)
             {
                 Ice::InitializationData initData;
                 initData.properties = communicator->getProperties()->clone();
@@ -712,10 +712,12 @@ twoways(const Ice::CommunicatorPtr& communicator, const Test::MyClassPrx& p)
                 ic->getImplicitContext()->setContext(ctx);
                 test(ic->getImplicitContext()->getContext() == ctx);
                 test(p->opContext() == ctx);
-
-                ic->getImplicitContext()->set("zero", "ZERO");
+                
+                test(ic->getImplicitContext()->containsKey("zero") == false);
+                string r = ic->getImplicitContext()->put("zero", "ZERO");
+                test(r == "");
+                test(ic->getImplicitContext()->containsKey("zero") == true);
                 test(ic->getImplicitContext()->get("zero") == "ZERO");
-                test(ic->getImplicitContext()->getWithDefault("foobar", "foo") == "foo");
 
                 ctx = ic->getImplicitContext()->getContext();
                 test(p->opContext() == ctx);
@@ -735,6 +737,8 @@ twoways(const Ice::CommunicatorPtr& communicator, const Test::MyClassPrx& p)
 
                 ic->getImplicitContext()->setContext(ctx);
                 test(p->opContext() == combined);
+
+                test(ic->getImplicitContext()->remove("one") == "ONE");
                 
                 ic->destroy();
             }
diff --git a/cpp/test/Ice/operations/TwowaysAMI.cpp b/cpp/test/Ice/operations/TwowaysAMI.cpp
index 37bdf2895b4..158f0c6df25 100644
--- a/cpp/test/Ice/operations/TwowaysAMI.cpp
+++ b/cpp/test/Ice/operations/TwowaysAMI.cpp
@@ -1329,8 +1329,8 @@ twowaysAMI(const Ice::CommunicatorPtr& communicator, const Test::MyClassPrx& p)
             // Test implicit context propagation
             //
             
-            string impls[] = {"Shared", "SharedWithoutLocking", "PerThread"};
-            for(int i = 0; i < 3; i++)
+            string impls[] = {"Shared", "PerThread"};
+            for(int i = 0; i < 2; i++)
             {
                 Ice::InitializationData initData;
                 initData.properties = communicator->getProperties()->clone();
@@ -1356,10 +1356,8 @@ twowaysAMI(const Ice::CommunicatorPtr& communicator, const Test::MyClassPrx& p)
                     test(cb->check());
                 }
 
-                ic->getImplicitContext()->set("zero", "ZERO");
-                test(ic->getImplicitContext()->get("zero") == "ZERO");
-                test(ic->getImplicitContext()->getWithDefault("foobar", "foo") == "foo");
-
+                ic->getImplicitContext()->put("zero", "ZERO");
+          
                 ctx = ic->getImplicitContext()->getContext();
                 {
                     AMI_MyClass_opContextEqualIPtr cb = new AMI_MyClass_opContextEqualI(ctx);