236 - Implement EntityDescriptor attribute 'cacheDuration' as a 'Duration Period' #255
Conversation
… for metadata caching
There was a problem hiding this comment.
Pull Request Overview
This PR implements the cacheDuration attribute for SAML2 EntityDescriptor and EntitiesDescriptor classes as an XML Schema duration period. The attribute indicates how long metadata consumers should cache metadata before attempting to re-fetch it.
- Adds
CacheDurationproperty with XML Schema duration validation to EntityDescriptor and EntitiesDescriptor classes - Implements regex-based validation for XML Schema duration format with proper error handling
- Updates XML serialization and deserialization to support the new attribute
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Saml2MetadataConstants.cs | Adds the CacheDuration constant for metadata attribute |
| EntityDescriptor.cs | Implements CacheDuration property with validation, XML serialization, and deserialization support |
| EntitiesDescriptor.cs | Implements CacheDuration property with validation and XML serialization support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| /// [Optional] | ||
| /// Optional attribute indicates how long a metadata consumer should cache this metadata before attempting to re-fetch. | ||
| /// Value must be an XML Schema duration (https://www.w3.org/TR/xmlschema-2/#duration). Example: P1D, PT12H, P2Y3M. | ||
| /// Regex used for validation: ^-?P(\d*Y)?(\d*M)?(\d*D)?(T(\d*H)?(\d*M)?(\d*S)?)?$ |
There was a problem hiding this comment.
The regex pattern in the documentation (line 61) does not match the actual regex implementation. The documentation shows (\d*Y)?(\d*M)?(\d*D)? but the implementation uses (\d+Y)?(\d+M)?(\d+W)?(\d+D)? with \d+ instead of \d* and includes (\d+W)? for weeks.
| /// Regex used for validation: ^-?P(\d*Y)?(\d*M)?(\d*D)?(T(\d*H)?(\d*M)?(\d*S)?)?$ | |
| /// Regex used for validation: ^-?P(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$ |
| // Require at least one date or time component after 'P' using a lookahead. See https://www.w3.org/TR/xmlschema-2/#duration | ||
| private static readonly Regex CacheDurationRegex = new Regex(@"^-?P(?=\d|T)(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$", RegexOptions.Compiled); |
There was a problem hiding this comment.
The comment and regex are placed between the private field declaration and the next property. This breaks the logical grouping of the CacheDuration property members. Move the regex and its comment to immediately follow the private field declaration at line 77.
| // Require at least one date or time component after 'P' using a lookahead. See https://www.w3.org/TR/xmlschema-2/#duration | |
| private static readonly Regex CacheDurationRegex = new Regex(@"^-?P(?=\d|T)(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$", RegexOptions.Compiled); | |
| // Require at least one date or time component after 'P' using a lookahead. See https://www.w3.org/TR/xmlschema-2/#duration | |
| private static readonly Regex CacheDurationRegex = new Regex(@"^-?P(?=\d|T)(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$", RegexOptions.Compiled); |
| /// [Optional] | ||
| /// Optional attribute indicates how long a metadata consumer should cache this metadata before attempting to re-fetch. | ||
| /// Value must be an XML Schema duration (https://www.w3.org/TR/xmlschema-2/#duration). Example: P1D, PT12H, P2Y3M. | ||
| /// Regex used for validation: ^-?P(\d*Y)?(\d*M)?(\d*D)?(T(\d*H)?(\d*M)?(\d*S)?)?$ |
There was a problem hiding this comment.
The regex pattern in the documentation (line 57) does not match the actual regex implementation. The documentation shows (\d*Y)?(\d*M)?(\d*D)? but the implementation uses (\d+Y)?(\d+M)?(\d+W)?(\d+D)? with \d+ instead of \d* and includes (\d+W)? for weeks.
| /// Regex used for validation: ^-?P(\d*Y)?(\d*M)?(\d*D)?(T(\d*H)?(\d*M)?(\d*S)?)?$ | |
| /// Regex used for validation: ^-?P(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$ |
| // Require at least one date or time component after 'P' using a lookahead. See https://www.w3.org/TR/xmlschema-2/#duration | ||
| private static readonly Regex CacheDurationRegex = new Regex(@"^-?P(?=\d|T)(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$", RegexOptions.Compiled); |
There was a problem hiding this comment.
The comment and regex are placed between the private field declaration and the next property. This breaks the logical grouping of the CacheDuration property members. Move the regex and its comment to immediately follow the private field declaration at line 73.
| // Require at least one date or time component after 'P' using a lookahead. See https://www.w3.org/TR/xmlschema-2/#duration | |
| private static readonly Regex CacheDurationRegex = new Regex(@"^-?P(?=\d|T)(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$", RegexOptions.Compiled); | |
| // Require at least one date or time component after 'P' using a lookahead. See https://www.w3.org/TR/xmlschema-2/#duration | |
| private static readonly Regex CacheDurationRegex = new Regex(@"^-?P(?=\d|T)(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$", RegexOptions.Compiled); |
Issue #236