From 66c890807fb77845264c6f103ac861ae8adfc022 Mon Sep 17 00:00:00 2001
From: Adam Leventhal <ahl@oxide.computer>
Date: Mon, 15 May 2023 08:59:07 -0700
Subject: [PATCH] CLI: improve required settings for paginated operations
 (#469)

---
 progenitor-impl/src/cli.rs                 | 36 +++++++++++++++-------
 progenitor-impl/src/method.rs              | 11 +++----
 progenitor-impl/tests/output/nexus-cli.out |  4 +--
 sample_openapi/nexus.json                  |  7 ++++-
 4 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/progenitor-impl/src/cli.rs b/progenitor-impl/src/cli.rs
index 0a07091..0d58bfd 100644
--- a/progenitor-impl/src/cli.rs
+++ b/progenitor-impl/src/cli.rs
@@ -286,28 +286,42 @@ impl Generator {
             .collect::<Vec<_>>();
         let fn_name = format_ident!("cli_{}", &method.operation_id);
 
+        let first_page_required_set = method
+            .dropshot_paginated
+            .as_ref()
+            .map(|d| &d.first_page_params);
+
         let args = method
             .params
             .iter()
             .filter(|param| {
                 !matches!(&param.kind, OperationParameterKind::Body(_))
                     && (method.dropshot_paginated.is_none()
-                        || (param.name.as_str() != "page_token"))
+                        || param.name.as_str() != "page_token")
             })
             .map(|param| {
                 let arg_name = param.name.to_kebab_case();
 
-                let required = match &param.kind {
-                    OperationParameterKind::Path => Volitionality::Required,
-                    OperationParameterKind::Query(true)
-                    | OperationParameterKind::Header(true) => {
-                        Volitionality::Required
+                let first_page_required = first_page_required_set
+                    .map_or(false, |required| {
+                        required.contains(&param.api_name)
+                    });
+
+                let required = if first_page_required {
+                    Volitionality::Required
+                } else {
+                    match &param.kind {
+                        OperationParameterKind::Path => Volitionality::Required,
+                        OperationParameterKind::Query(true)
+                        | OperationParameterKind::Header(true) => {
+                            Volitionality::Required
+                        }
+                        OperationParameterKind::Query(false)
+                        | OperationParameterKind::Header(false) => {
+                            Volitionality::Optional
+                        }
+                        OperationParameterKind::Body(_) => unreachable!(),
                     }
-                    OperationParameterKind::Query(false)
-                    | OperationParameterKind::Header(false) => {
-                        Volitionality::Optional
-                    }
-                    OperationParameterKind::Body(_) => unreachable!(),
                 };
 
                 let OperationParameterType::Type(arg_type_id) = &param.typ
diff --git a/progenitor-impl/src/method.rs b/progenitor-impl/src/method.rs
index abca9f0..9a1dc18 100644
--- a/progenitor-impl/src/method.rs
+++ b/progenitor-impl/src/method.rs
@@ -1,4 +1,4 @@
-// Copyright 2022 Oxide Computer Company
+// Copyright 2023 Oxide Computer Company
 
 use std::{
     cmp::Ordering,
@@ -88,18 +88,15 @@ struct BuilderImpl {
 }
 
 pub struct DropshotPagination {
-    item: TypeId,
-    // TODO this is going to be used by the SDK and CLI generation to validate
-    // inputs.
-    #[allow(unused)]
-    first_page_params: Vec<String>,
+    pub item: TypeId,
+    pub first_page_params: Vec<String>,
 }
 
 pub struct OperationParameter {
     /// Sanitized parameter name.
     pub name: String,
     /// Original parameter name provided by the API.
-    api_name: String,
+    pub api_name: String,
     pub description: Option<String>,
     pub typ: OperationParameterType,
     pub kind: OperationParameterKind,
diff --git a/progenitor-impl/tests/output/nexus-cli.out b/progenitor-impl/tests/output/nexus-cli.out
index 5cc4092..ce2207f 100644
--- a/progenitor-impl/tests/output/nexus-cli.out
+++ b/progenitor-impl/tests/output/nexus-cli.out
@@ -1066,7 +1066,7 @@ impl Cli {
                 clap::Arg::new("end-time")
                     .long("end-time")
                     .value_parser(clap::value_parser!(chrono::DateTime<chrono::offset::Utc>))
-                    .required(false)
+                    .required(true)
                     .help("An exclusive end time of metrics."),
             )
             .arg(
@@ -1080,7 +1080,7 @@ impl Cli {
                 clap::Arg::new("start-time")
                     .long("start-time")
                     .value_parser(clap::value_parser!(chrono::DateTime<chrono::offset::Utc>))
-                    .required(false)
+                    .required(true)
                     .help("An inclusive start time of metrics."),
             )
             .about("Fetch disk metrics")
diff --git a/sample_openapi/nexus.json b/sample_openapi/nexus.json
index 3ae33fe..9adda4f 100644
--- a/sample_openapi/nexus.json
+++ b/sample_openapi/nexus.json
@@ -1683,7 +1683,12 @@
             "$ref": "#/components/responses/Error"
           }
         },
-        "x-dropshot-pagination": true
+        "x-dropshot-pagination": {
+          "required": [
+            "end_time",
+            "start_time"
+          ]
+        }
       }
     },
     "/organizations/{organization_name}/projects/{project_name}/images": {