Python changes for bug2522

6 files changed, 131 insertions, 127 deletions

diff --git a/py/modules/IcePy/Operation.cpp b/py/modules/IcePy/Operation.cpp
index 19ae001c952..b98679030a5 100644
--- a/py/modules/IcePy/Operation.cpp
+++ b/py/modules/IcePy/Operation.cpp
@@ -298,6 +298,14 @@ extern PyTypeObject AMDCallbackType;
 
 }
 
+static OperationPtr
+getOperation(PyObject* p)
+{
+    assert(PyObject_IsInstance(p, reinterpret_cast<PyObject*>(&OperationType)) == 1);
+    OperationObject* obj = reinterpret_cast<OperationObject*>(p);
+    return *obj->op;
+}
+
 #ifdef WIN32
 extern "C"
 #endif
@@ -2070,6 +2078,66 @@ IcePy::BlobjectUpcall::exception(PyException& ex)
 }
 
 PyObject*
+IcePy::iceIsA(const Ice::ObjectPrx& prx, PyObject* args)
+{
+    PyObject* objectType = lookupType("Ice.Object");
+    assert(objectType);
+    PyObjectHandle obj = PyObject_GetAttrString(objectType, "_op_ice_isA");
+    assert(obj.get());
+
+    OperationPtr op = getOperation(obj.get());
+    assert(op);
+
+    InvocationPtr i = new SyncTypedInvocation(prx, op);
+    return i->invoke(args);
+}
+
+PyObject*
+IcePy::icePing(const Ice::ObjectPrx& prx, PyObject* args)
+{
+    PyObject* objectType = lookupType("Ice.Object");
+    assert(objectType);
+    PyObjectHandle obj = PyObject_GetAttrString(objectType, "_op_ice_ping");
+    assert(obj.get());
+
+    OperationPtr op = getOperation(obj.get());
+    assert(op);
+
+    InvocationPtr i = new SyncTypedInvocation(prx, op);
+    return i->invoke(args);
+}
+
+PyObject*
+IcePy::iceIds(const Ice::ObjectPrx& prx, PyObject* args)
+{
+    PyObject* objectType = lookupType("Ice.Object");
+    assert(objectType);
+    PyObjectHandle obj = PyObject_GetAttrString(objectType, "_op_ice_ids");
+    assert(obj.get());
+
+    OperationPtr op = getOperation(obj.get());
+    assert(op);
+
+    InvocationPtr i = new SyncTypedInvocation(prx, op);
+    return i->invoke(args);
+}
+
+PyObject*
+IcePy::iceId(const Ice::ObjectPrx& prx, PyObject* args)
+{
+    PyObject* objectType = lookupType("Ice.Object");
+    assert(objectType);
+    PyObjectHandle obj = PyObject_GetAttrString(objectType, "_op_ice_id");
+    assert(obj.get());
+
+    OperationPtr op = getOperation(obj.get());
+    assert(op);
+
+    InvocationPtr i = new SyncTypedInvocation(prx, op);
+    return i->invoke(args);
+}
+
+PyObject*
 IcePy::iceInvoke(const Ice::ObjectPrx& prx, PyObject* args)
 {
     InvocationPtr i = new SyncBlobjectInvocation(prx);
diff --git a/py/modules/IcePy/Operation.h b/py/modules/IcePy/Operation.h
index c7c72e1005c..be3b99a870f 100644
--- a/py/modules/IcePy/Operation.h
+++ b/py/modules/IcePy/Operation.h
@@ -20,6 +20,14 @@ namespace IcePy
 bool initOperation(PyObject*);
 
 //
+// Builtin operations.
+//
+PyObject* iceIsA(const Ice::ObjectPrx&, PyObject*);
+PyObject* icePing(const Ice::ObjectPrx&, PyObject*);
+PyObject* iceIds(const Ice::ObjectPrx&, PyObject*);
+PyObject* iceId(const Ice::ObjectPrx&, PyObject*);
+
+//
 // Blobject invocations.
 //
 PyObject* iceInvoke(const Ice::ObjectPrx&, PyObject*);
diff --git a/py/modules/IcePy/Proxy.cpp b/py/modules/IcePy/Proxy.cpp
index 78fe4ff32ba..5fc598916f2 100644
--- a/py/modules/IcePy/Proxy.cpp
+++ b/py/modules/IcePy/Proxy.cpp
@@ -222,46 +222,18 @@ static PyObject*
 proxyIceIsA(ProxyObject* self, PyObject* args)
 {
     char* type;
-    PyObject* ctx = 0;
+    PyObject* ctx = Py_None;
     if(!PyArg_ParseTuple(args, STRCAST("s|O!"), &type, &PyDict_Type, &ctx))
     {
         return 0;
     }
 
-    assert(self->proxy);
-
-    bool b;
-    try
-    {
-        AllowThreads allowThreads; // Release Python's global interpreter lock during remote invocations.
-        if(ctx)
-        {
-            Ice::Context context;
-            if(!dictionaryToContext(ctx, context))
-            {
-                return 0;
-            }
-            b = (*self->proxy)->ice_isA(type, context);
-        }
-        else
-        {
-            b = (*self->proxy)->ice_isA(type);
-        }
-    }
-    catch(const Ice::Exception& ex)
-    {
-        setPythonException(ex);
-        return 0;
-    }
+    //
+    // We need to reformat the arguments to match what is used by the generated code: ((params...), ctx|None)
+    //
+    PyObjectHandle newArgs = Py_BuildValue(STRCAST("((O), O)"), type, ctx);
 
-    if(b)
-    {
-        PyRETURN_TRUE;
-    }
-    else
-    {
-        PyRETURN_FALSE;
-    }
+    return iceIsA(*self->proxy, newArgs.get());
 }
 
 #ifdef WIN32
@@ -270,39 +242,18 @@ extern "C"
 static PyObject*
 proxyIcePing(ProxyObject* self, PyObject* args)
 {
-    PyObject* ctx = 0;
+    PyObject* ctx = Py_None;
     if(!PyArg_ParseTuple(args, STRCAST("|O!"), &PyDict_Type, &ctx))
     {
         return 0;
     }
 
-    assert(self->proxy);
-
-    try
-    {
-        AllowThreads allowThreads; // Release Python's global interpreter lock during remote invocations.
-        if(ctx)
-        {
-            Ice::Context context;
-            if(!dictionaryToContext(ctx, context))
-            {
-                return 0;
-            }
-            (*self->proxy)->ice_ping(context);
-        }
-        else
-        {
-            (*self->proxy)->ice_ping();
-        }
-    }
-    catch(const Ice::Exception& ex)
-    {
-        setPythonException(ex);
-        return 0;
-    }
+    //
+    // We need to reformat the arguments to match what is used by the generated code: ((params...), ctx|None)
+    //
+    PyObjectHandle newArgs = Py_BuildValue(STRCAST("((), O)"), ctx);
 
-    Py_INCREF(Py_None);
-    return Py_None;
+    return icePing(*self->proxy, newArgs.get());
 }
 
 #ifdef WIN32
@@ -311,45 +262,18 @@ extern "C"
 static PyObject*
 proxyIceIds(ProxyObject* self, PyObject* args)
 {
-    PyObject* ctx = 0;
+    PyObject* ctx = Py_None;
     if(!PyArg_ParseTuple(args, STRCAST("|O!"), &PyDict_Type, &ctx))
     {
         return 0;
     }
 
-    assert(self->proxy);
-
-    Ice::StringSeq ids;
-    try
-    {
-        AllowThreads allowThreads; // Release Python's global interpreter lock during remote invocations.
-        if(ctx)
-        {
-            Ice::Context context;
-            if(!dictionaryToContext(ctx, context))
-            {
-                return 0;
-            }
-            ids = (*self->proxy)->ice_ids(context);
-        }
-        else
-        {
-            ids = (*self->proxy)->ice_ids();
-        }
-    }
-    catch(const Ice::Exception& ex)
-    {
-        setPythonException(ex);
-        return 0;
-    }
-
-    PyObject* list = PyList_New(0);
-    if(!list || !stringSeqToList(ids, list))
-    {
-        return 0;
-    }
+    //
+    // We need to reformat the arguments to match what is used by the generated code: ((params...), ctx|None)
+    //
+    PyObjectHandle newArgs = Py_BuildValue(STRCAST("((), O)"), ctx);
 
-    return list;
+    return iceIds(*self->proxy, newArgs.get());
 }
 
 #ifdef WIN32
@@ -358,39 +282,18 @@ extern "C"
 static PyObject*
 proxyIceId(ProxyObject* self, PyObject* args)
 {
-    PyObject* ctx = 0;
+    PyObject* ctx = Py_None;
     if(!PyArg_ParseTuple(args, STRCAST("|O!"), &PyDict_Type, &ctx))
     {
         return 0;
     }
 
-    assert(self->proxy);
-
-    string id;
-    try
-    {
-        AllowThreads allowThreads; // Release Python's global interpreter lock during remote invocations.
-        if(ctx)
-        {
-            Ice::Context context;
-            if(!dictionaryToContext(ctx, context))
-            {
-                return 0;
-            }
-            id = (*self->proxy)->ice_id(context);
-        }
-        else
-        {
-            id = (*self->proxy)->ice_id();
-        }
-    }
-    catch(const Ice::Exception& ex)
-    {
-        setPythonException(ex);
-        return 0;
-    }
+    //
+    // We need to reformat the arguments to match what is used by the generated code: ((params...), ctx|None)
+    //
+    PyObjectHandle newArgs = Py_BuildValue(STRCAST("((), O)"), ctx);
 
-    return Py_BuildValue(STRCAST("s"), id.c_str());
+    return iceId(*self->proxy, newArgs.get());
 }
 
 #ifdef WIN32
diff --git a/py/test/Ice/servantLocator/AllTests.py b/py/test/Ice/servantLocator/AllTests.py
index fb8434769bf..9c7bea5794b 100644
--- a/py/test/Ice/servantLocator/AllTests.py
+++ b/py/test/Ice/servantLocator/AllTests.py
@@ -32,7 +32,6 @@ def testExceptions(obj, collocated):
         test(False)
     except Ice.UnknownUserException, ex:
         test(ex.unknown == "reason")
-        pass
     except:
         test(False)
 
@@ -41,7 +40,8 @@ def testExceptions(obj, collocated):
         test(False)
     except Ice.UnknownLocalException, ex:
         test(ex.unknown == "reason")
-        pass
+    except:
+        test(False)
 
     try:
         obj.unknownException()
@@ -107,7 +107,7 @@ def testExceptions(obj, collocated):
         obj.intfUserException(False)
         test(False)
     except Test.TestImpossibleException:
-        # Operation doesn't throw, but locate() and finshed() throw TestIntfUserException.
+        # Operation doesn't throw, but locate() and finished() throw TestImpossibleException.
         pass
     except:
         test(False)
@@ -116,7 +116,7 @@ def testExceptions(obj, collocated):
         obj.intfUserException(True)
         test(False)
     except Test.TestImpossibleException:
-        # Operation doesn't throw, but locate() and finshed() throw TestIntfUserException.
+        # Operation throws TestIntfUserException, but locate() and finished() throw TestImpossibleException.
         pass
     except:
         test(False)
@@ -135,6 +135,27 @@ def allTests(communicator, collocated):
     test(obj == base)
     print "ok"
 
+    print "testing ice_ids...",
+    sys.stdout.flush()
+    try:
+        obj = communicator.stringToProxy("category/locate:default -p 12010 -t 10000")
+        obj.ice_ids()
+        test(False)
+    except Ice.UnknownUserException, ex:
+        test(ex.unknown == "Test::TestIntfUserException")
+    except:
+        test(False)
+
+    try:
+        obj = communicator.stringToProxy("category/finished:default -p 12010 -t 10000")
+        obj.ice_ids()
+        test(False)
+    except Ice.UnknownUserException, ex:
+        test(ex.unknown == "Test::TestIntfUserException")
+    except:
+        test(False)
+    print "ok"
+
     print "testing servant locator...",
     sys.stdout.flush()
     base = communicator.stringToProxy("category/locate:default -p 12010 -t 10000")
diff --git a/py/test/Ice/servantLocator/TestAMDI.py b/py/test/Ice/servantLocator/TestAMDI.py
index 189702a4225..9663fdcc80e 100644
--- a/py/test/Ice/servantLocator/TestAMDI.py
+++ b/py/test/Ice/servantLocator/TestAMDI.py
@@ -109,7 +109,9 @@ class ServantLocatorI(Ice.ServantLocator):
         self._deactivated = True
 
     def exception(self, current):
-        if current.operation == "requestFailedException":
+        if current.operation == "ice_ids":
+            raise Test.TestIntfUserException()
+        elif current.operation == "requestFailedException":
             raise Ice.ObjectNotExistException()
         elif current.operation == "unknownUserException":
             raise Ice.UnknownUserException("reason")
diff --git a/py/test/Ice/servantLocator/TestI.py b/py/test/Ice/servantLocator/TestI.py
index 52b367308ea..3f9cbd8bc96 100644
--- a/py/test/Ice/servantLocator/TestI.py
+++ b/py/test/Ice/servantLocator/TestI.py
@@ -106,7 +106,9 @@ class ServantLocatorI(Ice.ServantLocator):
         self._deactivated = True
 
     def exception(self, current):
-        if current.operation == "requestFailedException":
+        if current.operation == "ice_ids":
+            raise Test.TestIntfUserException()
+        elif current.operation == "requestFailedException":
             raise Ice.ObjectNotExistException()
         elif current.operation == "unknownUserException":
             raise Ice.UnknownUserException("reason")