api: fix POST and PUT parameter passing

In general, these need to pass parameters via the body, not the URL.
This matches the old code using `.form()` for the parameters of a `POST`
or `PUT` query.

Fixes: #41

CHANGELOG.md

 # v0.1300.0 (unreleased)
 
+## New request body handling
+
+It was observed (#41) that the new API pattern was not handling `POST` and
+`PUT` parameters properly. This has now been fixed.
+
 ## Additional merge status cases
 
 Some additional merge status names for merge requests were missing and have

src/api/common.rs

@@ -14,6 +14,8 @@ use std::fmt;
 
 use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS};
 
+use crate::api::ParamValue;
+
 /// Access levels for groups and projects.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 pub enum AccessLevel {
@@ -105,6 +107,12 @@ impl EnableState {
     }
 }
 
+impl ParamValue<'static> for EnableState {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// A strucutre for storing a name or ID where either is allowed.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum NameOrId<'a> {
@@ -184,6 +192,12 @@ impl VisibilityLevel {
     }
 }
 
+impl ParamValue<'static> for VisibilityLevel {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// The string representation of booleans for GitLab.
 pub fn bool_str(b: bool) -> &'static str {
     if b {

src/api/groups/create.rs

@@ -6,8 +6,9 @@
 
 use derive_builder::Builder;
 
-use crate::api::common::{self, VisibilityLevel};
+use crate::api::common::VisibilityLevel;
 use crate::api::endpoint_prelude::*;
+use crate::api::ParamValue;
 
 /// Access levels for creating a project within a group.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -30,6 +31,12 @@ impl GroupProjectCreationAccessLevel {
     }
 }
 
+impl ParamValue<'static> for GroupProjectCreationAccessLevel {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// Access levels for creating a subgroup within a group.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum SubgroupCreationAccessLevel {
@@ -48,6 +55,12 @@ impl SubgroupCreationAccessLevel {
     }
 }
 
+impl ParamValue<'static> for SubgroupCreationAccessLevel {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// Branch protection rules for groups.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum BranchProtection {
@@ -69,6 +82,12 @@ impl BranchProtection {
     }
 }
 
+impl ParamValue<'static> for BranchProtection {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// Create a new group on an instance.
 #[derive(Debug, Builder)]
 #[builder(setter(strip_option))]
@@ -151,47 +170,40 @@ impl<'a> Endpoint for CreateGroup<'a> {
         "groups".into()
     }
 
-    fn add_parameters(&self, mut pairs: Pairs) {
-        pairs.append_pair("name", &self.name);
-        pairs.append_pair("path", &self.path);
+    fn body(&self) -> Result<Option<(&'static str, Vec<u8>)>, BodyError> {
+        let mut params = FormParams::default();
+
+        params
+            .push("name", &self.name)
+            .push("path", &self.path)
+            .push_opt("description", self.description.as_ref())
+            .push_opt("membership_lock", self.membership_lock)
+            .push_opt("visibility", self.visibility)
+            .push_opt("share_with_group_lock", self.share_with_group_lock)
+            .push_opt(
+                "require_two_factor_authentication",
+                self.require_two_factor_authentication,
+            )
+            .push_opt("two_factor_grace_period", self.two_factor_grace_period)
+            .push_opt("project_creation_level", self.project_creation_level)
+            .push_opt("auto_devops_enabled", self.auto_devops_enabled)
+            .push_opt("subgroup_creation_level", self.subgroup_creation_level)
+            .push_opt("emails_disabled", self.emails_disabled)
+            .push_opt("mentions_disabled", self.mentions_disabled)
+            .push_opt("lfs_enabled", self.lfs_enabled)
+            .push_opt("request_access_enabled", self.request_access_enabled)
+            .push_opt("parent_id", self.parent_id)
+            .push_opt("default_branch_protection", self.default_branch_protection)
+            .push_opt(
+                "shared_runners_minutes_limit",
+                self.shared_runners_minutes_limit,
+            )
+            .push_opt(
+                "extra_shared_runners_minutes_limit",
+                self.extra_shared_runners_minutes_limit,
+            );
 
-        self.description
-            .as_ref()
-            .map(|value| pairs.append_pair("description", value));
-        self.membership_lock
-            .map(|value| pairs.append_pair("membership_lock", common::bool_str(value)));
-        self.visibility
-            .map(|value| pairs.append_pair("visibility", value.as_str()));
-        self.share_with_group_lock
-            .map(|value| pairs.append_pair("share_with_group_lock", common::bool_str(value)));
-        self.require_two_factor_authentication.map(|value| {
-            pairs.append_pair("require_two_factor_authentication", common::bool_str(value))
-        });
-        self.two_factor_grace_period
-            .map(|value| pairs.append_pair("two_factor_grace_period", &format!("{}", value)));
-        self.project_creation_level
-            .map(|value| pairs.append_pair("project_creation_level", value.as_str()));
-        self.auto_devops_enabled
-            .map(|value| pairs.append_pair("auto_devops_enabled", common::bool_str(value)));
-        self.subgroup_creation_level
-            .map(|value| pairs.append_pair("subgroup_creation_level", value.as_str()));
-        self.emails_disabled
-            .map(|value| pairs.append_pair("emails_disabled", common::bool_str(value)));
-        self.mentions_disabled
-            .map(|value| pairs.append_pair("mentions_disabled", common::bool_str(value)));
-        self.lfs_enabled
-            .map(|value| pairs.append_pair("lfs_enabled", common::bool_str(value)));
-        self.request_access_enabled
-            .map(|value| pairs.append_pair("request_access_enabled", common::bool_str(value)));
-        self.parent_id
-            .map(|value| pairs.append_pair("parent_id", &format!("{}", value)));
-        self.default_branch_protection
-            .map(|value| pairs.append_pair("default_branch_protection", value.as_str()));
-        self.shared_runners_minutes_limit
-            .map(|value| pairs.append_pair("shared_runners_minutes_limit", &format!("{}", value)));
-        self.extra_shared_runners_minutes_limit.map(|value| {
-            pairs.append_pair("extra_shared_runners_minutes_limit", &format!("{}", value))
-        });
+        params.into_body()
     }
 }
 

src/api/projects/create.rs

@@ -9,8 +9,9 @@ use std::collections::HashSet;
 
 use derive_builder::Builder;
 
-use crate::api::common::{self, EnableState, VisibilityLevel};
+use crate::api::common::{EnableState, VisibilityLevel};
 use crate::api::endpoint_prelude::*;
+use crate::api::ParamValue;
 
 /// Access levels available for most features.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -34,6 +35,12 @@ impl FeatureAccessLevel {
     }
 }
 
+impl ParamValue<'static> for FeatureAccessLevel {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// Access levels available for features.
 ///
 /// Note that only the `pages` feature currently uses this.
@@ -61,6 +68,12 @@ impl FeatureAccessLevelPublic {
     }
 }
 
+impl ParamValue<'static> for FeatureAccessLevelPublic {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// How often the container expiration policy is applied.
 ///
 /// Note that GitLab only supports a few discrete values for this setting.
@@ -91,6 +104,12 @@ impl ContainerExpirationCadence {
     }
 }
 
+impl ParamValue<'static> for ContainerExpirationCadence {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// How many container instances to keep around.
 ///
 /// Note that GitLab only supports a few discrete values for this setting.
@@ -124,6 +143,12 @@ impl ContainerExpirationKeepN {
     }
 }
 
+impl ParamValue<'static> for ContainerExpirationKeepN {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// How old containers need to be before they are candidates for expiration.
 ///
 /// Note that GitLab only supports a few discrete values for this setting.
@@ -151,6 +176,12 @@ impl ContainerExpirationOlderThan {
     }
 }
 
+impl ParamValue<'static> for ContainerExpirationOlderThan {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// The expiration policies for container images attached to the project.
 #[derive(Debug, Clone, Builder)]
 #[builder(setter(strip_option))]
@@ -181,34 +212,28 @@ impl<'a> ContainerExpirationPolicy<'a> {
         ContainerExpirationPolicyBuilder::default()
     }
 
-    pub(crate) fn add_query(&self, pairs: &mut Pairs) {
-        self.cadence.map(|value| {
-            pairs.append_pair(
+    pub(crate) fn add_query<'b>(&'b self, params: &mut FormParams<'b>) {
+        params
+            .push_opt(
                 "container_expiration_policy_attributes[cadence]",
-                value.as_str(),
+                self.cadence,
             )
-        });
-        self.enabled.map(|value| {
-            pairs.append_pair(
+            .push_opt(
                 "container_expiration_policy_attributes[enabled]",
-                common::bool_str(value),
+                self.enabled,
             )
-        });
-        self.keep_n.map(|value| {
-            pairs.append_pair(
+            .push_opt(
                 "container_expiration_policy_attributes[keep_n]",
-                value.as_str(),
+                self.keep_n,
             )
-        });
-        self.older_than.map(|value| {
-            pairs.append_pair(
+            .push_opt(
                 "container_expiration_policy_attributes[older_than]",
-                value.as_str(),
+                self.older_than,
             )
-        });
-        self.name_regex.as_ref().map(|value| {
-            pairs.append_pair("container_expiration_policy_attributes[name_regex]", value)
-        });
+            .push_opt(
+                "container_expiration_policy_attributes[name_regex]",
+                self.name_regex.as_ref(),
+            );
     }
 }
 
@@ -234,6 +259,12 @@ impl AutoDevOpsDeployStrategy {
     }
 }
 
+impl ParamValue<'static> for AutoDevOpsDeployStrategy {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// How merge requests should be merged when using the "Merge" button.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum MergeMethod {
@@ -256,6 +287,12 @@ impl MergeMethod {
     }
 }
 
+impl ParamValue<'static> for MergeMethod {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// The default Git strategy for CI jobs.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum BuildGitStrategy {
@@ -284,6 +321,12 @@ impl BuildGitStrategy {
     }
 }
 
+impl ParamValue<'static> for BuildGitStrategy {
+    fn as_value(self) -> Cow<'static, str> {
+        self.as_str().into()
+    }
+}
+
 /// A structure to handle the fact that at least one of the name and path is required.
 #[derive(Debug, Clone)]
 enum ProjectName<'a> {
@@ -621,159 +664,126 @@ impl<'a> Endpoint for CreateProject<'a> {
         "projects".into()
     }
 
-    fn add_parameters(&self, mut pairs: Pairs) {
+    fn body(&self) -> Result<Option<(&'static str, Vec<u8>)>, BodyError> {
+        let mut params = FormParams::default();
+
         match &self.name_and_path {
             ProjectName::Name {
                 name,
             } => {
-                pairs.append_pair("name", &name);
+                params.push("name", name);
             },
             ProjectName::Path {
                 path,
             } => {
-                pairs.append_pair("path", &path);
+                params.push("path", path);
             },
             ProjectName::NameAndPath {
                 name,
                 path,
             } => {
-                pairs.append_pair("name", &name);
-                pairs.append_pair("path", &path);
+                params.push("name", name).push("path", path);
             },
         }
 
-        self.namespace_id
-            .map(|value| pairs.append_pair("namespace_id", &format!("{}", value)));
-        self.default_branch
-            .as_ref()
-            .map(|value| pairs.append_pair("default_branch", value));
-        self.description
-            .as_ref()
-            .map(|value| pairs.append_pair("description", value));
-
-        self.issues_access_level
-            .map(|value| pairs.append_pair("issues_access_level", value.as_str()));
-        self.repository_access_level
-            .map(|value| pairs.append_pair("repository_access_level", value.as_str()));
-        self.merge_requests_access_level
-            .map(|value| pairs.append_pair("merge_requests_access_level", value.as_str()));
-        self.forking_access_level
-            .map(|value| pairs.append_pair("forking_access_level", value.as_str()));
-        self.builds_access_level
-            .map(|value| pairs.append_pair("builds_access_level", value.as_str()));
-        self.wiki_access_level
-            .map(|value| pairs.append_pair("wiki_access_level", value.as_str()));
-        self.snippets_access_level
-            .map(|value| pairs.append_pair("snippets_access_level", value.as_str()));
-        self.pages_access_level
-            .map(|value| pairs.append_pair("pages_access_level", value.as_str()));
-
-        self.emails_disabled
-            .map(|value| pairs.append_pair("emails_disabled", common::bool_str(value)));
-        self.resolve_outdated_diff_discussions.map(|value| {
-            pairs.append_pair("resolve_outdated_diff_discussions", common::bool_str(value))
-        });
-        self.container_registry_enabled
-            .map(|value| pairs.append_pair("container_registry_enabled", common::bool_str(value)));
-        if let Some(policy) = self.container_expiration_policy_attributes.as_ref() {
-            policy.add_query(&mut pairs);
-        }
-        self.shared_runners_enabled
-            .map(|value| pairs.append_pair("shared_runners_enabled", common::bool_str(value)));
-        self.visibility
-            .map(|value| pairs.append_pair("visibility", value.as_str()));
-        self.import_url
-            .as_ref()
-            .map(|value| pairs.append_pair("import_url", value));
-        self.public_builds
-            .map(|value| pairs.append_pair("public_builds", common::bool_str(value)));
-        self.only_allow_merge_if_pipeline_succeeds.map(|value| {
-            pairs.append_pair(
+        params
+            .push_opt("namespace_id", self.namespace_id)
+            .push_opt("default_branch", self.default_branch.as_ref())
+            .push_opt("description", self.description.as_ref())
+            .push_opt("issues_access_level", self.issues_access_level)
+            .push_opt("repository_access_level", self.repository_access_level)
+            .push_opt(
+                "merge_requests_access_level",
+                self.merge_requests_access_level,
+            )
+            .push_opt("forking_access_level", self.forking_access_level)
+            .push_opt("builds_access_level", self.builds_access_level)
+            .push_opt("wiki_access_level", self.wiki_access_level)
+            .push_opt("snippets_access_level", self.snippets_access_level)
+            .push_opt("pages_access_level", self.pages_access_level)
+            .push_opt("emails_disabled", self.emails_disabled)
+            .push_opt(
+                "resolve_outdated_diff_discussions",
+                self.resolve_outdated_diff_discussions,
+            )
+            .push_opt(
+                "container_registry_enabled",
+                self.container_registry_enabled,
+            )
+            .push_opt("shared_runners_enabled", self.shared_runners_enabled)
+            .push_opt("visibility", self.visibility)
+            .push_opt("import_url", self.import_url.as_ref())
+            .push_opt("public_builds", self.public_builds)
+            .push_opt(
                 "only_allow_merge_if_pipeline_succeeds",
-                common::bool_str(value),
+                self.only_allow_merge_if_pipeline_succeeds,
             )
-        });
-        self.only_allow_merge_if_all_discussions_are_resolved
-            .map(|value| {
-                pairs.append_pair(
-                    "only_allow_merge_if_all_discussions_are_resolved",
-                    common::bool_str(value),
-                )
-            });
-        self.merge_method
-            .map(|value| pairs.append_pair("merge_method", value.as_str()));
-        self.autoclose_referenced_issues
-            .map(|value| pairs.append_pair("autoclose_referenced_issues", common::bool_str(value)));
-        self.remove_source_branch_after_merge.map(|value| {
-            pairs.append_pair("remove_source_branch_after_merge", common::bool_str(value))
-        });
-        self.lfs_enabled
-            .map(|value| pairs.append_pair("lfs_enabled", common::bool_str(value)));
-        self.request_access_enabled
-            .map(|value| pairs.append_pair("request_access_enabled", common::bool_str(value)));
-        pairs.extend_pairs(self.tag_list.iter().map(|value| ("tag_list[]", value)));
-        self.printing_merge_request_link_enabled.map(|value| {
-            pairs.append_pair(
+            .push_opt(
+                "only_allow_merge_if_all_discussions_are_resolved",
+                self.only_allow_merge_if_all_discussions_are_resolved,
+            )
+            .push_opt("merge_method", self.merge_method)
+            .push_opt(
+                "autoclose_referenced_issues",
+                self.autoclose_referenced_issues,
+            )
+            .push_opt(
+                "remove_source_branch_after_merge",
+                self.remove_source_branch_after_merge,
+            )
+            .push_opt("lfs_enabled", self.lfs_enabled)
+            .push_opt("request_access_enabled", self.request_access_enabled)
+            .extend(self.tag_list.iter().map(|value| ("tag_list[]", value)))
+            .push_opt(
                 "printing_merge_request_link_enabled",
-                common::bool_str(value),
+                self.printing_merge_request_link_enabled,
             )
-        });
-        self.build_git_strategy
-            .map(|value| pairs.append_pair("build_git_strategy", value.as_str()));
-        self.build_timeout
-            .map(|value| pairs.append_pair("build_timeout", &format!("{}", value)));
-        self.auto_cancel_pending_pipelines
-            .map(|value| pairs.append_pair("auto_cancel_pending_pipelines", value.as_str()));
-        self.build_coverage_regex
-            .as_ref()
-            .map(|value| pairs.append_pair("build_coverage_regex", value));
-        self.ci_config_path
-            .as_ref()
-            .map(|value| pairs.append_pair("ci_config_path", value));
-        self.auto_devops_enabled
-            .map(|value| pairs.append_pair("auto_devops_enabled", common::bool_str(value)));
-        self.auto_devops_deploy_strategy
-            .map(|value| pairs.append_pair("auto_devops_deploy_strategy", value.as_str()));
-        self.repository_storage
-            .as_ref()
-            .map(|value| pairs.append_pair("repository_storage", value));
-        self.approvals_before_merge
-            .map(|value| pairs.append_pair("approvals_before_merge", &format!("{}", value)));
-        self.external_authorization_classification_label
-            .as_ref()
-            .map(|value| pairs.append_pair("external_authorization_classification_label", value));
-        self.mirror
-            .map(|value| pairs.append_pair("mirror", common::bool_str(value)));
-        self.mirror_trigger_builds
-            .map(|value| pairs.append_pair("mirror_trigger_builds", common::bool_str(value)));
-        self.initialize_with_readme
-            .map(|value| pairs.append_pair("initialize_with_readme", common::bool_str(value)));
-        self.template_name
-            .as_ref()
-            .map(|value| pairs.append_pair("template_name", value));
-        self.template_project_id
-            .map(|value| pairs.append_pair("template_project_id", &format!("{}", value)));
-        self.use_custom_template
-            .map(|value| pairs.append_pair("use_custom_template", common::bool_str(value)));
-        self.group_with_project_templates_id.map(|value| {
-            pairs.append_pair("group_with_project_templates_id", &format!("{}", value))
-        });
-        self.packages_enabled
-            .map(|value| pairs.append_pair("packages_enabled", common::bool_str(value)));
+            .push_opt("build_git_strategy", self.build_git_strategy)
+            .push_opt("build_timeout", self.build_timeout)
+            .push_opt(
+                "auto_cancel_pending_pipelines",
+                self.auto_cancel_pending_pipelines,
+            )
+            .push_opt("build_coverage_regex", self.build_coverage_regex.as_ref())
+            .push_opt("ci_config_path", self.ci_config_path.as_ref())
+            .push_opt("auto_devops_enabled", self.auto_devops_enabled)
+            .push_opt(
+                "auto_devops_deploy_strategy",
+                self.auto_devops_deploy_strategy,
+            )
+            .push_opt("repository_storage", self.repository_storage.as_ref())
+            .push_opt("approvals_before_merge", self.approvals_before_merge)
+            .push_opt(
+                "external_authorization_classification_label",
+                self.external_authorization_classification_label.as_ref(),
+            )
+            .push_opt("mirror", self.mirror)
+            .push_opt("mirror_trigger_builds", self.mirror_trigger_builds)
+            .push_opt("initialize_with_readme", self.initialize_with_readme)
+            .push_opt("template_name", self.template_name.as_ref())
+            .push_opt("template_project_id", self.template_project_id)
+            .push_opt("use_custom_template", self.use_custom_template)
+            .push_opt(
+                "group_with_project_templates_id",
+                self.group_with_project_templates_id,
+            )
+            .push_opt("packages_enabled", self.packages_enabled);
+
+        if let Some(policy) = self.container_expiration_policy_attributes.as_ref() {
+            policy.add_query(&mut params);
+        }
 
         #[allow(deprecated)]
         {
-            self.issues_enabled
-                .map(|value| pairs.append_pair("issues_enabled", common::bool_str(value)));
-            self.merge_requests_enabled
-                .map(|value| pairs.append_pair("merge_requests_enabled", common::bool_str(value)));
-            self.jobs_enabled
-                .map(|value| pairs.append_pair("jobs_enabled", common::bool_str(value)));
-            self.wiki_enabled
-                .map(|value| pairs.append_pair("wiki_enabled", common::bool_str(value)));
-            self.snippets_enabled
-                .map(|value| pairs.append_pair("snippets_enabled", common::bool_str(value)));
+            params
+                .push_opt("issues_enabled", self.issues_enabled)
+                .push_opt("merge_requests_enabled", self.merge_requests_enabled)
+                .push_opt("jobs_enabled", self.jobs_enabled)
+                .push_opt("wiki_enabled", self.wiki_enabled)
+                .push_opt("snippets_enabled", self.snippets_enabled);
         }
+
+        params.into_body()
     }
 }
 

src/api/projects/edit.rs

@@ -9,7 +9,7 @@ use std::collections::HashSet;
 
 use derive_builder::Builder;
 
-use crate::api::common::{self, EnableState, NameOrId, VisibilityLevel};
+use crate::api::common::{EnableState, NameOrId, VisibilityLevel};
 use crate::api::endpoint_prelude::*;
 use crate::api::projects::{
     AutoDevOpsDeployStrategy, BuildGitStrategy, ContainerExpirationPolicy, FeatureAccessLevel,
@@ -241,143 +241,111 @@ impl<'a> Endpoint for EditProject<'a> {
         format!("projects/{}", self.project).into()
     }
 
-    fn add_parameters(&self, mut pairs: Pairs) {
-        self.name
-            .as_ref()
-            .map(|value| pairs.append_pair("name", value));
-        self.path
-