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

```py # -*- 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

```py 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: ====================================================================== 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: ====================================================================== 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: ====================================================================== 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: ---------------------------------------------------------------------- ```

[geppi]

Those name changes are fine with me.

[geppi]

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