-
Notifications
You must be signed in to change notification settings - Fork 352
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
Retrieve the content type for stream property from vocabulary annotation by convention #1995
Conversation
var mediaTypes = this.MetadataContext.Model.GetVocabularyStringCollection(entityType, CoreVocabularyModel.AcceptableMediaTypesTerm); | ||
if (mediaTypes != null) | ||
{ | ||
// TODO: Make sure we use ',' to concatant the multiple media types or pick the first one? |
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.
List doesn't make sense for the returned ContentType.
This should use the first item.
So the convention behavior could only work when the acceptable types is a collection of one, which will be a fairly common case. #Resolved
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.
OData spec says: http://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_UpdateStreamValues:
Stream properties MAY specify a list of acceptable media types using an annotation with term Core.AcceptableMediaTypes, see [OData-VocCore].
http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_MediaEntityType
:
Media entity types MAY specify a list of acceptable media types using an annotation with term Core.AcceptableMediaTypes, see [OData‑VocCore].
The AcceptableMediaTypes is defined as COLLECTION of media type.
<Term Name="AcceptableMediaTypes" Type="Collection(Edm.String)" Nullable="false" AppliesTo="EntityType Property Term" />
But, there's no Executable instruction about how to format it into "ContentType" of stream (entity/property). #WontFix
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.
AcceptableMediaTypes should return a collection of media types, which is the set of media types that the media entity stream/stream property may contain. If this collection contains a single media type (which is likely to be a common case), then we should set the Content-Type to be that single media type. As Gareth says, we should not attempt to set Content-Type to multiple mediatypes; that's invalid as per the definition of the Content-Type header.
In reply to: 570412053 [](ancestors = 570412053)
var mediaTypes = this.MetadataContext.Model.GetVocabularyStringCollection(edmProperty, CoreVocabularyModel.AcceptableMediaTypesTerm); | ||
if (mediaTypes != null) | ||
{ | ||
// TODO: Make sure we use ',' to concatant the multiple media types or pick the first one? |
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.
see comment above. #Resolved
if (mediaTypes != null) | ||
{ | ||
// TODO: Make sure we use ',' to concatant the multiple media types or pick the first one? | ||
streamPropertyValue.ContentType = string.Join(",", mediaTypes); |
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.
See previous comment. #Resolved
/// <summary> | ||
/// Extension methods for the vocabulary annotations. | ||
/// </summary> | ||
public static class VocabularyAnnotationExtensions |
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.
So glad you made this public :-) I can use this. #Closed
/// <returns>Null or a collection string of qualified type name.</returns> | ||
public static IEnumerable<string> GetVocabularyStringCollection(this IEdmModel model, IEdmVocabularyAnnotatable target, IEdmTerm term) | ||
{ | ||
if (model == null || target == null || term == null) |
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 would expect argumentnullexceptions rather than returning null in a public API. #Resolved
} | ||
} | ||
|
||
return null; |
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 would expect Enumerable.Empty rather than null for any API that returns an IEnumerable when there aren't any items. #Resolved
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.
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.
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.
0458637
to
bc9d6e4
Compare
…ion by default
Issues
OData spec says:
6.4 Media Entity Type
An entity type that does not specify a base type MAY specify that it is a media entity type. Media entities are entities that represent a media stream, such as a photo. Use a media entity if the out-of-band stream is the main topic of interest and the media entity is just additional structured information attached to the stream. Use a normal entity with one or more properties of type Edm.Stream if the structured data of the entity is the main topic of interest and the stream data is just additional information attached to the structured data. For more information on media entities see [OData‑Protocol].
An entity type derived from a media entity type MUST indicate that it is also a media entity type.
Media entity types MAY specify a list of acceptable media types using an annotation with term Core.AcceptableMediaTypes, see [OData‑VocCore].
and
Stream properties MAY specify a list of acceptable media types using an annotation with term Core.AcceptableMediaTypes, see [OData-VocCore].
This PR is indented to retrieve the media types from the vocabulary annotation.
Description
Briefly describe the changes of this pull request.
Checklist (Uncheck if it is not completed)
Additional work necessary
If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.