Skip to content

feat(resource): support custom generated record names #432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rapids-bot merged 2 commits into from
Jul 22, 2026
Merged

feat(resource): support custom generated record names #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
37dc4a3
feat(resource): support custom record names
johanpel Jul 21, 2026
048a88d
test(resource): use shared identifier helper
johanpel Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 74 additions & 27 deletions crates/resource/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use quent_schema::{
Annotations, DataType, Field, Identifier, Record,
builder::{AnnotationsBuilder, BuilderError, RecordBuilder},
schema::identifier::IdentifierError,
};
use thiserror::Error;

Expand All @@ -25,15 +24,32 @@ pub struct ResourceParts {
/// Builds a resource definition and its usage and bounds record types.
pub struct ResourceBuilder {
name: Identifier,
usage_record_name: Identifier,
bounds_record_name: Identifier,
capacities: Capacities,
errors: Vec<BuildError>,
}

impl ResourceBuilder {
/// Start a resource named `name`.
/// Start a resource using `{name}Usage` and `{name}Bounds` record names.
pub fn new(name: Identifier) -> Self {
let usage_record_name = suffixed_identifier(&name, "Usage");
let bounds_record_name = suffixed_identifier(&name, "Bounds");
Self::with_record_names(name, usage_record_name, bounds_record_name)
}

/// Start a resource with explicit generated record names.
///
/// `bounds_record_name` is used only when a capacity is bounded.
pub fn with_record_names(
name: Identifier,
usage_record_name: Identifier,
bounds_record_name: Identifier,
) -> Self {
Self {
name,
usage_record_name,
bounds_record_name,
capacities: Capacities::default(),
errors: Vec::new(),
}
Expand Down Expand Up @@ -70,21 +86,27 @@ impl ResourceBuilder {
self
}

/// Build the definition and the usage and bounds record types.
/// Build the definition and its usage and bounds record types.
///
/// With no capacities the result is a unit resource: an empty usage record
/// and no bounds.
///
/// # Errors
///
/// Errors if a name is repeated, or generating the records or constraint
/// data fails.
/// Errors if a capacity name is repeated, the supplied names are equal when
/// bounds are generated, or generation fails.
pub fn build(self) -> Result<ResourceParts, BuildError> {
let ResourceBuilder {
name,
usage_record_name,
bounds_record_name,
capacities,
mut errors,
} = self;
let has_bounds = capacities.values().any(Capacity::is_bounded);
if has_bounds && usage_record_name == bounds_record_name {
errors.push(BuildError::DuplicateRecordName(usage_record_name.clone()));
}
match errors.len() {
0 => {}
1 => return Err(errors.pop().unwrap()),
Expand All @@ -93,17 +115,17 @@ impl ResourceBuilder {

// The usage record carries a claim field for each capacity.
let usage = build_resource_record(
suffixed_identifier(&name, "Usage")?,
usage_record_name,
Resource::Usage {
resource: name.clone(),
},
capacities.keys(),
)?;

// The bounds record carries a field for each bounded capacity, if any.
let bounds = if capacities.values().any(Capacity::is_bounded) {
let bounds = if has_bounds {
Some(build_resource_record(
suffixed_identifier(&name, "Bounds")?,
bounds_record_name,
Resource::Bounds {
resource: name.clone(),
},
Expand Down Expand Up @@ -133,7 +155,7 @@ fn build_resource_record<'a>(
fields: impl Iterator<Item = &'a Identifier>,
) -> Result<Record, BuildError> {
let annotations = AnnotationsBuilder::new()
.try_with_constraint(Resource::NAME, Some(serde_json::to_string(&resource)?))?
.try_with_constraint(Resource::NAME, Some(resource.constraint_data()?))?
.build();
let mut builder = RecordBuilder::new(name).with_annotations(annotations);
for field in fields {
Expand All @@ -146,20 +168,21 @@ fn build_resource_record<'a>(
Ok(builder.build())
}

fn suffixed_identifier(resource: &Identifier, suffix: &str) -> Result<Identifier, BuildError> {
Ok(Identifier::try_new(format!("{resource}{suffix}"))?)
fn suffixed_identifier(resource: &Identifier, suffix: &str) -> Identifier {
Identifier::try_new(format!("{resource}{suffix}"))
.expect("suffixing a valid identifier preserves validity")
}

#[derive(Debug, Error)]
pub enum BuildError {
#[error("duplicate capacity \"{0}\"")]
DuplicateCapacity(Identifier),
#[error("usage and bounds records have the same name \"{0}\"")]
DuplicateRecordName(Identifier),
#[error("multiple resource builder errors: {0:?}")]
Multiple(Vec<BuildError>),
#[error(transparent)]
Schema(#[from] BuilderError),
#[error(transparent)]
Identifier(#[from] IdentifierError),
#[error("serializing resource data: {0}")]
Serialize(#[from] serde_json::Error),
}
Expand All @@ -168,16 +191,18 @@ pub enum BuildError {
mod tests {
use super::*;
use crate::CapacityKind;
use quent_schema::test_utils::ident;

#[test]
fn builds_definition_and_records() -> Result<(), BuildError> {
let bytes = Identifier::try_new("bytes")?;
let usage_name = Identifier::try_new("MemoryUsage")?;
let bounds_name = Identifier::try_new("MemoryBounds")?;
fn builds_definition_and_records() {
let bytes = ident("bytes");
let usage_name = ident("MemoryUsage");
let bounds_name = ident("MemoryBounds");

let parts = ResourceBuilder::new(Identifier::try_new("Memory")?)
let parts = ResourceBuilder::new(ident("Memory"))
.with_capacity(bytes.clone(), Capacity::new(CapacityKind::Occupancy, true))
.build()?;
.build()
.unwrap();

let mut capacities = parts.definition.capacities().unwrap();
let (name, capacity) = capacities.next().unwrap();
Expand All @@ -192,27 +217,49 @@ mod tests {
.bounds
.is_some_and(|bounds| bounds.name() == &bounds_name)
);
Ok(())
}

/// A resource with no capacities is a unit resource: a fieldless usage
/// record and no bounds.
#[test]
fn builds_unit_resource() -> Result<(), BuildError> {
let parts = ResourceBuilder::new(Identifier::try_new("Thread")?).build()?;
fn builds_unit_resource() {
let parts = ResourceBuilder::new(ident("Thread")).build().unwrap();
assert!(parts.definition.capacities().unwrap().next().is_none());
assert_eq!(parts.usage.name(), &Identifier::try_new("ThreadUsage")?);
assert_eq!(parts.usage.name(), &ident("ThreadUsage"));
assert_eq!(parts.usage.fields().count(), 0);
assert!(parts.bounds.is_none());
Ok(())
}

#[test]
fn uses_supplied_record_names() {
let parts = ResourceBuilder::with_record_names(
ident("Memory"),
ident("MemoryClaim"),
ident("MemoryLimits"),
)
.with_capacity(ident("bytes"), Capacity::new(CapacityKind::Occupancy, true))
.build()
.unwrap();

assert_eq!(parts.usage.name(), &ident("MemoryClaim"));
assert_eq!(parts.bounds.unwrap().name(), &ident("MemoryLimits"));
}

#[test]
fn rejects_duplicate_record_names() {
let shared = ident("MemoryData");
let result = ResourceBuilder::with_record_names(ident("Memory"), shared.clone(), shared)
.with_capacity(ident("bytes"), Capacity::new(CapacityKind::Occupancy, true))
.build();
assert!(matches!(result, Err(BuildError::DuplicateRecordName(_))));
}
Comment on lines +248 to 255

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Cover equal names when no bounds record is produced.

The duplicate-name test only covers bounded capacities. Add a unit-resource or unbounded-capacity case with equal names and assert that build() succeeds, matching the has_bounds condition on Lines 106-108.

As per coding guidelines, “New Rust components must include accompanying tests.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/resource/src/builder.rs` around lines 248 - 255, Extend
rejects_duplicate_record_names or add a focused test for unbounded/unit capacity
using equal data and bounds record names, and assert ResourceBuilder::build
succeeds when no bounds record is produced. Preserve the existing failure
assertion for bounded capacities and align the new case with the has_bounds
condition.

Source: Coding guidelines


/// Requirement 1: capacity identifiers are unique within a resource.
#[test]
fn rejects_duplicate_capacities() {
let bytes = Identifier::try_new("bytes").unwrap();
let watts = Identifier::try_new("watts").unwrap();
let result = ResourceBuilder::new(Identifier::try_new("Memory").unwrap())
let bytes = ident("bytes");
let watts = ident("watts");
let result = ResourceBuilder::new(ident("Memory"))
.with_capacity(bytes.clone(), Capacity::new(CapacityKind::Occupancy, true))
.with_capacity(bytes.clone(), Capacity::new(CapacityKind::Rate, false))
.with_capacity(watts.clone(), Capacity::new(CapacityKind::Rate, false))
Expand Down
9 changes: 9 additions & 0 deletions crates/resource/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ impl Resource {
/// Constraint identifier.
pub const NAME: &'static str = "quent.resource.v0.1.0";

/// Encode this resource as a constraint payload.
///
/// # Errors
///
/// Returns an error if serialization fails.
pub fn constraint_data(&self) -> Result<String, serde_json::Error> {
serde_json::to_string(self)
}
Comment on lines +114 to +121

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add direct coverage for the public payload contract.

No changed test asserts that constraint_data() produces JSON that decodes back to the same Resource. Add serialization round-trip coverage for at least usage and definition resources; builder success alone does not verify this API’s payload contract.

As per coding guidelines, “New Rust components must include accompanying tests.”

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/resource/src/lib.rs` around lines 114 - 121, Add tests for the public
Resource::constraint_data API covering both usage and definition resources. For
each case, build a representative resource, serialize it with constraint_data(),
deserialize the JSON back into Resource, and assert equality with the original;
retain existing builder success tests.

Source: Coding guidelines


/// Return the declared capacity names and definitions.
///
/// Return `None` unless this is [`Self::Definition`].
Expand Down