3d_party: Adapt 3rd party remove repo functions

Signed-off-by: Otavio Pontes <otavio.pontes@intel.com>

src/3rd_party_remove.c

@@ -18,22 +18,29 @@
  */
 
 #define _GNU_SOURCE
+#include "3rd_party_repos.h"
 #include "swupd.h"
 
+#include <errno.h>
+
 #ifdef THIRDPARTY
 
 static int remove_repo_directory(char *repo_name)
 {
 	char *repo_dir;
 	int ret = 0;
-	repo_dir = get_repo_path(repo_name);
-	if (is_dir(repo_dir)) {
-		ret = sys_rm_recursive(repo_dir);
-		if (ret) {
-			error("Failed to delete repository directory\n");
-		}
+
+	//TODO: use a global function to get this value
+	repo_dir = str_or_die("%s/%s/%s", globals.path_prefix, "opt/3rd_party", repo_name);
+	ret = sys_rm_recursive(repo_dir);
+	if (ret == -ENOENT) {
+		ret = 0;
 	}
-	free_string(&repo_dir);
+	if (ret < 0) {
+		error("Failed to delete repository directory\n");
+	}
+
+	free(repo_dir);
 	return ret;
 }
 
@@ -80,6 +87,7 @@ static bool parse_options(int argc, char **argv)
 enum swupd_code third_party_remove_main(int argc, char **argv)
 {
 	enum swupd_code ret = SWUPD_OK;
+	int err;
 	const int step_in_third_party_remove = 1;
 
 	if (!parse_options(argc, argv)) {
@@ -90,23 +98,29 @@ enum swupd_code third_party_remove_main(int argc, char **argv)
 
 	ret = swupd_init(SWUPD_NO_ROOT);
 	if (ret != SWUPD_OK) {
-		goto finish;
+		goto exit;
 	}
 
 	/* The last argument has to be the repo-name to be deleted */
-	if (remove_repo_from_config(argv[argc - 1]) == 0) {
-		if (remove_repo_directory(argv[argc - 1])) {
-			ret = SWUPD_NO;
+	err = third_party_remove_repo(argv[argc - 1]);
+	if (err < 0) {
+		if (err == -ENOENT) {
+			ret = SWUPD_INVALID_OPTION;
 		} else {
-			info("Repository %s and its contents removed successfully\n", argv[argc - 1]);
+			ret = SWUPD_COULDNT_WRITE_FILE;
 		}
-	} else {
-		ret = SWUPD_NO;
+		goto exit;
 	}
 
-	swupd_deinit();
+	if (remove_repo_directory(argv[argc - 1]) < 0) {
+		ret = SWUPD_COULDNT_REMOVE_FILE;
+		goto exit;
+	}
 
-finish:
+	info("Repository %s and its content removed successfully\n", argv[argc - 1]);
+
+exit:
+	swupd_deinit();
 	progress_finish_steps(ret);
 	return ret;
 }

src/3rd_party_repos.c

@@ -89,9 +89,11 @@ struct list *third_party_get_repos(void)
 
 void repo_free(struct repo *repo)
 {
-	free(repo->name);
-	free(repo->url);
-	free(repo->version);
+	if (repo) {
+		free(repo->name);
+		free(repo->url);
+		free(repo->version);
+	}
 	free(repo);
 }
 
@@ -142,7 +144,33 @@ exit:
 	return ret;
 }
 
-static int adjust_repo_config_file()
+static int write_repo(FILE *fp, struct repo *repo)
+{
+	int ret;
+
+	ret = config_write_section(fp, repo->name);
+	if (ret) {
+		return ret;
+	}
+
+	if (repo->url) {
+		ret = config_write_config(fp, "url", repo->url);
+		if (ret) {
+			return ret;
+		}
+	}
+
+	if (repo->version) {
+		ret = config_write_config(fp, "version", repo->version);
+		if (ret) {
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int overwrite_repo_config_file(struct list *repos)
 {
 	struct list *repo;
 	int ret = 0;
@@ -151,18 +179,19 @@ static int adjust_repo_config_file()
 
 	repo_config_file_path = get_repo_config_path();
 	// open it in trunc mode as we re-write all the contents again
-	fp = fopen(repo_config_file_path, "w+");
-	repo = list_head(repos);
+	fp = fopen(repo_config_file_path, "w");
+	if (!fp) {
+		error("Impossible to write to file %s", repo_config_file_path);
+		goto exit;
+	}
+
+	repo = repos;
 	while (repo) {
-		// If we are here, we are cleared to write to file
-		struct repo *repo_temp = repo->data;
-		ret = config_write_section(fp, repo_temp->name);
-		if (ret) {
+		ret = write_repo(fp, repo->data);
+		if (ret < 0) {
+			error("Write error when trying to write to %s", repo_config_file_path);
 			goto exit;
 		}
-
-		// write url
-		ret = config_write_config(fp, "url", repo_temp->url);
 		repo = repo->next;
 	}
 
@@ -174,54 +203,29 @@ exit:
 	return ret;
 }
 
-/**
- * @brief This callback function helps remove_repo perform an item by item comparision
- * from the repos list.
- *
- * @param repo_item Each repo_item retreived from a list
- * @param repo_name_find A string containing the repo_name the user passed
- *
- * @returns 0 on success ie: a repo is found, any other value on a mismatch to
- * move to next element in list.
- */
-int compare_repos(const void *repo_item, const void *repo_name_find)
-{
-	struct repo *repo = (struct repo *)repo_item;
-	char *repo_name = (char *)repo_name_find;
-	int ret = strncmp(repo->name, repo_name, strlen(repo->name));
-	if (!ret) {
-		info("Match found for repository: %s\n", repo_name);
-	}
-	return ret;
-}
-
-int remove_repo_from_config(char *repo_name)
+int third_party_remove_repo(char *repo_name)
 {
 	struct repo *repo;
-	struct list *iter;
+	struct list *repos;
 	int ret = 0;
 
-	if (repo_config_init()) {
-		ret = -1;
+	repos = third_party_get_repos();
+	repo = list_remove(repo_name, &repos, repo_name_cmp);
+
+	if (!repo) {
+		error("Repository not found\n");
+		ret = -ENOENT;
 		goto exit;
 	}
 
-	iter = list_head(repos);
-	repo = list_remove(repo_name, &iter, compare_repos);
-	repos = iter;
-
-	if (!repo) {
-		info("Repository not found\n");
-		ret = -1;
-	} else {
-		ret = adjust_repo_config_file();
-		if (ret) {
-			error("Failed while adjusting repository config file");
-		}
+	repo_free(repo);
+	ret = overwrite_repo_config_file(repos);
+	if (ret) {
+		error("Failed while adjusting repository config file");
 	}
 
 exit:
-	repo_config_deinit();
+	list_free_list_and_data(repos, repo_free_data);
 	return ret;
 }
 #endif

test/functional/3rd-party-repo/3rd-party-repo-remove-negative.bats

@@ -26,63 +26,57 @@ test_setup(){
 	)
 
 	repo_config_file="$STATEDIR"/3rd_party/repo.ini
-	run sudo sh -c "mkdir -p $STATEDIR/3rd_party/"
-	run sudo sh -c "mkdir -p $PATH_PREFIX/opt/3rd-party/{test1,test2,test3,test4,test5}"
 	write_to_protected_file -a "$repo_config_file" "$contents"
 
 }
 
 test_teardown(){
 
-	run sudo sh -c "rm -r $PATH_PREFIX/opt/3rd-party"
-	run sudo sh -c "rm -r $STATEDIR/3rd_party"
 	destroy_test_environment "$TEST_NAME"
 
 }
 
-@test "TRA007: Negative test, repo remove usage" {
+@test "TPR009: Negative test, repo remove usage" {
 
 	run sudo sh -c "$SWUPD 3rd-party remove $SWUPD_OPTS"
 	assert_status_is "$SWUPD_INVALID_OPTION"
 	expected_output=$(cat <<-EOM
 			Error: The positional args: repo-name is missing
-			*.*
 	EOM
 	)
-	assert_regex_in_output "$expected_output"
+	assert_in_output "$expected_output"
 
 	run sudo sh -c "$SWUPD 3rd-party remove test-repo1 junk_positional $SWUPD_OPTS"
 	assert_status_is "$SWUPD_INVALID_OPTION"
 	expected_output=$(cat <<-EOM
 			Error: Unexpected arguments
-			*.*
 	EOM
 	)
-	assert_regex_in_output "$expected_output"
+	assert_in_output "$expected_output"
 
 }
 
-@test "TRA008: Negative test, Remove a repo which does not exist" {
+@test "TPR010: Negative test, Remove a repo which does not exist" {
 
 	run sudo sh -c "$SWUPD 3rd-party remove test3 $SWUPD_OPTS"
-	assert_status_is "$SWUPD_NO"
+	assert_status_is "$SWUPD_INVALID_OPTION"
 	expected_output=$(cat <<-EOM
-			Repository not found
+			Error: Repository not found
 	EOM
 	)
 	assert_is_output "$expected_output"
 
 }
 
-@test "TRA009: Negative test, Remove a repo on a new system" {
+@test "TPR011: Negative test, Remove a repo on a new system" {
 
 	run sudo sh -c "rm -r $PATH_PREFIX/opt/3rd-party"
 	run sudo sh -c "rm -r $STATEDIR/3rd_party"
 
 	run sudo sh -c "$SWUPD 3rd-party remove test3 $SWUPD_OPTS"
-	assert_status_is "$SWUPD_NO"
+	assert_status_is "$SWUPD_INVALID_OPTION"
 	expected_output=$(cat <<-EOM
-			Repository not found
+			Error: Repository not found
 	EOM
 	)
 	assert_is_output "$expected_output"

test/functional/3rd-party-repo/3rd-party-repo-remove.bats

@@ -12,44 +12,43 @@ test_setup(){
 	contents=$(cat <<- EOM
 		\n
 		[test1]
-		\n
 		url=www.abc.com
-		\n
+		version=0
+
 		[test2]
-		\n
 		url=www.efg.com
-		\n
+		version=0
+
 		[test3]
-		\n
 		url=www.xyz.com
-		\n
+		version=0
+
 		[test4]
-		\n
 		url=www.pqr.com
-		\n
+		version=123
+		invalid=456
+
 		[test5]
-		\n
 		url=www.lmn.com
+		version=0
 		\n
 	EOM
 	)
 
 	repo_config_file="$STATEDIR"/3rd_party/repo.ini
 	run sudo sh -c "mkdir -p $STATEDIR/3rd_party/"
-	run sudo sh -c "mkdir -p $PATH_PREFIX/opt/3rd-party/{test1,test2,test3,test4,test5}"
+	run sudo sh -c "mkdir -p $PATH_PREFIX/opt/3rd_party/{test1,test2,test3,test4,test5}"
 	write_to_protected_file -a "$repo_config_file" "$contents"
 
 }
 
 test_teardown(){
 
-	run sudo sh -c "rm -r $PATH_PREFIX/opt/3rd-party"
-	run sudo sh -c "rm -r $STATEDIR/3rd_party"
 	destroy_test_environment "$TEST_NAME"
 
 }
 
-@test "TRA006: Remove a multiple repos" {
+@test "TPR008: Remove multiple repos" {
 
 	repo_config_file="$STATEDIR"/3rd_party/repo.ini
 
@@ -57,8 +56,7 @@ test_teardown(){
 	run sudo sh -c "$SWUPD 3rd-party remove test1 $SWUPD_OPTS"
 	assert_status_is "$SWUPD_OK"
 	expected_output=$(cat <<-EOM
-		Match found for repository: test1
-		Repository test1 and its contents removed successfully
+		Repository test1 and its content removed successfully
 	EOM
 	)
 	assert_is_output "$expected_output"
@@ -68,8 +66,7 @@ test_teardown(){
 	run sudo sh -c "$SWUPD 3rd-party remove test3 $SWUPD_OPTS"
 	assert_status_is "$SWUPD_OK"
 	expected_output=$(cat <<-EOM
-		Match found for repository: test3
-		Repository test3 and its contents removed successfully
+		Repository test3 and its content removed successfully
 	EOM
 	)
 	assert_is_output "$expected_output"
@@ -79,23 +76,20 @@ test_teardown(){
 	run sudo sh -c "$SWUPD 3rd-party remove test5 $SWUPD_OPTS"
 	assert_status_is "$SWUPD_OK"
 	expected_output=$(cat <<-EOM
-		Match found for repository: test5
-		Repository test5 and its contents removed successfully
+		Repository test5 and its content removed successfully
 	EOM
 	)
 	assert_is_output "$expected_output"
 	assert_dir_not_exists "$PATH_PREFIX/opt/3rd-party/test1"
 
 	expected_contents=$(cat <<- EOM
-
 		[test2]
-
 		url=www.efg.com
+		version=0
 
 		[test4]
-
 		url=www.pqr.com
-
+		version=123
 	EOM
 	)