Implement the creation of SAFEARRAY(VT_RECORD) from a sequence of COM…

… Records (#2317)

* Implemented the creation of SAFEARRAY(VT_RECORD) from a sequence
of COM Records and the possibility to use instances of this type
as COM Record fields.
* Added a untitest for COM Records with SAFEARRAY(VT_RECORD) fields.
* Added CHANGELOG entry.

CHANGES.txt

@@ -15,7 +15,8 @@ Coming in build 309, as yet unreleased
 --------------------------------------
 
 * Dropped support for Python 3.7 (#2207, @Avasam)
-* Implement record pointers as [in, out] method parameters of a Dispatch Interface (#2310)
+* Implement the creation of SAFEARRAY(VT_RECORD) from a sequence of COM Records (#2317, @geppi)
+* Implement record pointers as [in, out] method parameters of a Dispatch Interface (#2304, #2310, @geppi)
 * Fix memory leak converting to PyObject from some SAFEARRAY elements (#2316)
 * Fix bug where makepy support was unnecessarily generated (#2354, #2353, @geppi)
 * Fail sooner on invalid `win32timezone.TimeZoneInfo` creation (#2338, @Avasam)

com/TestSources/PyCOMTest/PyCOMImpl.cpp

@@ -626,6 +626,27 @@ HRESULT CPyCOMTest::ModifyStruct(TestStruct1 *prec)
     return S_OK;
 }
 
+HRESULT CPyCOMTest::VerifyArrayOfStructs(TestStruct2 *prec, VARIANT_BOOL *is_ok)
+{
+    long i;
+    TestStruct1 *pdata = NULL;
+    HRESULT hr;
+
+    hr = SafeArrayAccessData(prec->array_of_records, reinterpret_cast<void **>(&pdata));
+    if (FAILED(hr)) {
+        return E_FAIL;
+    }
+    *is_ok = VARIANT_TRUE;
+    for (i = 0; i < prec->rec_count; i++)
+    {
+        if (_wcsicmp(pdata[i].str_value, L"This is record number") != 0 || pdata[i].int_value != i + 1) {
+            *is_ok = VARIANT_FALSE;
+            break;
+        }
+    }
+    return S_OK;
+}
+
 HRESULT CPyCOMTest::DoubleString(BSTR in, BSTR *out)
 {
     *out = SysAllocStringLen(NULL, SysStringLen(in) * 2);

com/TestSources/PyCOMTest/PyCOMImpl.h

@@ -120,6 +120,7 @@ class CPyCOMTest : public IDispatchImpl<IPyCOMTest, &IID_IPyCOMTest, &LIBID_PyCO
     STDMETHOD(def)();
 
     STDMETHOD(ModifyStruct)(TestStruct1 *prec);
+    STDMETHOD(VerifyArrayOfStructs)(TestStruct2 *prec, VARIANT_BOOL *is_ok);
 
     // info associated to each session
     struct PyCOMTestSessionData {

com/TestSources/PyCOMTest/PyCOMTest.idl

@@ -89,6 +89,11 @@ library PyCOMTestLib
 		int int_value;
 		BSTR str_value;
 	} StructWithoutUUID;
+    typedef [uuid(78F0EA07-B7CF-42EA-A251-A4C6269F76AF), version(1.0)]
+    struct TestStruct2 {
+        SAFEARRAY(TestStruct1) array_of_records;
+        int rec_count;
+    } TestStruct2;
 
 	// Test enumerators.
    [
@@ -295,6 +300,7 @@ library PyCOMTestLib
 		HRESULT def();
         // Test struct byref as [ in, out ] parameter.
         HRESULT ModifyStruct([ in, out ] TestStruct1 * prec);
+        HRESULT VerifyArrayOfStructs([in] TestStruct2 * prec, [ out, retval ] VARIANT_BOOL * is_ok);
 	};
 
 	// Define a new class to test how Python handles derived interfaces!

com/win32com/src/PyRecord.cpp

@@ -499,9 +499,6 @@ PyObject *PyRecord::getattro(PyObject *self, PyObject *obname)
         return new PyRecord(V_RECORDINFO(&vret), V_RECORD(&vret), pyrec->owner);
     else if (V_VT(&vret) == (VT_BYREF | VT_ARRAY | VT_RECORD)) {
         SAFEARRAY *psa = *V_ARRAYREF(&vret);
-        int d = SafeArrayGetDim(psa);
-        if (sub_data == NULL)
-            return PyErr_Format(PyExc_RuntimeError, "Did not get a buffer for the array!");
         if (SafeArrayGetDim(psa) != 1)
             return PyErr_Format(PyExc_TypeError, "Only support single dimensional arrays of records");
         IRecordInfo *sub = NULL;
@@ -526,7 +523,13 @@ PyObject *PyRecord::getattro(PyObject *self, PyObject *obname)
         ret_tuple = PyTuple_New(nelems);
         if (ret_tuple == NULL)
             goto array_end;
-        this_data = (BYTE *)sub_data;
+        // We're dealing here with a Record field that is a SAFEARRAY of Records.
+        // Therefore the VARIANT that was returned by the call to 'pyrec->pri->GetFieldNoCopy'
+        // does contain a reference to the SAFEARRAY of Records, i.e. the actual data of the
+        // Record elements of this SAFEARRAY is referenced by the 'pvData' field of the SAFEARRAY.
+        // In this particular case the implementation of 'GetFieldNoCopy' returns a NULL pointer
+        // in the last parameter, i.e. 'sub_data == NULL'.
+        this_data = (BYTE *)psa->pvData;
         for (i = 0; i < nelems; i++) {
             PyTuple_SET_ITEM(ret_tuple, i, new PyRecord(sub, this_data, pyrec->owner));
             this_data += element_size;

com/win32com/src/oleargs.cpp

@@ -4,6 +4,7 @@
 
 #include "stdafx.h"
 #include "PythonCOM.h"
+#include "PyRecord.h"
 
 extern PyObject *PyObject_FromRecordInfo(IRecordInfo *, void *, ULONG);
 extern PyObject *PyObject_FromSAFEARRAYRecordInfo(SAFEARRAY *psa);
@@ -278,9 +279,23 @@ BOOL PyCom_VariantFromPyObject(PyObject *obj, VARIANT *var)
     // So make sure this check is after anything else which qualifies.
     else if (PySequence_Check(obj)) {
         V_ARRAY(var) = NULL;  // not a valid, existing array.
-        if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var)))
-            return FALSE;
-        V_VT(var) = VT_ARRAY | VT_VARIANT;
+        BOOL is_record_item = false;
+        if (PyObject_Length(obj) > 0) {
+            PyObject *obItemCheck = PySequence_GetItem(obj, 0);
+            is_record_item = PyRecord_Check(obItemCheck);
+        }
+        // If the sequence elements are PyRecord objects we do NOT package
+        // them as VARIANT elements but put them directly into the SAFEARRAY.
+        if (is_record_item) {
+            if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var), VT_RECORD))
+                    return FALSE;
+            V_VT(var) = VT_ARRAY | VT_RECORD;
+        }
+        else {
+            if (!PyCom_SAFEARRAYFromPyObject(obj, &V_ARRAY(var)))
+                return FALSE;
+            V_VT(var) = VT_ARRAY | VT_VARIANT;
+        }
     }
     else if (PyRecord_Check(obj)) {
         if (!PyObject_AsVARIANTRecordInfo(obj, var))
@@ -554,6 +569,9 @@ static BOOL PyCom_SAFEARRAYFromPyObjectBuildDimension(PyObject *obj, SAFEARRAY *
                     helper.m_reqdType = vt;
                     ok = helper.MakeObjToVariant(item, &element);
                     switch (vt) {
+                        case VT_RECORD:
+                            pvData = V_RECORD(&element);
+                            break;
                         case VT_DISPATCH:
                             pvData = V_DISPATCH(&element);
                             break;
@@ -759,7 +777,14 @@ static BOOL PyCom_SAFEARRAYFromPyObjectEx(PyObject *obj, SAFEARRAY **ppSA, bool
 
     if (bAllocNewArray) {
         // OK - Finally can create the array...
-        *ppSA = SafeArrayCreate(vt, cDims, pBounds);
+        if (vt == VT_RECORD) {
+            // SAFEARRAYS of UDTs need a special treatment.
+            obItemCheck = PySequence_GetItem(obj, 0);
+            PyRecord *pyrec = (PyRecord *)obItemCheck;
+            *ppSA = SafeArrayCreateEx(vt, cDims, pBounds, pyrec->pri);
+        }
+        else
+            *ppSA = SafeArrayCreate(vt, cDims, pBounds);
         if (*ppSA == NULL) {
             delete[] pBounds;
             PyErr_SetString(PyExc_MemoryError, "CreatingSafeArray");

com/win32com/test/testPyComTest.py

@@ -411,6 +411,7 @@ def TestDynamic():
 
 def TestGenerated():
     # Create an instance of the server.
+    from win32com.client import Record
     from win32com.client.gencache import EnsureDispatch
 
     o = EnsureDispatch("PyCOMTest.PyCOMTest")
@@ -433,6 +434,19 @@ def TestGenerated():
     coclass = GetClass("{B88DD310-BAE8-11D0-AE86-76F2C1000000}")()
     TestCounter(coclass, True)
 
+    # Test records with SAFEARRAY(VT_RECORD) fields.
+    progress("Testing records with SAFEARRAY(VT_RECORD) fields.")
+    l = []
+    for i in range(3):
+        rec = Record("TestStruct1", o)
+        rec.str_value = "This is record number"
+        rec.int_value = i + 1
+        l.append(rec)
+    test_rec = Record("TestStruct2", o)
+    test_rec.array_of_records = l
+    test_rec.rec_count = i + 1
+    assert o.VerifyArrayOfStructs(test_rec)
+
     # XXX - this is failing in dynamic tests, but should work fine.
     i1, i2 = o.GetMultipleInterfaces()
     # Yay - is now an instance returned!