adding ice_preMarshal/ice_postUnmarshal, and tests in Ice/objects

11 files changed, 267 insertions, 18 deletions

diff --git a/cpp/CHANGES b/cpp/CHANGES
index 669c9896004..d965a92b6d0 100644
--- a/cpp/CHANGES
+++ b/cpp/CHANGES
@@ -1,7 +1,12 @@
 Changes since version 1.2.0
 ---------------------------
 
-- Added the demo/IcePack/simple demo.
+- Added the virtual member functions ice_preMarshal and
+  ice_postUnmarshal to Ice::Object. The default implementations do
+  nothing, but subclasses may override them to take special action
+  before marshaling and after unmarshaling, respectively.
+
+- Added the demo/IcePack/simple example.
 
 - Fixed a bug in IcePack that could cause the node to crash if a
   server failed to start.
@@ -9,8 +14,8 @@ Changes since version 1.2.0
 - IceBox services deployed with IcePack are now loaded in the order
   they appear in the XML IceBox descriptor.
 
-- Connections are now not closed anymore when that last proxy using
-  the connection is destroyed. Doing so is error prone:
+- Connections are no longer closed when the last proxy using the
+  connection is destroyed. Doing so is error prone:
 
   * Quite often, proxies are created on the fly, resulting in
     connections being opened and closed all the time. This is
@@ -23,8 +28,8 @@ Changes since version 1.2.0
   * It doesn't work well with AMI requests, because the AMI callback
     objects keep track of connections directly. This would mean that
     if a process only uses AMI requests, a connection is opened and
-    closed for each such request, as each request typically has its
-    own AMI callback object.
+    closed for each request, as each request typically has its own
+    AMI callback object.
 
   Instead, ACM (Automatic Connection Management) is now enabled by
   default, with a default value of one minute. This means that idle
diff --git a/cpp/include/Ice/BasicStream.h b/cpp/include/Ice/BasicStream.h
index 161a5861168..4a799f84245 100644
--- a/cpp/include/Ice/BasicStream.h
+++ b/cpp/include/Ice/BasicStream.h
@@ -189,6 +189,8 @@ public:
     typedef std::map<Ice::ObjectPtr, Ice::Int> PtrToIndexMap;
     typedef std::map<std::string, Ice::Int> TypeIdWriteMap;
 
+    typedef std::vector<Ice::ObjectPtr> ObjectList;
+
 private:
 
     //
@@ -260,6 +262,8 @@ private:
     bool _sliceObjects;
 
     const Container::size_type _messageSizeMax;
+
+    ObjectList* _objectList;
 };
 
 }
diff --git a/cpp/include/Ice/Object.h b/cpp/include/Ice/Object.h
index 9e1a24d9616..321974b0680 100644
--- a/cpp/include/Ice/Object.h
+++ b/cpp/include/Ice/Object.h
@@ -79,6 +79,9 @@ public:
     void __copyMembers(::Ice::ObjectPtr) const;
     virtual ::Ice::ObjectPtr ice_clone() const;
 
+    virtual void ice_preMarshal();
+    virtual void ice_postUnmarshal();
+
     static ::std::string __all[];
     virtual ::IceInternal::DispatchStatus __dispatch(::IceInternal::Incoming&, const Current&);
 
diff --git a/cpp/src/Ice/BasicStream.cpp b/cpp/src/Ice/BasicStream.cpp
index 24d9ea4bbda..179bb853e04 100644
--- a/cpp/src/Ice/BasicStream.cpp
+++ b/cpp/src/Ice/BasicStream.cpp
@@ -25,6 +25,7 @@
 #include <Ice/FactoryTable.h>
 #include <Ice/TraceUtil.h>
 #include <Ice/TraceLevels.h>
+#include <Ice/LoggerUtil.h>
 
 template<typename InputIter, typename OutputIter>
 inline void
@@ -55,7 +56,8 @@ IceInternal::BasicStream::BasicStream(Instance* instance) :
     _traceSlicing(-1),
     _marshalFacets(true),
     _sliceObjects(true),
-    _messageSizeMax(_instance->messageSizeMax()) // Cached for efficiency.
+    _messageSizeMax(_instance->messageSizeMax()), // Cached for efficiency.
+    _objectList(0)
 {
 }
 
@@ -74,6 +76,8 @@ IceInternal::BasicStream::~BasicStream()
 	_currentWriteEncaps = _currentWriteEncaps->previous;
 	delete oldEncaps;
     }
+
+    delete _objectList;
 }
 
 Instance*
@@ -91,6 +95,7 @@ IceInternal::BasicStream::swap(BasicStream& other)
     std::swap(i, other.i);
     std::swap(_currentReadEncaps, other._currentReadEncaps);
     std::swap(_currentWriteEncaps, other._currentWriteEncaps);
+    std::swap(_objectList, other._objectList);
 }
 
 void
@@ -1312,6 +1317,16 @@ IceInternal::BasicStream::read(PatchFunc patchFunc, void* patchAddr)
 	IndexToPtrMap::const_iterator unmarshaledPos =
 			    _currentReadEncaps->unmarshaledMap->insert(make_pair(index, v)).first;
 
+        //
+        // Record each object instance so that readPendingObjects can invoke ice_postUnmarshal
+        // after all objects have been unmarshaled.
+        //
+        if(!_objectList)
+        {
+            _objectList = new ObjectList;
+        }
+        _objectList->push_back(v);
+
 	v->__read(this, false);
 	patchPointers(index, unmarshaledPos, _currentReadEncaps->patchMap->end());
 	return;
@@ -1449,6 +1464,37 @@ IceInternal::BasicStream::readPendingObjects()
 	}
     }
     while(num);
+
+    //
+    // Iterate over the object list and invoke ice_postUnmarshal on each object.
+    // We must do this after all objects have been unmarshaled in order to ensure
+    // that any object data members have been properly patched.
+    //
+    if(_objectList)
+    {
+        for(ObjectList::iterator p = _objectList->begin(); p != _objectList->end(); ++p)
+        {
+            try
+            {
+                (*p)->ice_postUnmarshal();
+            }
+            catch(const Ice::Exception& ex)
+            {
+                Ice::Warning out(_instance->logger());
+                out << "Ice::Exception raised by ice_postUnmarshal:\n" << ex;
+            }
+            catch(const std::exception& ex)
+            {
+                Ice::Warning out(_instance->logger());
+                out << "std::exception raised by ice_postUnmarshal:\n" << ex.what();
+            }
+            catch(...)
+            {
+                Ice::Warning out(_instance->logger());
+                out << "unknown exception raised by ice_postUnmarshal";
+            }
+        }
+    }
 }
 
 void
@@ -1479,6 +1525,25 @@ void
 IceInternal::BasicStream::writeInstance(const ObjectPtr& v, Int index)
 {
     write(index);
+    try
+    {
+        v->ice_preMarshal();
+    }
+    catch(const Ice::Exception& ex)
+    {
+        Ice::Warning out(_instance->logger());
+        out << "Ice::Exception raised by ice_preMarshal:\n" << ex;
+    }
+    catch(const std::exception& ex)
+    {
+        Ice::Warning out(_instance->logger());
+        out << "std::exception raised by ice_preMarshal:\n" << ex.what();
+    }
+    catch(...)
+    {
+        Ice::Warning out(_instance->logger());
+        out << "unknown exception raised by ice_preMarshal";
+    }
     v->__write(this, _marshalFacets);
 }
 
diff --git a/cpp/src/Ice/Object.cpp b/cpp/src/Ice/Object.cpp
index 123370949ed..20cc3371795 100644
--- a/cpp/src/Ice/Object.cpp
+++ b/cpp/src/Ice/Object.cpp
@@ -122,6 +122,16 @@ Ice::Object::ice_clone() const
     return __p;
 }
 
+void
+Ice::Object::ice_preMarshal()
+{
+}
+
+void
+Ice::Object::ice_postUnmarshal()
+{
+}
+
 DispatchStatus
 Ice::Object::___ice_isA(Incoming& __in, const Current& __current)
 {
diff --git a/cpp/test/Ice/objects/AllTests.cpp b/cpp/test/Ice/objects/AllTests.cpp
index 53ab064d30e..4e6b4cb2ebc 100644
--- a/cpp/test/Ice/objects/AllTests.cpp
+++ b/cpp/test/Ice/objects/AllTests.cpp
@@ -67,6 +67,15 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
     test(BPtr::dynamicCast(b1->theA)->theB == b1);
     test(CPtr::dynamicCast(BPtr::dynamicCast(b1->theA)->theC));
     test(CPtr::dynamicCast(BPtr::dynamicCast(b1->theA)->theC)->theB == b1->theA);
+    if(!collocated)
+    {
+        test(b1->preMarshalInvoked);
+        test(b1->postUnmarshalInvoked());
+        test(b1->theA->preMarshalInvoked);
+        test(b1->theA->postUnmarshalInvoked());
+        test(BPtr::dynamicCast(b1->theA)->theC->preMarshalInvoked);
+        test(BPtr::dynamicCast(b1->theA)->theC->postUnmarshalInvoked());
+    }
     // More tests possible for b2 and d, but I think this is already sufficient.
     test(b2->theA == b2);
     test(d->theC == 0);
@@ -127,6 +136,17 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
     test(d->theA == b1);
     test(d->theB == b2);
     test(d->theC == 0);
+    if(!collocated)
+    {
+        test(d->preMarshalInvoked);
+        test(d->postUnmarshalInvoked());
+        test(d->theA->preMarshalInvoked);
+        test(d->theA->postUnmarshalInvoked());
+        test(d->theB->preMarshalInvoked);
+        test(d->theB->postUnmarshalInvoked());
+        test(d->theB->theC->preMarshalInvoked);
+        test(d->theB->theC->postUnmarshalInvoked());
+    }
     cout << "ok" << endl;
 
     //
diff --git a/cpp/test/Ice/objects/Client.cpp b/cpp/test/Ice/objects/Client.cpp
index 9a6a80c1873..2eea6e63466 100644
--- a/cpp/test/Ice/objects/Client.cpp
+++ b/cpp/test/Ice/objects/Client.cpp
@@ -14,7 +14,7 @@
 
 #include <Ice/Ice.h>
 #include <TestCommon.h>
-#include <Test.h>
+#include <TestI.h>
 
 using namespace std;
 
@@ -26,15 +26,15 @@ public:
     {
 	if(type == "::B")
 	{
-	    return new B;
+	    return new BI;
 	}
 	else if(type == "::C")
 	{
-	    return new C;
+	    return new CI;
 	}
 	else if(type == "::D")
 	{
-	    return new D;
+	    return new DI;
 	}
 	assert(false); // Should never be reached
 	return 0;
diff --git a/cpp/test/Ice/objects/Makefile b/cpp/test/Ice/objects/Makefile
index f3502da1d93..9f7c5f346b7 100644
--- a/cpp/test/Ice/objects/Makefile
+++ b/cpp/test/Ice/objects/Makefile
@@ -21,15 +21,14 @@ COLLOCATED	= collocated
 TARGETS		= $(CLIENT) $(SERVER) $(COLLOCATED)
 
 OBJS		= Test.o \
+                  TestI.o
 
 COBJS		= Client.o \
 		  AllTests.o
 
-SOBJS		= TestI.o \
-		  Server.o
+SOBJS		= Server.o
 
-COLOBJS		= TestI.o \
-		  Collocated.o \
+COLOBJS		= Collocated.o \
 		  AllTests.o
 
 SRCS		= $(OBJS:.o=.cpp) \
diff --git a/cpp/test/Ice/objects/Test.ice b/cpp/test/Ice/objects/Test.ice
index e1d696af92c..c6c94c810ee 100644
--- a/cpp/test/Ice/objects/Test.ice
+++ b/cpp/test/Ice/objects/Test.ice
@@ -22,6 +22,9 @@ class A
 {
     B theB;
     C theC;
+
+    bool preMarshalInvoked;
+    bool postUnmarshalInvoked();
 };
 
 class B extends A
@@ -32,6 +35,9 @@ class B extends A
 class C
 {
     B theB;
+
+    bool preMarshalInvoked;
+    bool postUnmarshalInvoked();
 };
 
 class D
@@ -39,6 +45,9 @@ class D
     A theA;
     B theB;
     C theC;    
+
+    bool preMarshalInvoked;
+    bool postUnmarshalInvoked();
 };
 
 class Initial
diff --git a/cpp/test/Ice/objects/TestI.cpp b/cpp/test/Ice/objects/TestI.cpp
index ef961501878..5dd46076762 100644
--- a/cpp/test/Ice/objects/TestI.cpp
+++ b/cpp/test/Ice/objects/TestI.cpp
@@ -15,12 +15,81 @@
 #include <Ice/Ice.h>
 #include <TestI.h>
 
+BI::BI() :
+    _postUnmarshalInvoked(false)
+{
+}
+
+bool
+BI::postUnmarshalInvoked(const Ice::Current&)
+{
+    return _postUnmarshalInvoked;
+}
+
+void
+BI::ice_preMarshal()
+{
+    preMarshalInvoked = true;
+}
+
+void
+BI::ice_postUnmarshal()
+{
+    _postUnmarshalInvoked = true;
+}
+
+CI::CI() :
+    _postUnmarshalInvoked(false)
+{
+}
+
+bool
+CI::postUnmarshalInvoked(const Ice::Current&)
+{
+    return _postUnmarshalInvoked;
+}
+
+void
+CI::ice_preMarshal()
+{
+    preMarshalInvoked = true;
+}
+
+void
+CI::ice_postUnmarshal()
+{
+    _postUnmarshalInvoked = true;
+}
+
+DI::DI() :
+    _postUnmarshalInvoked(false)
+{
+}
+
+bool
+DI::postUnmarshalInvoked(const Ice::Current&)
+{
+    return _postUnmarshalInvoked;
+}
+
+void
+DI::ice_preMarshal()
+{
+    preMarshalInvoked = true;
+}
+
+void
+DI::ice_postUnmarshal()
+{
+    _postUnmarshalInvoked = true;
+}
+
 InitialI::InitialI(const Ice::ObjectAdapterPtr& adapter) :
     _adapter(adapter),
-    _b1(new B),
-    _b2(new B),
-    _c(new C),
-    _d(new D)
+    _b1(new BI),
+    _b2(new BI),
+    _c(new CI),
+    _d(new DI)
 {
     _b1->theA = _b2; // Cyclic reference to another B
     _b1->theB = _b1; // Self reference.
@@ -64,30 +133,47 @@ InitialI::shutdown(const Ice::Current&)
 BPtr
 InitialI::getB1(const Ice::Current&)
 {
+    _b1->preMarshalInvoked = false;
+    _b2->preMarshalInvoked = false;
+    _c->preMarshalInvoked = false;
     return _b1;
 }
 
 BPtr
 InitialI::getB2(const Ice::Current&)
 {
+    _b1->preMarshalInvoked = false;
+    _b2->preMarshalInvoked = false;
+    _c->preMarshalInvoked = false;
     return _b2;
 }
 
 CPtr
 InitialI::getC(const Ice::Current&)
 {
+    _b1->preMarshalInvoked = false;
+    _b2->preMarshalInvoked = false;
+    _c->preMarshalInvoked = false;
     return _c;
 }
 
 DPtr
 InitialI::getD(const Ice::Current&)
 {
+    _b1->preMarshalInvoked = false;
+    _b2->preMarshalInvoked = false;
+    _c->preMarshalInvoked = false;
+    _d->preMarshalInvoked = false;
     return _d;
 }
 
 void
 InitialI::getAll(BPtr& b1, BPtr& b2, CPtr& c, DPtr& d, const Ice::Current&)
 {
+    _b1->preMarshalInvoked = false;
+    _b2->preMarshalInvoked = false;
+    _c->preMarshalInvoked = false;
+    _d->preMarshalInvoked = false;
     b1 = _b1;
     b2 = _b2;
     c = _c;
diff --git a/cpp/test/Ice/objects/TestI.h b/cpp/test/Ice/objects/TestI.h
index f21e677677f..b03ff8d43b3 100644
--- a/cpp/test/Ice/objects/TestI.h
+++ b/cpp/test/Ice/objects/TestI.h
@@ -17,6 +17,54 @@
 
 #include <Test.h>
 
+class BI : public B
+{
+public:
+
+    BI();
+
+    virtual bool postUnmarshalInvoked(const Ice::Current&);
+
+    virtual void ice_preMarshal();
+    virtual void ice_postUnmarshal();
+
+private:
+
+    bool _postUnmarshalInvoked;
+};
+
+class CI : public C
+{
+public:
+
+    CI();
+
+    virtual bool postUnmarshalInvoked(const Ice::Current&);
+
+    virtual void ice_preMarshal();
+    virtual void ice_postUnmarshal();
+
+private:
+
+    bool _postUnmarshalInvoked;
+};
+
+class DI : public D
+{
+public:
+
+    DI();
+
+    virtual bool postUnmarshalInvoked(const Ice::Current&);
+
+    virtual void ice_preMarshal();
+    virtual void ice_postUnmarshal();
+
+private:
+
+    bool _postUnmarshalInvoked;
+};
+
 class InitialI : public Initial
 {
 public: