From cb9d57b4539b12372097994bd58c65718c55cc31 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
Date: Thu, 12 Aug 2021 10:15:12 +0200
Subject: [PATCH] put API_SCHEMA variable into ApiType trait

This way we can assign `API_SCHEMA` constants to `Option`
types.

Here's why:

The api-macro generated code usese `T::API_SCHEMA` when
building ObjectSchemas.

For Updaters we replace `T` with
  `<T as Updatable>::Updater`

This means for "simple" wrappers like our `Authid` or
`Userid`, the ObjectSchema will try to refer to
  `<Authid as Updatable>::Updater::API_SCHEMA`
which resolves to:
  `Option<Authid>::API_SCHEMA`
which does not exist, for which we cannot add a normal
`impl` block to add the schema variable, since `Option` is
not "ours".

But we now have a blanket implementation of `ApiType` for
`Option<T> where T: ApiType` which just points to the
original `T::API_SCHEMA`.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/enums.rs   |  6 ++++--
 proxmox-api-macro/src/api/mod.rs     |  4 ++--
 proxmox-api-macro/src/api/structs.rs | 15 +++++++++++----
 proxmox-api-macro/tests/updater.rs   | 17 +++++++++++++++--
 proxmox/src/api/schema.rs            |  8 ++++++++
 5 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/proxmox-api-macro/src/api/enums.rs b/proxmox-api-macro/src/api/enums.rs
index 4654e0e2..344ffa39 100644
--- a/proxmox-api-macro/src/api/enums.rs
+++ b/proxmox-api-macro/src/api/enums.rs
@@ -76,8 +76,10 @@ pub fn handle_enum(
 
     Ok(quote_spanned! { name.span() =>
         #enum_ty
-        impl #name {
-            pub const API_SCHEMA: ::proxmox::api::schema::Schema =
+
+        #[automatically_derived]
+        impl ::proxmox::api::schema::ApiType for #name {
+            const API_SCHEMA: ::proxmox::api::schema::Schema =
                 #schema
                 .format(&::proxmox::api::schema::ApiStringFormat::Enum(&[#variants]))
                 .schema();
diff --git a/proxmox-api-macro/src/api/mod.rs b/proxmox-api-macro/src/api/mod.rs
index 60c3ce7a..5a644b35 100644
--- a/proxmox-api-macro/src/api/mod.rs
+++ b/proxmox-api-macro/src/api/mod.rs
@@ -148,7 +148,7 @@ impl Schema {
     fn to_schema_reference(&self) -> Option<TokenStream> {
         match &self.item {
             SchemaItem::ExternType(path) => {
-                Some(quote_spanned! { path.span() => &#path::API_SCHEMA })
+                Some(quote_spanned! { path.span() => &<#path as ::proxmox::api::schema::ApiType>::API_SCHEMA })
             }
             SchemaItem::ExternSchema(path) => Some(quote_spanned! { path.span() => &#path }),
             _ => None,
@@ -375,7 +375,7 @@ impl SchemaItem {
                     error!(description => "description not allowed on external type");
                 }
 
-                ts.extend(quote_spanned! { path.span() => #path::API_SCHEMA });
+                ts.extend(quote_spanned! { path.span() => <#path as ::proxmox::api::schema::ApiType>::API_SCHEMA });
                 return Ok(true);
             }
             SchemaItem::ExternSchema(path) => {
diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs
index f761c9be..f59c3940 100644
--- a/proxmox-api-macro/src/api/structs.rs
+++ b/proxmox-api-macro/src/api/structs.rs
@@ -84,8 +84,10 @@ fn finish_schema(
 
     Ok(quote_spanned! { name.span() =>
         #stru
-        impl #name {
-            pub const API_SCHEMA: ::proxmox::api::schema::Schema = #schema;
+
+        #[automatically_derived]
+        impl ::proxmox::api::schema::ApiType for #name {
+            const API_SCHEMA: ::proxmox::api::schema::Schema = #schema;
         }
     })
 }
@@ -347,9 +349,14 @@ fn finish_all_of_struct(
 
     Ok(quote_spanned!(name.span() =>
         #stru
+
         impl #name {
             #inner_schema
-            pub const API_SCHEMA: ::proxmox::api::schema::Schema =
+        }
+
+        #[automatically_derived]
+        impl ::proxmox::api::schema::ApiType for #name {
+            const API_SCHEMA: ::proxmox::api::schema::Schema =
                 ::proxmox::api::schema::AllOfSchema::new(
                     #description,
                     &[
@@ -528,7 +535,7 @@ fn handle_updater_field(
 
     if field_schema.flatten_in_struct {
         let updater_ty = &field.ty;
-        all_of_schemas.extend(quote::quote! {&#updater_ty::API_SCHEMA,});
+        all_of_schemas.extend(quote::quote! {&<#updater_ty as ::proxmox::api::schema::ApiType>::API_SCHEMA,});
     }
 
     if !is_empty_impl.is_empty() {
diff --git a/proxmox-api-macro/tests/updater.rs b/proxmox-api-macro/tests/updater.rs
index df4e5393..0ccde105 100644
--- a/proxmox-api-macro/tests/updater.rs
+++ b/proxmox-api-macro/tests/updater.rs
@@ -1,5 +1,15 @@
 use proxmox::api::api;
-use proxmox::api::schema::Updater;
+use proxmox::api::schema::{Schema, StringSchema, Updatable, Updater};
+
+#[derive(Updatable)]
+pub struct Custom(String);
+
+impl proxmox::api::schema::ApiType for Custom {
+    const API_SCHEMA: Schema = StringSchema::new("Custom String")
+        .min_length(3)
+        .max_length(64)
+        .schema();
+}
 
 #[api]
 /// An example of a simple struct type.
@@ -38,7 +48,7 @@ pub struct Complex {
     },
 )]
 /// One of the baaaad cases.
-#[derive(Default, Updater)]
+#[derive(Updater)]
 #[serde(rename_all = "kebab-case")]
 pub struct SuperComplex {
     /// An extra field not part of the flattened struct.
@@ -50,4 +60,7 @@ pub struct SuperComplex {
     /// A field which should not appear in the updater.
     #[updater(skip)]
     not_in_updater: String,
+
+    /// A custom type with an Updatable implementation.
+    custom: Custom,
 }
diff --git a/proxmox/src/api/schema.rs b/proxmox/src/api/schema.rs
index 5b7e3d5d..751dc1d1 100644
--- a/proxmox/src/api/schema.rs
+++ b/proxmox/src/api/schema.rs
@@ -1560,6 +1560,14 @@ where
     const UPDATER_IS_OPTION: bool = true;
 }
 
+pub trait ApiType {
+    const API_SCHEMA: Schema;
+}
+
+impl<T: ApiType> ApiType for Option<T> {
+    const API_SCHEMA: Schema = T::API_SCHEMA;
+}
+
 /// A helper type for "Updater" structs. This trait is *not* implemented for an api "base" type
 /// when deriving an `Updater` for it, though the generated *updater* type does implement this
 /// trait!