Re: [pdm-devel] [PATCH datacenter-manager v3 02/18] lib: api-types: add 'layout' property to ViewConfig

[Dominik Csapak <d.csapak@proxmox.com>]

On 11/17/25 3:57 PM, Shannon Sterz wrote:
> On Mon Nov 17, 2025 at 1:44 PM CET, Dominik Csapak wrote:
>> this is a simple string that holds the layout json. We can't currently
>> add it as a normal property with 'correct' api types, since we want to
>> use enum features that are not available with the api macro.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   lib/pdm-api-types/src/views.rs | 12 +++++++++++-
>>   server/src/views/tests.rs      | 15 +++++++++++++++
>>   2 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
>> index ef39cc62..4b837384 100644
>> --- a/lib/pdm-api-types/src/views.rs
>> +++ b/lib/pdm-api-types/src/views.rs
>> @@ -47,7 +47,10 @@ pub const FILTER_RULE_LIST_SCHEMA: Schema =
>>           "exclude": {
>>               schema: FILTER_RULE_LIST_SCHEMA,
>>               optional: true,
>> -        }
>> +        },
>> +        layout: {
>> +            optional: true,
>> +        },
>>       }
>>   )]
>>   #[derive(Clone, Debug, Default, Deserialize, Serialize, Updater, PartialEq)]
>> @@ -67,6 +70,13 @@ pub struct ViewConfig {
>>       #[serde(default, skip_serializing_if = "Vec::is_empty")]
>>       #[updater(serde(skip_serializing_if = "Option::is_none"))]
>>       pub exclude: Vec<FilterRule>,
>> +
>> +    // we can't currently describe this with the 'api' macro so save
>> +    // it simply as a string and check it in the add/update call
> 
> one question: shouldn't this be `Value` instead? right now this needs to
> be a string that is valid json. meaning that the api expects a json body
> that contains `layout` as a string, with that string being escaped json
> again.
> 
> with `Value` this could be the json directly, avoiding having to escape
> the json string. this would also make it easier to describe this later
> on via the api macro if i'm not mistaken. the last commit in this series
> could then also be dropped.
> 
> what do you think?

had a short look and it seems like that we can't currently combine
the Updater derivation with Value since there is no
impl of UpdaterType/ApiType for Value and we can't provide one.

i shortly tried to implment a wrapper type, but then we run into
the issue that we'd have to impl ApiType for that too and
the fact that we can't currently represent the Layout in a schema is
the whole point of the string workaround here.

The only thing that could work here is to simply define
an ObjectSchema with no properties and 'additional_properties' to true.
I'll try that and see if it makes the situation better.

> 
>> +    /// The configured layout, encoded as json
>> +    #[serde(default, skip_serializing_if = "String::is_empty")]
>> +    #[updater(serde(skip_serializing_if = "Option::is_none"))]
>> +    pub layout: String,
>>   }
>>
>>   #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
>> diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs
>> index 030b7994..c015acd2 100644
>> --- a/server/src/views/tests.rs
>> +++ b/server/src/views/tests.rs
>> @@ -184,6 +184,7 @@ fn include_exclude_remotes() {
>>               FilterRule::Remote("remote-b".into()),
>>               FilterRule::Remote("remote-c".into()),
>>           ],
>> +        layout: String::new(),
>>       };
>>       run_test(
>>           config.clone(),
>> @@ -327,6 +328,7 @@ fn include_exclude_type() {
>>               id: "exclude-resource-type".into(),
>>               include: vec![FilterRule::ResourceType(ResourceType::PveQemu)],
>>               exclude: vec![FilterRule::ResourceType(ResourceType::PveStorage)],
>> +            layout: String::new(),
>>           },
>>           &[
>>               (
>> @@ -355,6 +357,7 @@ fn include_exclude_tags() {
>>                   FilterRule::Tag("tag2".to_string()),
>>               ],
>>               exclude: vec![FilterRule::Tag("tag3".to_string())],
>> +            layout: String::new(),
>>           },
>>           &[
>>               (
>> @@ -400,6 +403,7 @@ fn include_exclude_resource_pool() {
>>                   FilterRule::ResourcePool("pool2".to_string()),
>>               ],
>>               exclude: vec![FilterRule::ResourcePool("pool2".to_string())],
>> +            layout: String::new(),
>>           },
>>           &[
>>               (
>> @@ -449,6 +453,7 @@ fn include_exclude_resource_id() {
>>                   FilterRule::ResourceId("remote/otherremote/guest/101".to_string()),
>>                   FilterRule::ResourceId(format!("remote/{REMOTE}/storage/{NODE}/otherstorage")),
>>               ],
>> +            layout: String::new(),
>>           },
>>           &[
>>               (
>> @@ -495,6 +500,7 @@ fn node_included() {
>>               FilterRule::ResourceId("remote/someremote/node/test".to_string()),
>>           ],
>>           exclude: vec![FilterRule::Remote("remote-b".to_string())],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(view.is_node_included("remote-a", "somenode"));
>> @@ -512,6 +518,7 @@ fn can_skip_remote_if_excluded() {
>>           id: "abc".into(),
>>           include: vec![],
>>           exclude: vec![FilterRule::Remote("remote-b".to_string())],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(!view.can_skip_remote("remote-a"));
>> @@ -524,6 +531,7 @@ fn can_skip_remote_if_included() {
>>           id: "abc".into(),
>>           include: vec![FilterRule::Remote("remote-b".to_string())],
>>           exclude: vec![],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(!view.can_skip_remote("remote-b"));
>> @@ -539,6 +547,7 @@ fn can_skip_remote_cannot_skip_if_any_other_include() {
>>               FilterRule::ResourceId("resource/remote-a/guest/100".to_string()),
>>           ],
>>           exclude: vec![],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(!view.can_skip_remote("remote-b"));
>> @@ -553,6 +562,7 @@ fn can_skip_remote_explicit_remote_exclude() {
>>               "resource/remote-a/guest/100".to_string(),
>>           )],
>>           exclude: vec![FilterRule::Remote("remote-a".to_string())],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(view.can_skip_remote("remote-a"));
>> @@ -564,6 +574,7 @@ fn can_skip_remote_with_empty_config() {
>>           id: "abc".into(),
>>           include: vec![],
>>           exclude: vec![],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(!view.can_skip_remote("remote-a"));
>> @@ -578,6 +589,7 @@ fn can_skip_remote_with_no_remote_includes() {
>>               "resource/remote-a/guest/100".to_string(),
>>           )],
>>           exclude: vec![],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(!view.can_skip_remote("remote-a"));
>> @@ -590,6 +602,7 @@ fn explicitly_included_remote() {
>>           id: "abc".into(),
>>           include: vec![FilterRule::Remote("remote-b".to_string())],
>>           exclude: vec![],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(view.is_remote_explicitly_included("remote-b"));
>> @@ -601,6 +614,7 @@ fn included_and_excluded_same_remote() {
>>           id: "abc".into(),
>>           include: vec![FilterRule::Remote("remote-b".to_string())],
>>           exclude: vec![FilterRule::Remote("remote-b".to_string())],
>> +        layout: String::new(),
>>       });
>>
>>       assert!(!view.is_remote_explicitly_included("remote-b"));
>> @@ -612,6 +626,7 @@ fn not_explicitly_included_remote() {
>>           id: "abc".into(),
>>           include: vec![],
>>           exclude: vec![],
>> +        layout: String::new(),
>>       });
>>
>>       // Assert that is not *explicitly* included
> 



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel