-
Notifications
You must be signed in to change notification settings - Fork 473
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.
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
Added structural types and properties documentation properties #2236
base: master
Are you sure you want to change the base?
Conversation
@henning-krause Thanks for contribution. Just For your information, did you see "OData/ModelBuilder#9". |
@xuzhg By all means, do that. Can I help to speed things up? |
@xuzhg since OData/ModelBuilder#9 has been merged, is there a chance to merge this also? I don't see anything of my code in the referenced PR. |
@xuzhg Any news on this PR? |
Hi @henning-krause -happy new year! Thanks for your contribution, and sorry to have taken so long to follow up. OData/ModelBuilder#9 provides the ability to implement patterns such as:
Is this sufficient for what you're trying to do? |
Hi @mikepizzo, I've just upgraded my solution to the latest odata libs (Microsoft.AspNet.OData 7.5.4 and Microsoft.OData.Core and Microsoft.OData.Edm 7.8.1) but I can't find the methods you're refering to. I'm working with the
It seems that I'm missing something here. |
It looks like OData/ModelBuilder#9 hasn't been backported to AspNet.OData 7.x. Let's try and do that first, so that developers using this feature in 7.x will have a consistent experience in 8.x. @g2mula -- Can you look at porting your OData/ModelBuilder#9 PR to this branch? |
|
||
if (edmType is IEdmStructuredType structuredType) | ||
{ | ||
foreach (var entry in structuredType.DeclaredProperties.Join(structuralType.Properties, p => p.Name, p => p.Name, (property, configuration) => new {property, configuration})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand this correctly, I think that maybe ToDictionary
is more appropriate than Join
. Wouldn't this be equivalent functionality, but a bit more optimized?
var structuredTypeProperties = structuredType.DeclaredProperties.ToDictionary(p => p.Name, p => p);
foreach (var structuralTypeProperty in structuralType.Properties)
{
if (!structuredTypeProperties .TryGetValue(structuralTypeProperty.Name, out var structuredTypeProperty)
{
continue;
}
if (!string.IsNullOrEmpty(structuralTypeProperty.Description)
{
model.SetDescriptionAnnotation(structuredTypeProperty, structuralTypeProperty.Description);
}
if (!string.IsNullOrEmpty(structuralTypeProperty.LongDescription)
{
model.SetDescriptionAnnotation(structuredTypeProperty, structuralTypeProperty.LongDescription);
}
}
public string LongDescription | ||
{ | ||
get => _longDescription; | ||
set => _longDescription = value ?? throw Error.PropertyNull(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that throw
ing is appropriate here because _longDescription
will be null
by default, so we aren't really protecting against anything
Issues
This PR adds documentation capabilities to the OdataConventionModelBuilder (as requested by #1732).
Description
The changes add new properties
Description
andLongDescription
toStructualTypeConfiguration
andPropertyConfiguration
. Additionally, a new AttributeDescriptionAttribute
is introduced to allow the developer to set the description via attributes on properties and classes.Using this, one can now utilize the C# XML documentation (Using the DocXml - C# XML documentation reader) for the entities:
Checklist (Uncheck if it is not completed)
Additional work necessary
If the PR is accepted, I would add this functionality to functions and actions and their parameters.