Implement SAFEARRAY pointers and SAFEARRAYs as method parameters of a Dispatch Interface

[geppi]

With a COM Dual Interface it is possible to get server side changes to SAFEARRAYs marshaled back to the client.
However, for a pure IDispatch interface this is currently not implemented.

This pull request adds handling of SAFEARRAYs to the implementation of the automation interface.

A unittest and a new COM test interface for the C++ COM test server are also added.

I have to apologize for a mistake I made in #535.
Back at the time I did favor the implementation of several related interfaces in a single component.
Unfortunately I didn't realize that this is a problem when the interfaces inherit from the same interface but require different implementations for this interface as is the case for IDispatch (see Multiple Dual Interfaces).

While adding another component to the C++ COM test server was not a big deal, the component naming scheme for which I had advocated in #535 is inadequate for this setup.

So I was wrong and therefore have now also changed the name of the component that implements the interface for record parameter testing to reflect its limited scope.

In the future, adding additional interfaces and components implementing them will be more straight forward.

[junkmd]

Thank you for contributing the new feature.
Also, thank you for discovering and fixing the “Multiple Dual Interface” problem in the codebase.

I overlooked it because I had never implemented a COM server that defined a Dual interface.

Before I review the codebase related to safearray, I have a request for you.

I would like you to submit another PR focused solely on the correction of the record parameter test, separate from this PR.
At this point, this patch has many changes in terms of the number of files, and I’m afraid I might overlook something in my review.
If you divide the PR by topic, it will be easier to review.

Please divide the commits included in this patch into the following two, cherry-pick the former, and submit a new PR:

• Commits related to the correction of the record parameter test
  • Change of filename from source/CppTestSrv/CoComtypesDispIfcParamTests.h to source/CppTestSrv/CoComtypesDispRecordParamTest.h
  • Change of the ProgID specified in test_dispifc_records.py
• Commits related to the addition of safearray functionality
  • Implementation change in automation.py
  • Addition of CoComtypesDispSafearrayParamTest.cpp
  • Addition of test_dispifc_safearrays.py

I believe that a PR that only makes corrections to the record parameter test can be merged without any problems before this PR.

When you submit a new PR, you do NOT need to close this PR (#569).
Rather, I think it would be useful for future maintainers to leave a log on GitHub that you have rebased on or merged to your patch, as the history of the development process.

[geppi]

OK, I'll look into this week after next week.

[junkmd]

Thank you for considering splitting this PR.
I haven't written much C++ codebase, so I want to keep the granularity of changes I have to review at once as small as possible.

For the Python code part, I think I can do some reviews at this point.
So I might make review comments as appropriate, but there’s no need to respond to them until after you submit the new PR and it gets merged.

[junkmd]

I've read through the C++ code again.

As previously mentioned, it's likely changing such as CoComtypesDispIfcParamTests to CoComtypesDispRecordParamTest will be split off and submitted as a new PR, while adding CoComtypesDispSafearrayParamTest will remain in this PR.

Now that this COM library has multiple components, changes to UnregisterAll in CFACTORY.CPP have become necessary, which were not necessary when there was only a single component.
I think it works even if the library has only a single component. For this kind of codebase, it doesn't problem whether it stays in this patch or is moved to a new PR.

[junkmd]

I believe that submitting large PRs is not a problem.
I think it's healthy for the community for reviewers and reviewees to interact, dividing the codebase into appropriate sizes to make them easier to merge.
On the other hand, there are often cases where it cannot be divided. #535 was just that, even if we separated the GHA workflows and Python code, there were still 2000 lines of C++ code.
Even in such cases, the interaction between the reviewer and the reviewee can reduce the cognitive load due to the amount of code changes. Indeed, this was present in #535, which has been merged.

I NEVER say that "if it is not something that can be easily accepted by the reviewer at first glance, you should not PR".
That reviewer may be strong in the technical area of that codebase and in good health that day, so they may be able to review hundreds or thousands of lines, or they may not be very familiar with that technical area and want to review as few lines as possible. That is something we don't know until we get feedback for the first time that day.

What's important is whether we can interact to make it easier to merge after submitting the PR. I think that I can interact with you in that way and I am looking forward to it.

If you are unsure, please do not hesitate to consult.

[geppi]

OK, I've separated the name changes for the record parameter testing component in #575.

[geppi]

Rebased on main after #575 was merged.

[junkmd]

Memorandum

At the point of 850cedd

Modules generated from the COM type library

_07D2AEE5_1DF8_4D2C_953A_554ADFD25F99_0_1_0.py

wrapper module

# -*- coding: mbcs -*-

from ctypes import *
from comtypes import (
    _check_version, BSTR, CoClass, COMMETHOD, dispid, DISPMETHOD, GUID
)
import comtypes.gen._00020430_0000_0000_C000_000000000046_0_2_0
from ctypes import HRESULT
from ctypes.wintypes import VARIANT_BOOL
from comtypes.automation import _midlSAFEARRAY
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from typing import Any, Tuple
    from comtypes import hints


_lcid = 0  # change this if required
typelib_path = 'D:\\a\\comtypes\\comtypes\\source\\CppTestSrv\\server.tlb'
WSTRING = c_wchar_p



class CoComtypesDispSafearrayParamTest(CoClass):
    """Comtypes component for dispinterface Safearray parameter tests."""
    _reg_clsid_ = GUID('{091D762E-FF4B-4532-8B24-23807FE873C3}')
    _idlflags_ = []
    _typelib_path_ = typelib_path
    _reg_typelib_ = ('{07D2AEE5-1DF8-4D2C-953A-554ADFD25F99}', 1, 0)


class IDualSafearrayParamTest(comtypes.gen._00020430_0000_0000_C000_000000000046_0_2_0.IDispatch):
    """IDualSafearrayParamTest Interface"""
    _case_insensitive_ = True
    _iid_ = GUID('{1F4F3B8B-D07E-4BB6-8D2C-D79B375696DA}')
    _idlflags_ = ['dual', 'nonextensible', 'oleautomation']

    if TYPE_CHECKING:  # commembers
        def InitArray(self, test_array: hints.Incomplete) -> hints.Incomplete: ...
        def VerifyArray(self, test_array: hints.Incomplete) -> hints.Incomplete: ...


class IDispSafearrayParamTest(comtypes.gen._00020430_0000_0000_C000_000000000046_0_2_0.IDispatch):
    _case_insensitive_ = True
    _iid_ = GUID('{4097A6D0-A111-40E2-BD0B-177B775A9496}')
    _idlflags_ = []
    _methods_ = []

    if TYPE_CHECKING:  # dispmembers
        def InitArray(self, test_array: hints.Incomplete) -> hints.Incomplete: ...
        def VerifyArray(self, test_array: hints.Incomplete) -> hints.Incomplete: ...


CoComtypesDispSafearrayParamTest._com_interfaces_ = [IDualSafearrayParamTest, IDispSafearrayParamTest]


class IDualRecordParamTest(comtypes.gen._00020430_0000_0000_C000_000000000046_0_2_0.IDispatch):
    """Dual Interface for testing record parameters."""
    _case_insensitive_ = True
    _iid_ = GUID('{0C4E01E8-4625-46A2-BC4C-2E889A8DBBD6}')
    _idlflags_ = ['dual', 'nonextensible', 'oleautomation']

    if TYPE_CHECKING:  # commembers
        def InitRecord(self, test_record: hints.Incomplete) -> hints.Incomplete: ...
        def VerifyRecord(self, test_record: hints.Incomplete) -> hints.Incomplete: ...


class StructRecordParamTest(Structure):
    _recordinfo_ = ('{07D2AEE5-1DF8-4D2C-953A-554ADFD25F99}', 1, 0, 0, '{00FABB0F-5691-41A6-B7C1-11606671F8E5}')


IDualRecordParamTest._methods_ = [
    COMMETHOD(
        [dispid(1)],
        HRESULT,
        'InitRecord',
        (['in', 'out'], POINTER(StructRecordParamTest), 'test_record')
    ),
    COMMETHOD(
        [dispid(2)],
        HRESULT,
        'VerifyRecord',
        (['in'], POINTER(StructRecordParamTest), 'test_record'),
        (['out', 'retval'], POINTER(VARIANT_BOOL), 'result')
    ),
]

################################################################
# code template for IDualRecordParamTest implementation
# class IDualRecordParamTest_Impl(object):
#     def InitRecord(self):
#         '-no docstring-'
#         #return test_record
#
#     def VerifyRecord(self, test_record):
#         '-no docstring-'
#         #return result
#


class IDispRecordParamTest(comtypes.gen._00020430_0000_0000_C000_000000000046_0_2_0.IDispatch):
    """Dispinterface for testing record parameters."""
    _case_insensitive_ = True
    _iid_ = GUID('{033E4C10-0A7F-4E93-8377-499AD4B6583A}')
    _idlflags_ = []
    _methods_ = []

    if TYPE_CHECKING:  # dispmembers
        def InitRecord(self, test_record: hints.Incomplete) -> hints.Incomplete: ...
        def VerifyRecord(self, test_record: hints.Incomplete) -> hints.Incomplete: ...


IDispRecordParamTest._disp_methods_ = [
    DISPMETHOD(
        [dispid(1)],
        None,
        'InitRecord',
        (['in', 'out'], POINTER(StructRecordParamTest), 'test_record')
    ),
    DISPMETHOD(
        [dispid(2)],
        VARIANT_BOOL,
        'VerifyRecord',
        (['in'], POINTER(StructRecordParamTest), 'test_record')
    ),
]

IDualSafearrayParamTest._methods_ = [
    COMMETHOD(
        [dispid(1)],
        HRESULT,
        'InitArray',
        (['in', 'out'], POINTER(_midlSAFEARRAY(c_double)), 'test_array')
    ),
    COMMETHOD(
        [dispid(2)],
        HRESULT,
        'VerifyArray',
        (['in'], _midlSAFEARRAY(c_double), 'test_array'),
        (['out', 'retval'], POINTER(VARIANT_BOOL), 'result')
    ),
]

################################################################
# code template for IDualSafearrayParamTest implementation
# class IDualSafearrayParamTest_Impl(object):
#     def InitArray(self):
#         '-no docstring-'
#         #return test_array
#
#     def VerifyArray(self, test_array):
#         '-no docstring-'
#         #return result
#

StructRecordParamTest._fields_ = [
    ('question', BSTR),
    ('answer', c_int),
    ('needs_clarification', VARIANT_BOOL),
]

assert sizeof(StructRecordParamTest) == 16, sizeof(StructRecordParamTest)
assert alignment(StructRecordParamTest) == 8, alignment(StructRecordParamTest)


class CoComtypesDispRecordParamTest(CoClass):
    """Comtypes component for dispinterface record parameter tests."""
    _reg_clsid_ = GUID('{5E78C9A8-4C19-4285-BCD6-3FFBBA5B17A8}')
    _idlflags_ = []
    _typelib_path_ = typelib_path
    _reg_typelib_ = ('{07D2AEE5-1DF8-4D2C-953A-554ADFD25F99}', 1, 0)


CoComtypesDispRecordParamTest._com_interfaces_ = [IDualRecordParamTest, IDispRecordParamTest]


class Library(object):
    """Comtypes C++ Test COM Server 1.0 Type Library."""
    name = 'ComtypesCppTestSrvLib'
    _reg_typelib_ = ('{07D2AEE5-1DF8-4D2C-953A-554ADFD25F99}', 1, 0)


IDispSafearrayParamTest._disp_methods_ = [
    DISPMETHOD(
        [dispid(1)],
        None,
        'InitArray',
        (['in', 'out'], POINTER(_midlSAFEARRAY(c_double)), 'test_array')
    ),
    DISPMETHOD(
        [dispid(2)],
        VARIANT_BOOL,
        'VerifyArray',
        (['in'], _midlSAFEARRAY(c_double), 'test_array')
    ),
]

__all__ = [
    'StructRecordParamTest', 'typelib_path', 'IDualRecordParamTest',
    'CoComtypesDispSafearrayParamTest', 'IDispRecordParamTest',
    'CoComtypesDispRecordParamTest', 'IDualSafearrayParamTest',
    'Library', 'IDispSafearrayParamTest'
]

_check_version('1.4.4', 1719240631.857339)

ComtypesCppTestSrvLib.py

friendly module

from enum import IntFlag

import comtypes.gen._07D2AEE5_1DF8_4D2C_953A_554ADFD25F99_0_1_0 as __wrapper_module__
from comtypes.gen._07D2AEE5_1DF8_4D2C_953A_554ADFD25F99_0_1_0 import (
    StructRecordParamTest, typelib_path, COMMETHOD, CoClass,
    CoComtypesDispSafearrayParamTest, IDispRecordParamTest, BSTR,
    _check_version, VARIANT_BOOL, DISPMETHOD, IDualRecordParamTest,
    WSTRING, CoComtypesDispRecordParamTest, _midlSAFEARRAY,
    IDualSafearrayParamTest, Library, IDispSafearrayParamTest, _lcid,
    HRESULT, dispid, GUID
)


__all__ = [
    'StructRecordParamTest', 'typelib_path', 'IDualRecordParamTest',
    'CoComtypesDispSafearrayParamTest', 'IDispRecordParamTest',
    'CoComtypesDispRecordParamTest', 'IDualSafearrayParamTest',
    'Library', 'IDispSafearrayParamTest'
]

Test that fails when (if) the changes to tagVARIANT._set_value are reverted

traceback

======================================================================
ERROR: test_in_safearray (test_dispifc_safearrays.Test_DispMethods) (expected=True, array_content=(0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0))
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\comtypes\comtypes\comtypes\test\test_dispifc_safearrays.py", line 81, in test_in_safearray
    self.assertEqual(self._create_dispifc().VerifyArray(sa), expected)
  File "D:\a\comtypes\comtypes\comtypes\_memberspec.py", line 495, in func
    return obj.Invoke(memid, _invkind=1, *args, **kw)  # DISPATCH_METHOD
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 899, in Invoke
    dp = self.__make_dp(_invkind, *args)
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 874, in __make_dp
    array[i].value = a
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 426, in _set_value
    self.vt = _ctype_to_vartype[type(ref)] | VT_BYREF
KeyError: <class 'comtypes.safearray.SAFEARRAY_c_double'>

======================================================================
ERROR: test_in_safearray (test_dispifc_safearrays.Test_DispMethods) (expected=False, array_content=(0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0))
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\comtypes\comtypes\comtypes\test\test_dispifc_safearrays.py", line 81, in test_in_safearray
    self.assertEqual(self._create_dispifc().VerifyArray(sa), expected)
  File "D:\a\comtypes\comtypes\comtypes\_memberspec.py", line 495, in func
    return obj.Invoke(memid, _invkind=1, *args, **kw)  # DISPATCH_METHOD
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 899, in Invoke
    dp = self.__make_dp(_invkind, *args)
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 874, in __make_dp
    array[i].value = a
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 426, in _set_value
    self.vt = _ctype_to_vartype[type(ref)] | VT_BYREF
KeyError: <class 'comtypes.safearray.SAFEARRAY_c_double'>

======================================================================
ERROR: test_inout_byref (test_dispifc_safearrays.Test_DispMethods)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\comtypes\comtypes\comtypes\test\test_dispifc_safearrays.py", line 45, in test_inout_byref
    dispifc.InitArray(byref(test_array))
  File "D:\a\comtypes\comtypes\comtypes\_memberspec.py", line 495, in func
    return obj.Invoke(memid, _invkind=1, *args, **kw)  # DISPATCH_METHOD
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 899, in Invoke
    dp = self.__make_dp(_invkind, *args)
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 874, in __make_dp
    array[i].value = a
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 409, in _set_value
    self.vt = _ctype_to_vartype[type(ref)] | VT_BYREF
KeyError: <class 'comtypes.safearray.LP_SAFEARRAY_c_double'>

======================================================================
ERROR: test_inout_pointer (test_dispifc_safearrays.Test_DispMethods)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\comtypes\comtypes\comtypes\test\test_dispifc_safearrays.py", line 57, in test_inout_pointer
    dispifc.InitArray(pointer(test_array))
  File "D:\a\comtypes\comtypes\comtypes\_memberspec.py", line 495, in func
    return obj.Invoke(memid, _invkind=1, *args, **kw)  # DISPATCH_METHOD
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 899, in Invoke
    dp = self.__make_dp(_invkind, *args)
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 874, in __make_dp
    array[i].value = a
  File "D:\a\comtypes\comtypes\comtypes\automation.py", line 426, in _set_value
    self.vt = _ctype_to_vartype[type(ref)] | VT_BYREF
KeyError: <class 'comtypes.safearray.LP_SAFEARRAY_c_double'>

----------------------------------------------------------------------

[junkmd]

I think there is no problem with the production codebase and the targets for assertions in tests.

What I would like to point out is about the naming parts.

For these tests, I would like to limit the ones named ...array... to those that are SAFEARRAY.
For concepts equivalent to "sequence"s or "collection"s other than that, I would like to use a name other than ...array....

[geppi]

Those name changes are fine with me.

Suggestions are applied.

[junkmd]

I correct my mistakes and confusions.

[geppi]

So I think this should be ready to be merged now.

[junkmd]

Thank you for your contribution.
Following #535, being able to see the development process when adding a new COM component to the existing COM library was very educational for me.

Here are my notes on tagSAFEARRAY and _midlSAFEARRAY:

• _midlSAFEARRAY is a kind of factory function that returns a pointer to a tagSAFEARRAY subclass, so it's not a type.
• Unlike in Implement record pointers as method parameters of a Dispatch Interface #535 where test_in_record was passing a record instance to the method instead of a record pointer, in Implement SAFEARRAY pointers and SAFEARRAYs as method parameters of a Dispatch Interface #569 test_in_safearray is not passing a tagSAFEARRAY instance as is, but a subclass pointer generated by _midlSAFEARRAY.
  However, since there are test_inout_pointer and test_inout_byref, I don't feel any discomfort with the name test_in_safearray.