diff --git a/cf-agent/simulate_mode.c b/cf-agent/simulate_mode.c index 35264e62cc2..2b1d2995ab7 100644 --- a/cf-agent/simulate_mode.c +++ b/cf-agent/simulate_mode.c @@ -793,6 +793,31 @@ static inline char *GetPkgOperationMsg(ChrootPkgOperationCode op, const char *pk return msg; } +static bool PkgRecordShouldBeReplaced(const PkgOperationRecord *prev_record, + const char *new_ver) +{ + if (prev_record == NULL) + { + /* Nothing recorded yet */ + return true; + } + + if (NULL_OR_EMPTY(new_ver)) + { + /* An unversioned record never replaces an existing one */ + return false; + } + + if (NULL_OR_EMPTY(prev_record->pkg_ver)) + { + /* A versioned record is more specific than an unversioned one */ + return true; + } + + /* Both are versioned, keep the higher version */ + return PkgVersionIsGreater(new_ver, prev_record->pkg_ver); +} + bool DiffPkgOperations() { const char *pkgs_ops_csv_file = ToChangesChroot(CHROOT_PKGS_OPS_FILE); @@ -893,7 +918,7 @@ bool DiffPkgOperations() MapRemove(removed, name_arch); PkgOperationRecord *prev_record = MapGet(installed, name_arch); - if ((prev_record == NULL) || PkgVersionIsGreater(pkg_ver, prev_record->pkg_ver)) + if (PkgRecordShouldBeReplaced(prev_record, pkg_ver)) { char *msg = GetPkgOperationMsg(CHROOT_PKG_OPERATION_CODE_INSTALL, pkg_name, pkg_arch, pkg_ver); diff --git a/tests/unit/simulate_mode_test.c b/tests/unit/simulate_mode_test.c index 10b5513121a..9aa8cc01a1e 100644 --- a/tests/unit/simulate_mode_test.c +++ b/tests/unit/simulate_mode_test.c @@ -82,8 +82,8 @@ static void test_diff_install_cancels_removal(void) char output[4096]; assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); - assert_true(strstr(output, "Package 'foo [1.2.3]' would be installed") != NULL); - assert_true(strstr(output, "would be removed") == NULL); + assert_true(StringContains(output, "Package 'foo [1.2.3]' would be installed")); + assert_false(StringContains(output, "would be removed")); unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); } @@ -98,7 +98,7 @@ static void test_diff_version_and_architecture(void) char output[4096]; assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); - assert_true(strstr(output, "Package 'foo-x86_64 [1.2.3]' would be installed") != NULL); + assert_true(StringContains(output, "Package 'foo-x86_64 [1.2.3]' would be installed")); unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); } @@ -141,8 +141,106 @@ static void test_manifest_install_cancels_removal(void) char output[4096]; assert_true(call_with_captured_stdout(&ManifestPkgOperations, output, sizeof(output))); - assert_true(strstr(output, "Package 'foo [1.2.3]' would be present") != NULL); - assert_true(strstr(output, "would be absent") == NULL); + assert_true(StringContains(output, "Package 'foo [1.2.3]' would be present")); + assert_false(StringContains(output, "would be absent")); + + unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); +} + +/* Two recorded installations of the same package without a version must be + * reported once, not abort on comparing the empty versions. */ +static void test_diff_repeated_unversioned_install(void) +{ + write_pkgs_ops("i,foo,,\r\n" + "i,foo,,\r\n"); + + char output[4096]; + assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); + + assert_true(StringContains(output, "Package 'foo' would be installed")); + /* Reported once: the report has no other line. */ + assert_int_equal(StringCountTokens(output, strlen(output), "\n"), 1); + + unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); +} + +/* A versioned installation record is more specific than an unversioned one + * of the same package, so it is the one to report. */ +static void test_diff_unversioned_then_versioned_install(void) +{ + write_pkgs_ops("i,foo,,\r\n" + "i,foo,1.2.3,\r\n"); + + char output[4096]; + assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); + + assert_true(StringContains(output, "Package 'foo [1.2.3]' would be installed")); + assert_false(StringContains(output, "Package 'foo' would be installed")); + + unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); +} + +/* An unversioned installation record must never replace an existing + * versioned installation record of the same package. */ +static void test_diff_versioned_then_unversioned_install(void) +{ + write_pkgs_ops("i,foo,1.2.3,\r\n" + "i,foo,,\r\n"); + + char output[4096]; + assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); + + assert_true(StringContains(output, "Package 'foo [1.2.3]' would be installed")); + assert_false(StringContains(output, "Package 'foo' would be installed")); + + unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); +} + +/* When two installations are recorded with different versions and the first + * has the higher version, the first record is kept. */ +static void test_diff_install_higher_version_first_keeps_first(void) +{ + write_pkgs_ops("i,foo,2.0.0,\r\n" + "i,foo,1.0.0,\r\n"); + + char output[4096]; + assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); + + assert_true(StringContains(output, "Package 'foo [2.0.0]' would be installed")); + assert_false(StringContains(output, "Package 'foo [1.0.0]' would be installed")); + + unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); +} + +/* When two installations are recorded with different versions and the last + * has the higher version, the last record replaces the first. */ +static void test_diff_install_higher_version_last_keeps_last(void) +{ + write_pkgs_ops("i,foo,1.0.0,\r\n" + "i,foo,2.0.0,\r\n"); + + char output[4096]; + assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); + + assert_true(StringContains(output, "Package 'foo [2.0.0]' would be installed")); + assert_false(StringContains(output, "Package 'foo [1.0.0]' would be installed")); + + unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); +} + +/* Two recorded installations of the same package with the same version + * must be reported once. */ +static void test_diff_repeated_versioned_install(void) +{ + write_pkgs_ops("i,foo,1.2.3,\r\n" + "i,foo,1.2.3,\r\n"); + + char output[4096]; + assert_true(call_with_captured_stdout(&DiffPkgOperations, output, sizeof(output))); + + assert_true(StringContains(output, "Package 'foo [1.2.3]' would be installed")); + /* Reported once: the report has no other line. */ + assert_int_equal(StringCountTokens(output, strlen(output), "\n"), 1); unlink(ToChangesChroot(CHROOT_PKGS_OPS_FILE)); } @@ -161,6 +259,12 @@ int main() unit_test(test_diff_removal_with_version), unit_test(test_diff_removal_with_other_version_keeps_install), unit_test(test_manifest_install_cancels_removal), + unit_test(test_diff_repeated_unversioned_install), + unit_test(test_diff_unversioned_then_versioned_install), + unit_test(test_diff_versioned_then_unversioned_install), + unit_test(test_diff_install_higher_version_first_keeps_first), + unit_test(test_diff_install_higher_version_last_keeps_last), + unit_test(test_diff_repeated_versioned_install), }; int ret = run_tests(tests);