[PATCH 0/3] MR8166: taskschd: Implements IRegisteredTaskCollection get_Count and get_Item
Implements IRegisteredTaskCollection get_Count and get_Item along with a test for both. When making the tests individually they were very similar so I merged them into one test function as per feedback.
From: Jacob Czekalla jczekalla@codeweavers.com
--- dlls/taskschd/tests/scheduler.c | 95 +++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)
diff --git a/dlls/taskschd/tests/scheduler.c b/dlls/taskschd/tests/scheduler.c index f9c209f1d84..d4256134766 100644 --- a/dlls/taskschd/tests/scheduler.c +++ b/dlls/taskschd/tests/scheduler.c @@ -1879,6 +1879,100 @@ static void test_TaskDefinition(void) ITaskService_Release(service); }
+static void test_get_Count_and_Item(void) +{ + HRESULT hr; + LONG num_tasks; + VARIANT v_null, index; + ITaskService *service; + ITaskFolder *folder, *root; + ITaskDefinition *task_def; + IRegistrationInfo *reg_info; + IActionCollection *actions; + IAction *action; + IExecAction *exec_action; + IRegisteredTask *task1, *ret_task1; + IRegisteredTaskCollection *tasks; + BSTR ret_task1_name = NULL; + + V_VT(&v_null) = VT_NULL; + + CoCreateInstance(&CLSID_TaskScheduler, NULL, CLSCTX_INPROC_SERVER, &IID_ITaskService, (void**)&service); + ITaskService_Connect(service, v_null, v_null, v_null, v_null); + + ITaskService_GetFolder(service, (BSTR)L"\", &root); + ITaskFolder_CreateFolder(root,(BSTR)L"\Wine", v_null, &folder); + + ITaskService_NewTask(service, 0, &task_def); + ITaskDefinition_get_RegistrationInfo(task_def, ®_info); + IRegistrationInfo_put_Author(reg_info, (BSTR)L"Wine Test"); + + ITaskDefinition_get_Actions(task_def, &actions); + IActionCollection_Create(actions, TASK_ACTION_EXEC, &action); + IAction_QueryInterface(action, &IID_IExecAction, (void**)&exec_action); + IExecAction_put_Path(exec_action, (BSTR)L"task1.exe"); + + ITaskFolder_RegisterTaskDefinition(folder, (BSTR)L"Task1", task_def, TASK_CREATE, v_null, v_null, TASK_LOGON_NONE, v_null, &task1); + ITaskFolder_GetTasks(folder, TASK_ENUM_HIDDEN, &tasks); + + /* Test get_Count */ + hr = IRegisteredTaskCollection_get_Count(tasks, NULL); + todo_wine + ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr); + + hr = IRegisteredTaskCollection_get_Count(tasks, &num_tasks); + todo_wine + ok(hr == S_OK, "expected S_OK, got %#lx\n", hr); + todo_wine + ok(num_tasks == 1, "expected 1 task, got %ld\n", num_tasks); + + /* Test get_Item */ + VariantInit(&index); + index.vt = VT_UI4; + index.uiVal = 1; + + hr = IRegisteredTaskCollection_get_Item(tasks, index, NULL); + todo_wine + ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr); + + index.uiVal = 0; + hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1); + todo_wine + ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr); + + index.uiVal = 2; + hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1); + todo_wine + ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr); + + index.uiVal = 1; + hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1); + todo_wine + ok(hr == S_OK, "expected S_OK, got %#lx\n", hr); + if (hr == S_OK) + { + IRegisteredTask_get_Name(ret_task1, &ret_task1_name); + if(ret_task1_name) + { + todo_wine + ok(!lstrcmpW(L"Task1", ret_task1_name), "expected name "Task1", got %ls\n", ret_task1_name); + } + else + { + todo_wine + ok(0, "expected name "Task1", got NULL\n"); + } + } + + ITaskFolder_DeleteTask(folder, (BSTR)L"Task1", 0); + ITaskFolder_DeleteFolder(root, (BSTR)L"\Wine", 0); + IRegisteredTask_Release(task1); + IExecAction_Release(exec_action); + IRegistrationInfo_Release(reg_info); + ITaskDefinition_Release(task_def); + ITaskService_Release(service); +} + START_TEST(scheduler) { OleInitialize(NULL); @@ -1888,6 +1982,7 @@ START_TEST(scheduler) test_FolderCollection(); test_GetTask(); test_TaskDefinition(); + test_get_Count_and_Item();
OleUninitialize(); }
From: Jacob Czekalla jczekalla@codeweavers.com
--- dlls/taskschd/regtask.c | 56 +++++++++++++++++++++++++++++++-- dlls/taskschd/tests/scheduler.c | 10 ------ 2 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/dlls/taskschd/regtask.c b/dlls/taskschd/regtask.c index bae0d4cb0b2..744f584b5a1 100644 --- a/dlls/taskschd/regtask.c +++ b/dlls/taskschd/regtask.c @@ -471,8 +471,60 @@ static HRESULT WINAPI regtasks_get_Count(IRegisteredTaskCollection *iface, LONG
static HRESULT WINAPI regtasks_get_Item(IRegisteredTaskCollection *iface, VARIANT index, IRegisteredTask **regtask) { - FIXME("%p,%s,%p: stub\n", iface, debugstr_variant(&index), regtask); - return E_NOTIMPL; + HRESULT hr; + WCHAR *xml = NULL; + VARIANT converted_index; + TASK_NAMES task_names = NULL; + ITaskDefinition *definition = NULL; + RegisteredTaskCollection *collection = NULL; + DWORD start_index = 0, num_tasks = 0; + + collection = impl_from_IRegisteredTaskCollection(iface); + if (!regtask) + return E_POINTER; + *regtask = NULL; + + VariantInit(&converted_index); + hr = VariantChangeType(&converted_index, &index, 0, VT_UI4); + if (FAILED(hr)) + return hr; + + start_index = V_UI4(&index); + if (start_index == 0) + { + VariantClear(&converted_index); + return E_INVALIDARG; + } + start_index -= 1; + + hr = SchRpcEnumTasks(collection->path, 0, &start_index, 1, &num_tasks, &task_names); + if (FAILED(hr)) + { + VariantClear(&converted_index); + return hr; + } + if (!task_names) + { + VariantClear(&converted_index); + return E_INVALIDARG; + } + + hr = TaskDefinition_create(&definition); + if (FAILED(hr)) + goto cleanup; + hr = RegisteredTask_create(collection->path, task_names[0], definition, TASK_VALIDATE_ONLY, TASK_LOGON_INTERACTIVE_TOKEN, regtask, FALSE); + if (FAILED(hr)) + goto cleanup; + + hr = S_OK; +cleanup: + MIDL_user_free(task_names[0]); + MIDL_user_free(task_names); + if (xml) + SysFreeString(xml); + VariantClear(&converted_index); + + return hr; }
static HRESULT WINAPI regtasks_get__NewEnum(IRegisteredTaskCollection *iface, IUnknown **penum) diff --git a/dlls/taskschd/tests/scheduler.c b/dlls/taskschd/tests/scheduler.c index d4256134766..b09fa8216ef 100644 --- a/dlls/taskschd/tests/scheduler.c +++ b/dlls/taskschd/tests/scheduler.c @@ -1932,36 +1932,26 @@ static void test_get_Count_and_Item(void) index.uiVal = 1;
hr = IRegisteredTaskCollection_get_Item(tasks, index, NULL); - todo_wine ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr);
index.uiVal = 0; hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1); - todo_wine ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr);
index.uiVal = 2; hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1); - todo_wine ok(hr == E_INVALIDARG, "expected E_INVALIDARG, got %#lx\n", hr);
index.uiVal = 1; hr = IRegisteredTaskCollection_get_Item(tasks, index, &ret_task1); - todo_wine ok(hr == S_OK, "expected S_OK, got %#lx\n", hr); if (hr == S_OK) { IRegisteredTask_get_Name(ret_task1, &ret_task1_name); if(ret_task1_name) - { - todo_wine ok(!lstrcmpW(L"Task1", ret_task1_name), "expected name "Task1", got %ls\n", ret_task1_name); - } else - { - todo_wine ok(0, "expected name "Task1", got NULL\n"); - } }
ITaskFolder_DeleteTask(folder, (BSTR)L"Task1", 0);
From: Jacob Czekalla jczekalla@codeweavers.com
--- dlls/taskschd/regtask.c | 25 ++++++++++++++++++++++--- dlls/taskschd/tests/scheduler.c | 3 --- 2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/dlls/taskschd/regtask.c b/dlls/taskschd/regtask.c index 744f584b5a1..2239809b1ca 100644 --- a/dlls/taskschd/regtask.c +++ b/dlls/taskschd/regtask.c @@ -464,9 +464,28 @@ static HRESULT WINAPI regtasks_Invoke(IRegisteredTaskCollection *iface, DISPID d
static HRESULT WINAPI regtasks_get_Count(IRegisteredTaskCollection *iface, LONG *count) { - FIXME("%p,%p: stub\n", iface, count); - if (count) *count = 0; - return E_NOTIMPL; + HRESULT hr; + RegisteredTaskCollection *reg_tasks = impl_from_IRegisteredTaskCollection(iface); + TASK_NAMES task_names = NULL; + DWORD start_index = 0, num_tasks = 0, i = 0; + + if (!count) + return E_POINTER; + + hr = SchRpcEnumTasks(reg_tasks->path, 0, &start_index, 0, &num_tasks, &task_names); + if (FAILED(hr)) + { + *count = 0; + return hr; + } + + *count = num_tasks; + + for (;i < num_tasks; i++) + MIDL_user_free(task_names[i]); + MIDL_user_free(task_names); + + return S_OK; }
static HRESULT WINAPI regtasks_get_Item(IRegisteredTaskCollection *iface, VARIANT index, IRegisteredTask **regtask) diff --git a/dlls/taskschd/tests/scheduler.c b/dlls/taskschd/tests/scheduler.c index b09fa8216ef..63f8fa786e7 100644 --- a/dlls/taskschd/tests/scheduler.c +++ b/dlls/taskschd/tests/scheduler.c @@ -1917,13 +1917,10 @@ static void test_get_Count_and_Item(void)
/* Test get_Count */ hr = IRegisteredTaskCollection_get_Count(tasks, NULL); - todo_wine ok(hr == E_POINTER, "expected E_POINTER, got %#lx\n", hr);
hr = IRegisteredTaskCollection_get_Count(tasks, &num_tasks); - todo_wine ok(hr == S_OK, "expected S_OK, got %#lx\n", hr); - todo_wine ok(num_tasks == 1, "expected 1 task, got %ld\n", num_tasks);
/* Test get_Item */
Alfred Agrell (@Alcaro) commented about dlls/taskschd/regtask.c:
Unused variable, mind deleting it?
Alfred Agrell (@Alcaro) commented about dlls/taskschd/regtask.c:
The other RegisteredTaskCollection* is initialized in the declaration section. I have no particular opinion in either direction, but better pick one.
Alfred Agrell (@Alcaro) commented about dlls/taskschd/regtask.c:
Other RegisteredTask_create callers Release the definition object if hr != S_OK.
Alfred Agrell (@Alcaro) commented about dlls/taskschd/tests/scheduler.c:
If the ok() passes on everything, then the if is unnecessary. (And if it doesn't, you should add a broken() or win_skip() somewhere.)
Alfred Agrell (@Alcaro) commented about dlls/taskschd/tests/scheduler.c:
Are these two objects leaked?
Good start. Functionality looks good to me (I didn't test it, but the added tests look pretty comprehensive), just a few mem leaks and other oddities.
On Wed May 28 13:59:15 2025 +0000, Alfred Agrell wrote:
If the ok() passes on everything, then the if is unnecessary. (And if it doesn't, you should add a broken() or win_skip() somewhere.)
Good point. I am also not testing other functions so I should assume they work as intended, therefore I will get rid of my other if statement below that as well.
-
Alfred Agrell (＠Alcaro) -
Jacob Czekalla
-
Jacob Czekalla (＠JacobCzekalla)