Skip to content

feat: Add property support to shapes and migrate ChangeRequest #491

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

Open
google-labs-jules wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: Add property support to shapes and migrate ChangeRequest #491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b4b5b1
Migrate ChangeRequest to properties and update ResourceShapeFactory
google-labs-jules[bot] Dec 7, 2025
7707778
Migrate ChangeRequest to properties using backing fields
google-labs-jules[bot] Dec 8, 2025
65e31b3
Merge branch 'main' into migrate-changerequest-to-properties
berezovskyi Dec 13, 2025
d75e144
Merge branch 'main' into migrate-changerequest-to-properties
berezovskyi Dec 30, 2025
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
2,047 changes: 1,191 additions & 856 deletions OSLC4Net_SDK/OSLC4Net.ChangeManagement/ChangeRequest.cs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcAllowedValue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace OSLC4Net.Core.Attribute;
/// OSLC AllowedValue attribute
/// </summary>
/// <remarks>See http://open-services.net/bin/view/Main/OSLCCoreSpecAppendixA </remarks>
[AttributeUsage(AttributeTargets.Method)
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
public class OslcAllowedValue : System.Attribute
{
Expand Down
2 changes: 1 addition & 1 deletion OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcDescription.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace OSLC4Net.Core.Attribute;
/// OSLC Description attribute
/// </summary>
/// <remarks>See http://open-services.net/bin/view/Main/OSLCCoreSpecAppendixA </remarks>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
public class OslcDescription : System.Attribute
{
Expand Down
7 changes: 4 additions & 3 deletions OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcName.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ namespace OSLC4Net.Core.Attribute;
/// <summary>
/// OSLC Name attribute
/// </summary>
[AttributeUsage(AttributeTargets.Method |
AttributeTargets.Class |
AttributeTargets.Interface | AttributeTargets.Property)
[AttributeUsage(AttributeTargets.Method |
AttributeTargets.Class |
AttributeTargets.Interface |
AttributeTargets.Property)
]
public class OslcName : System.Attribute
{
Expand Down
2 changes: 1 addition & 1 deletion OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcOccurs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace OSLC4Net.Core.Attribute;
/// <summary>
/// OSLC Occurs attribute
/// </summary>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
public class OslcOccurs : System.Attribute
{
Expand Down
2 changes: 1 addition & 1 deletion OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcPropertyDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace OSLC4Net.Core.Attribute;
/// <summary>
/// OSLC PropertyDefinition attribute
/// </summary>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
public class OslcPropertyDefinition : System.Attribute
{
Expand Down
2 changes: 1 addition & 1 deletion OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcRange.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace OSLC4Net.Core.Attribute;
/// OSLC Range attribute
/// </summary>
/// <remarks>See http://open-services.net/bin/view/Main/OSLCCoreSpecAppendixA </remarks>
[AttributeUsage(AttributeTargets.Method)
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
public class OslcRange : System.Attribute
{
Expand Down
2 changes: 1 addition & 1 deletion OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcReadOnly.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace OSLC4Net.Core.Attribute;
/// OSLC ReadOnly attribute
/// </summary>
/// <remarks>See http://open-services.net/bin/view/Main/OSLCCoreSpecAppendixA </remarks>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
public class OslcReadOnly : System.Attribute
{
Expand Down
2 changes: 1 addition & 1 deletion OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcTitle.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace OSLC4Net.Core.Attribute;
/// OSLC Title attribute
/// </summary>
/// <remarks>See http://open-services.net/bin/view/Main/OSLCCoreSpecAppendixA </remarks>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
public class OslcTitle : System.Attribute
{
Expand Down
2 changes: 1 addition & 1 deletion OSLC4Net_SDK/OSLC4Net.Core/Attribute/OslcValueType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace OSLC4Net.Core.Attribute;
/// <summary>
/// OSLC ValueType attribue
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

Spelling: "attribue" should be "attribute"

Suggested change
/// OSLC ValueType attribue
/// OSLC ValueType attribute

Copilot uses AI. Check for mistakes.
/// </summary>
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
Comment on lines +24 to 25
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

[nitpick] The closing bracket for AttributeUsage is placed on a separate line inconsistently. While this is valid C# syntax, it's unconventional and inconsistent with typical formatting. The attribute declaration should be on a single line: [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)]

Suggested change
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)
]
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property)]

Copilot uses AI. Check for mistakes.
public class OslcValueType : System.Attribute
{
Expand Down
84 changes: 58 additions & 26 deletions OSLC4Net_SDK/OSLC4Net.Core/Model/ResourceShapeFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,45 +145,71 @@ private static ResourceShape CreateResourceShape(string baseURI,
}
}

foreach (var propertyInfo in resourceType.GetProperties())
{
var propertyDefinitionAttribute = propertyInfo.GetCustomAttribute<OslcPropertyDefinition>();
if (propertyDefinitionAttribute != null)
{
var propertyDefinition = propertyDefinitionAttribute.value;
if (propertyDefinitions.Contains(propertyDefinition))
{
// throw new OslcCoreDuplicatePropertyDefinitionException(resourceType, propertyDefinitionAttribute);
// Skip duplicates (e.g. from getter/setter pairs if we scan them too, though here we scan properties)
continue;
Comment on lines +156 to +158
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The duplicate property definition check is commented out with a note about skipping duplicates. However, silently skipping duplicates can hide configuration errors. If properties and their backing getter/setter methods both have the same OslcPropertyDefinition, this should either throw an exception or log a warning, not silently continue.

Suggested change
// throw new OslcCoreDuplicatePropertyDefinitionException(resourceType, propertyDefinitionAttribute);
// Skip duplicates (e.g. from getter/setter pairs if we scan them too, though here we scan properties)
continue;
throw new OslcCoreDuplicatePropertyDefinitionException(resourceType, propertyDefinitionAttribute);

Copilot uses AI. Check for mistakes.
}

propertyDefinitions.Add(propertyDefinition);

var property = CreateProperty(baseURI, resourceType, propertyInfo,
propertyDefinitionAttribute, verifiedTypes);
resourceShape.AddProperty(property);
}
}

return resourceShape;
}

private static Property CreateProperty(string baseURI, Type resourceType, MethodInfo method,
private static Property CreateProperty(string baseURI, Type resourceType, MemberInfo member,
OslcPropertyDefinition propertyDefinitionAttribute, ISet<Type> verifiedTypes)
{
string name;
var nameAttribute = InheritedMethodAttributeHelper.GetAttribute<OslcName>(method);
var nameAttribute = GetAttribute<OslcName>(member);
if (nameAttribute != null)
{
name = nameAttribute.value;
}
else
{
name = GetDefaultPropertyName(method);
name = member is MethodInfo m ? GetDefaultPropertyName(m) : member.Name;
// lowercase first char for properties too? Usually yes for RDF properties
if (name.Length > 0 && char.IsUpper(name[0]))
{
name = char.ToLowerInvariant(name[0]) + name.Substring(1);
}
Comment on lines +184 to +188
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The inline comment "lowercase first char for properties too? Usually yes for RDF properties" suggests uncertainty about the implementation. This logic transforms property names to camelCase, but the comment indicates this may not be the correct approach in all cases. This should be verified and the comment either removed or turned into proper documentation explaining when and why this transformation occurs.

Copilot uses AI. Check for mistakes.
}

var propertyDefinition = propertyDefinitionAttribute.value;

if (!propertyDefinition.EndsWith(name, StringComparison.Ordinal))
{
throw new OslcCoreInvalidPropertyDefinitionException(resourceType, method,
propertyDefinitionAttribute);
// throw new OslcCoreInvalidPropertyDefinitionException(resourceType, member as MethodInfo, propertyDefinitionAttribute);
// Relaxed check or throw proper exception
Comment on lines +195 to +196
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The property definition validation is commented out and replaced with a silent no-op. The original code threw OslcCoreInvalidPropertyDefinitionException when the property definition didn't end with the property name. This validation should be restored or at least logged, as it catches configuration errors in OSLC property definitions.

Suggested change
// throw new OslcCoreInvalidPropertyDefinitionException(resourceType, member as MethodInfo, propertyDefinitionAttribute);
// Relaxed check or throw proper exception
throw new OslcCoreInvalidPropertyDefinitionException(resourceType, member as MethodInfo, propertyDefinitionAttribute);

Copilot uses AI. Check for mistakes.
Copy link
Member

@berezovskyi berezovskyi Dec 7, 2025

Choose a reason for hiding this comment

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

needs to be fixed properly

}

var returnType = method.ReturnType;
var returnType = member is MethodInfo mi ? mi.ReturnType : ((PropertyInfo)member).PropertyType;
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The cast ((PropertyInfo)member).PropertyType on line 199 is unsafe. If member is a MethodInfo, this cast will throw an InvalidCastException. While the ternary operator checks if it's a MethodInfo first, the logic could be clearer and safer. Consider: var returnType = member switch { MethodInfo mi => mi.ReturnType, PropertyInfo pi => pi.PropertyType, _ => throw new ArgumentException("Unsupported member type") };

Suggested change
var returnType = member is MethodInfo mi ? mi.ReturnType : ((PropertyInfo)member).PropertyType;
var returnType = member switch
{
MethodInfo mi => mi.ReturnType,
PropertyInfo pi => pi.PropertyType,
_ => throw new ArgumentException("Unsupported member type", nameof(member))
};

Copilot uses AI. Check for mistakes.
Occurs occurs;
var occursAttribute = InheritedMethodAttributeHelper.GetAttribute<OslcOccurs>(method);
var occursAttribute = GetAttribute<OslcOccurs>(member);
if (occursAttribute != null)
{
occurs = occursAttribute.value;
ValidateUserSpecifiedOccurs(resourceType, method, occursAttribute);
// ValidateUserSpecifiedOccurs(resourceType, member, occursAttribute); // Need overload
Copy link
Member

@berezovskyi berezovskyi Dec 7, 2025

Choose a reason for hiding this comment

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

needs to be fixed properly

}
else
{
occurs = GetDefaultOccurs(returnType);
}

var componentType = GetComponentType(resourceType, method, returnType);
var componentType = GetComponentType(resourceType, member as MethodInfo, returnType);
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

Passing member as MethodInfo to GetComponentType on line 212 can result in a null reference if member is actually a PropertyInfo. This could cause issues in GetComponentType if it doesn't handle null gracefully. Consider updating GetComponentType to accept MemberInfo or handle both cases explicitly.

Suggested change
var componentType = GetComponentType(resourceType, member as MethodInfo, returnType);
Type componentType;
if (member is MethodInfo methodInfo)
{
componentType = GetComponentType(resourceType, methodInfo, returnType);
}
else if (member is PropertyInfo propertyInfo)
{
// If GetComponentType can be overloaded to accept PropertyInfo, use that.
// Otherwise, pass null for MethodInfo and rely on returnType.
componentType = GetComponentType(resourceType, null, returnType);
}
else
{
throw new InvalidOperationException("Member is neither MethodInfo nor PropertyInfo.");
}

Copilot uses AI. Check for mistakes.

// Reified resources are a special case.
if (InheritedGenericInterfacesHelper.ImplementsGenericInterface(typeof(IReifiedResource<>),
Expand All @@ -204,34 +230,34 @@ private static Property CreateProperty(string baseURI, Type resourceType, Method
}

ValueType valueType;
var valueTypeAttribute = InheritedMethodAttributeHelper.GetAttribute<OslcValueType>(method);
var valueTypeAttribute = GetAttribute<OslcValueType>(member);
if (valueTypeAttribute != null)
{
valueType = valueTypeAttribute.value;
ValidateUserSpecifiedValueType(resourceType, method, valueType, componentType);
// ValidateUserSpecifiedValueType(resourceType, member, valueType, componentType);
Copy link
Member

@berezovskyi berezovskyi Dec 7, 2025

Choose a reason for hiding this comment

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

needs to be fixed properly

}
else
{
valueType = GetDefaultValueType(resourceType, method, componentType);
valueType = GetDefaultValueType(resourceType, member as MethodInfo, componentType);
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

Similarly, member as MethodInfo on line 241 is passed to GetDefaultValueType, which may not handle null properly if the member is a PropertyInfo. This pattern is repeated and could lead to runtime errors.

Suggested change
valueType = GetDefaultValueType(resourceType, member as MethodInfo, componentType);
MethodInfo methodInfo = member as MethodInfo;
if (methodInfo == null && member is PropertyInfo propertyInfo)
{
methodInfo = propertyInfo.GetGetMethod();
}
valueType = GetDefaultValueType(resourceType, methodInfo, componentType);

Copilot uses AI. Check for mistakes.
}

var property = new Property(name, occurs, new Uri(propertyDefinition), valueType);

property.SetTitle(property.GetName());
var titleAttribute = InheritedMethodAttributeHelper.GetAttribute<OslcTitle>(method);
var titleAttribute = GetAttribute<OslcTitle>(member);
if (titleAttribute != null)
{
property.SetTitle(titleAttribute.value);
}

var descriptionAttribute =
InheritedMethodAttributeHelper.GetAttribute<OslcDescription>(method);
GetAttribute<OslcDescription>(member);
if (descriptionAttribute != null)
{
property.SetDescription(descriptionAttribute.value);
}

var rangeAttribute = InheritedMethodAttributeHelper.GetAttribute<OslcRange>(method);
var rangeAttribute = GetAttribute<OslcRange>(member);
if (rangeAttribute != null)
{
foreach (var range in rangeAttribute.value)
Expand All @@ -241,12 +267,11 @@ private static Property CreateProperty(string baseURI, Type resourceType, Method
}

var representationAttribute =
InheritedMethodAttributeHelper.GetAttribute<OslcRepresentation>(method);
GetAttribute<OslcRepresentation>(member);
if (representationAttribute != null)
{
var representation = representationAttribute.value;
ValidateUserSpecifiedRepresentation(resourceType, method, representation,
componentType);
// ValidateUserSpecifiedRepresentation(resourceType, member, representation, componentType);
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

Multiple validation methods are commented out in the CreateProperty method (ValidateUserSpecifiedOccurs, ValidateUserSpecifiedValueType, ValidateUserSpecifiedRepresentation). This removes important validation logic that was present in the original method-based approach. Either these validations should be re-enabled with proper overloads that accept MemberInfo, or there should be a clear justification for why this validation is no longer necessary.

Suggested change
// ValidateUserSpecifiedRepresentation(resourceType, member, representation, componentType);
ValidateUserSpecifiedRepresentation(resourceType, member, representation, componentType);

Copilot uses AI. Check for mistakes.
Copy link
Member

@berezovskyi berezovskyi Dec 7, 2025

Choose a reason for hiding this comment

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

needs to be fixed properly

property.SetRepresentation(new Uri(RepresentationExtension.ToString(representation)));
}
else
Expand All @@ -260,7 +285,7 @@ private static Property CreateProperty(string baseURI, Type resourceType, Method
}

var allowedValueAttribute =
InheritedMethodAttributeHelper.GetAttribute<OslcAllowedValue>(method);
GetAttribute<OslcAllowedValue>(member);
if (allowedValueAttribute != null)
{
foreach (var allowedValue in allowedValueAttribute.value)
Expand All @@ -270,46 +295,46 @@ private static Property CreateProperty(string baseURI, Type resourceType, Method
}

var allowedValuesAttribute =
InheritedMethodAttributeHelper.GetAttribute<OslcAllowedValues>(method);
GetAttribute<OslcAllowedValues>(member);
if (allowedValuesAttribute != null)
{
property.SetAllowedValuesRef(new Uri(allowedValuesAttribute.value));
}

var defaultValueAttribute =
InheritedMethodAttributeHelper.GetAttribute<OslcDefaultValue>(method);
GetAttribute<OslcDefaultValue>(member);
if (defaultValueAttribute != null)
{
property.SetDefaultValue(defaultValueAttribute.value);
}

var hiddenAttribute = InheritedMethodAttributeHelper.GetAttribute<OslcHidden>(method);
var hiddenAttribute = GetAttribute<OslcHidden>(member);
if (hiddenAttribute != null)
{
property.SetHidden(hiddenAttribute.value);
}

var memberPropertyAttribute =
InheritedMethodAttributeHelper.GetAttribute<OslcMemberProperty>(method);
GetAttribute<OslcMemberProperty>(member);
if (memberPropertyAttribute != null)
{
property.SetMemberProperty(memberPropertyAttribute.value);
}

var readOnlyAttribute = InheritedMethodAttributeHelper.GetAttribute<OslcReadOnly>(method);
var readOnlyAttribute = GetAttribute<OslcReadOnly>(member);
if (readOnlyAttribute != null)
{
property.SetReadOnly(readOnlyAttribute.value);
}

var maxSizeAttribute = InheritedMethodAttributeHelper.GetAttribute<OslcMaxSize>(method);
var maxSizeAttribute = GetAttribute<OslcMaxSize>(member);
if (maxSizeAttribute != null)
{
property.SetMaxSize(maxSizeAttribute.value);
}

var valueShapeAttribute =
InheritedMethodAttributeHelper.GetAttribute<OslcValueShape>(method);
GetAttribute<OslcValueShape>(member);
if (valueShapeAttribute != null)
{
property.SetValueShape(new Uri(baseURI + "/" + valueShapeAttribute.value));
Expand All @@ -329,6 +354,13 @@ private static Property CreateProperty(string baseURI, Type resourceType, Method
return property;
}

private static T? GetAttribute<T>(MemberInfo member) where T : System.Attribute
{
return member is MethodInfo method
? InheritedMethodAttributeHelper.GetAttribute<T>(method)
: member.GetCustomAttribute<T>();
}

private static string GetDefaultPropertyName(MethodInfo method)
{
var methodName = method.Name;
Expand Down
8 changes: 0 additions & 8 deletions ...erTests/Snapshots/AllowedValuesSerializationTests.TestAllowedValuesRoundTrip.verified.ttl
View file
Open in desktop

This file was deleted.

27 changes: 0 additions & 27 deletions ...apshots/RdfXmlMediaTypeFormatterTests.TestRdfXmlCollectionSerializationAsync.verified.ttl
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions ...erTests/Snapshots/RdfXmlMediaTypeFormatterTests.TestRdfXmlSerializationAsync.verified.ttl
View file
Open in desktop

This file was deleted.

Loading
Loading