[PATCH v3 0/2] MR6827: msi: Fix MsiSummaryInfoPersist not saving data to disk

[Fabian Maurer (＠DarkShadow44)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55859

When a db is transacted, we need to call `Commit` to actually save the changes, but tests show this is not how msi works, so I changed `MsiGetSummaryInformationW` to open the db directly instead.

--
  v3: msi: Make MsiGetSummaryInformationW open database as direct instead of transacted
      msi: Add more tests for MsiSummaryInfoPersist

https://gitlab.winehq.org/wine/wine/-/merge_requests/6827

[PATCH v3 1/2] msi: Add more tests for MsiSummaryInfoPersist

[Fabian Maurer]

From: Fabian Maurer

---
 dlls/msi/tests/suminfo.c | 147 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/dlls/msi/tests/suminfo.c b/dlls/msi/tests/suminfo.c
index 59b56a2cbba..753f8bb53e5 100644
--- a/dlls/msi/tests/suminfo.c
+++ b/dlls/msi/tests/suminfo.c
@@ -278,6 +278,153 @@ static void test_suminfo(void)
     r = MsiCloseHandle(hsuminfo);
     ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");

+    /* try persisting on transacted msi */
+    r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_TRANSACT, &hdb);
+    ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n");
+
+    r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n");
+
+    r = MsiSummaryInfoSetPropertyA(hsuminfo, PID_AUTHOR, VT_LPSTR, 1, &ft, "Fabian");
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoSetProperty wrong error\n");
+
+    r = MsiSummaryInfoPersist(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoPersist wrong error %u\n", r);
+
+    r = MsiCloseHandle(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiCloseHandle(hdb);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb);
+    ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n");
+
+    r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n");
+
+    sz = 0x10;
+    strcpy(buf,"x");
+    r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz);
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n");
+    ok(!strcmp(buf,"Mike"), "buffer was wrong: %s\n", buf);
+
+    r = MsiCloseHandle(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiCloseHandle(hdb);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    /* try persisting on transacted msi with commit */
+    r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_TRANSACT, &hdb);
+    ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n");
+
+    r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n");
+
+    r = MsiSummaryInfoSetPropertyA(hsuminfo, PID_AUTHOR, VT_LPSTR, 1, &ft, "Fabian");
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoSetProperty wrong error\n");
+
+    r = MsiSummaryInfoPersist(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoPersist wrong error %u\n", r);
+
+    r = MsiCloseHandle(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiDatabaseCommit(hdb);
+    ok(r == ERROR_SUCCESS, "MsiDatabaseCommit wrong error %u\n", r);
+
+    r = MsiCloseHandle(hdb);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb);
+    ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n");
+
+    r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n");
+
+    sz = 0x10;
+    strcpy(buf,"x");
+    r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz);
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n");
+    ok(!strcmp(buf,"Fabian"), "buffer was wrong: %s\n", buf);
+
+    r = MsiCloseHandle(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiCloseHandle(hdb);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    /* try persisting on direct msi */
+    r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_DIRECT, &hdb);
+    ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n");
+
+    r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n");
+
+    r = MsiSummaryInfoSetPropertyA(hsuminfo, PID_AUTHOR, VT_LPSTR, 1, &ft, "Fabian2");
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoSetProperty wrong error\n");
+
+    r = MsiSummaryInfoPersist(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoPersist wrong error %u\n", r);
+
+    r = MsiCloseHandle(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiCloseHandle(hdb);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb);
+    ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n");
+
+    r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n");
+
+    sz = 0x10;
+    strcpy(buf,"x");
+    r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz);
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n");
+    ok(!strcmp(buf,"Fabian2"), "buffer was wrong: %s\n", buf);
+
+    r = MsiCloseHandle(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiCloseHandle(hdb);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    /* try persisting on indirectly opened msi */
+    r = MsiGetSummaryInformationA(0, msifile, 2, &hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error %u\n", r);
+
+    r = MsiSummaryInfoSetPropertyA(hsuminfo, PID_AUTHOR, VT_LPSTR, 1, &ft, "Fabian3");
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoSetProperty wrong error\n");
+
+    r = MsiSummaryInfoPersist(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoPersist wrong error %u\n", r);
+
+    r = MsiCloseHandle(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiOpenDatabaseW(msifileW, MSIDBOPEN_READONLY, &hdb);
+    ok(r == ERROR_SUCCESS, "MsiOpenDatabase failed\n");
+
+    r = MsiGetSummaryInformationA(hdb, NULL, 1, &hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiGetSummaryInformation wrong error\n");
+
+    sz = 0x10;
+    strcpy(buf,"x");
+    r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz);
+    ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n");
+    todo_wine
+    ok(!strcmp(buf,"Fabian3"), "buffer was wrong: %s\n", buf);
+
+    r = MsiCloseHandle(hsuminfo);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    r = MsiCloseHandle(hdb);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
+    /* Cleanup */
     r = DeleteFileA(msifile);
     ok(r, "DeleteFile failed\n");
 }

[PATCH v3 2/2] msi: Make MsiGetSummaryInformationW open database as direct instead of transacted

[Fabian Maurer]

From: Fabian Maurer

This fixes MsiSummaryInfoPersist not saving data to disk

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55859
---
 dlls/msi/suminfo.c       | 2 +-
 dlls/msi/tests/suminfo.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/dlls/msi/suminfo.c b/dlls/msi/suminfo.c
index b1934cf41c7..8670904c59b 100644
--- a/dlls/msi/suminfo.c
+++ b/dlls/msi/suminfo.c
@@ -515,7 +515,7 @@ UINT WINAPI MsiGetSummaryInformationW( MSIHANDLE hDatabase, const WCHAR *szDatab

if( szDatabase && szDatabase[0] )
     {
-        LPCWSTR persist = uiUpdateCount ? MSIDBOPEN_TRANSACT : MSIDBOPEN_READONLY;
+        LPCWSTR persist = uiUpdateCount ? MSIDBOPEN_DIRECT : MSIDBOPEN_READONLY;

ret = MSI_OpenDatabaseW( szDatabase, persist, &db );
         if( ret != ERROR_SUCCESS )
diff --git a/dlls/msi/tests/suminfo.c b/dlls/msi/tests/suminfo.c
index 753f8bb53e5..42e68c16d84 100644
--- a/dlls/msi/tests/suminfo.c
+++ b/dlls/msi/tests/suminfo.c
@@ -415,7 +415,6 @@ static void test_suminfo(void)
     strcpy(buf,"x");
     r = MsiSummaryInfoGetPropertyA(hsuminfo, PID_AUTHOR, &type, NULL, NULL, buf, &sz);
     ok(r == ERROR_SUCCESS, "MsiSummaryInfoGetPropertyA wrong error\n");
-    todo_wine
     ok(!strcmp(buf,"Fabian3"), "buffer was wrong: %s\n", buf);

r = MsiCloseHandle(hsuminfo);

[PATCH v3 2/2] msi: Make MsiGetSummaryInformationW open database as direct instead of transacted

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=149761

Your paranoid android.

=== debian11b (64 bit WoW report) ===

user32:
input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000019D00DA, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
win.c:4070: Test failed: Expected active window 0000000005710150, got 0000000000000000.
win.c:4071: Test failed: Expected focus window 0000000005710150, got 0000000000000000.

[Fabian Maurer (＠DarkShadow44)]

On Tue Nov 19 08:23:09 2024 +0000, Hans Leidekker wrote:
> Your test shows that calling MsiSummaryInfoPersist() has no effect if
> the database is transacted. That makes sense because changes should only
> be stored when MsiDatabaseCommit() is called. It would be nice if you
> could add a test that shows this. Then the commit message 'Fix
> MsiSummaryInfoPersist not saving data to disk' should be more specific.
> Your patch only handles the case where MsiGetSummaryInformationW() opens
> the database.

Thanks, changed the commit message and added another test using MsiDatabaseCommit to persistent the changes using a transacted database.

[PATCH v3 0/2] MR6827: msi: Fix MsiSummaryInfoPersist not saving data to disk - approved

[Hans Leidekker (＠hans)]

This merge request was approved by Hans Leidekker.