Add required properties to OpenAPI schemas#45
Conversation
This change adds 'required' property declarations to schemas based on the official Hue CLIP API documentation. Changes include: - Common schemas: Resource, ResourceIdentifier, ResourceOwned, GamutPosition, On, Dimming, Color, error, ApiResponse - Resource Get schemas: RoomGet, DeviceGet, SceneGet, LightGet, MotionGet, BridgeGet, BridgeHomeGet, TemperatureGet, DevicePowerGet, LightLevelGet, ActionGet, ProductData This ensures code generators produce properly typed models with non-optional fields where the API guarantees their presence. Fixes #44 Co-Authored-By: Warp <agent@warp.dev>
📝 WalkthroughWalkthroughThis pull request systematically adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hey @flrgh, here's the use crate::models;
use serde::{Deserialize, Serialize};
#[derive(Clone, Default, Debug, PartialEq, Serialize, Deserialize)]
pub struct RoomGet {
/// Type of the supported resources
#[serde(rename = "type")]
pub r#type: String,
/// Unique identifier representing a specific resource instance
#[serde(rename = "id")]
pub id: String,
/// Clip v1 resource identifier
#[serde(rename = "id_v1", skip_serializing_if = "Option::is_none")]
pub id_v1: Option<String>,
/// Child devices/services to group by the derived group
#[serde(rename = "children")]
pub children: Vec<models::ResourceIdentifier>,
/// References all services aggregating control and state of children in the group. This includes all services grouped in the group hierarchy given by child relation. This includes all services of a device grouped in the group hierarchy given by child relation. Aggregation is per service type, ie every service type which can be grouped has a corresponding definition of grouped type. Supported types: – grouped_light
#[serde(rename = "services")]
pub services: Vec<models::ResourceIdentifier>,
#[serde(rename = "metadata")]
pub metadata: Box<models::RoomGetAllOfMetadata>,
}
impl RoomGet {
pub fn new(r#type: String, id: String, children: Vec<models::ResourceIdentifier>, services: Vec<models::ResourceIdentifier>, metadata: models::RoomGetAllOfMetadata) -> RoomGet {
RoomGet {
r#type,
id,
id_v1: None,
children,
services,
metadata: Box::new(metadata),
}
}
}You can check the codegen from this PR using this ephemeral spec link.
That will indeed be seen as a breaking change for existing consumers, but I am convinced that this is for the good of the project! |
|
Nicely done! I think this is a good improvement for the auto-generated resource models. |
Summary
This PR adds
requiredproperty declarations to OpenAPI schemas based on the official Hue CLIP API documentation.Problem
As reported in #44, many object schemas were missing
requiredproperties. This caused code generators to treat all fields as optional, leading to clunky generated models (e.g.,Option<String>instead ofStringin Rust).Solution
Added
requireddeclarations to schemas by cross-referencing with the official Hue API documentation (onlinedoc.html).Updated schemas:
Common schemas:
Resource.yaml: Addedrequired: [type, id]ResourceIdentifier.yaml: Addedrequired: [rid, rtype]ResourceOwned.yaml: Addedrequired: [owner]GamutPosition.yaml: Addedrequired: [x, y]On.yaml: Addedrequired: [on]Dimming.yaml: Addedrequired: [brightness]Color.yaml: Addedrequired: [xy]error.yaml: Addedrequiredto ErrorResponse and ErrorApiResponse.yaml: Addedrequired: [errors, data]Resource Get schemas:
RoomGet,DeviceGet,SceneGet,LightGet,MotionGet,BridgeGet,BridgeHomeGet,TemperatureGet,DevicePowerGet,LightLevelGet,ActionGet,ProductDataValidation
redocly lintFixes #44
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores
Validation & Schema Updates
identify(visual identification trigger) andservice_id.