From febfa8e7e9faf9e2d583014d9f9d4feecdba8716 Mon Sep 17 00:00:00 2001 From: Vincent He Date: Thu, 1 Sep 2016 15:56:58 +0800 Subject: [PATCH] Address public API review comments --- src/Microsoft.Restier.Core/ApiBaseExtensions.cs | 6 +++--- src/Microsoft.Restier.Core/PropertyBag.cs | 4 ++-- .../Submit/ChangeSetInitializer.cs | 1 + .../Model/OperationAttribute.cs | 17 ++++++----------- .../Model/ResourceAttribute.cs | 6 +----- .../Model/RestierModelExtender.cs | 10 +++++----- .../Model/RestierOperationModelBuilder.cs | 2 +- .../PropertyBag.Tests.cs | 2 +- .../Model/RestierModelExtender.Tests.cs | 8 ++++---- test/Microsoft.Restier.TestCommon/PublicApi.bsl | 13 +++++-------- .../Api/TrippinApi.cs | 8 ++++++-- ...icrosoft.OData.Service.Sample.Trippin.csproj | 1 + .../Api/TrippinApi.cs | 2 +- 13 files changed, 37 insertions(+), 43 deletions(-) diff --git a/src/Microsoft.Restier.Core/ApiBaseExtensions.cs b/src/Microsoft.Restier.Core/ApiBaseExtensions.cs index ecde1601..5f3be300 100644 --- a/src/Microsoft.Restier.Core/ApiBaseExtensions.cs +++ b/src/Microsoft.Restier.Core/ApiBaseExtensions.cs @@ -143,7 +143,7 @@ public static void SetProperty(this ApiBase api, string name, object value) } /// - /// Clears a property. + /// Removes a property. /// /// /// An API. @@ -151,9 +151,9 @@ public static void SetProperty(this ApiBase api, string name, object value) /// /// The name of a property. /// - public static void ClearProperty(this ApiBase api, string name) + public static void RemoveProperty(this ApiBase api, string name) { - api.GetPropertyBag().ClearProperty(name); + api.GetPropertyBag().RemoveProperty(name); } #endregion diff --git a/src/Microsoft.Restier.Core/PropertyBag.cs b/src/Microsoft.Restier.Core/PropertyBag.cs index 62df5d74..c048eac0 100644 --- a/src/Microsoft.Restier.Core/PropertyBag.cs +++ b/src/Microsoft.Restier.Core/PropertyBag.cs @@ -87,12 +87,12 @@ public void SetProperty(string name, object value) } /// - /// Clears a property. + /// Removes a property. /// /// /// The name of a property. /// - public void ClearProperty(string name) + public void RemoveProperty(string name) { Ensure.NotNull(name, "name"); this.properties.Remove(name); diff --git a/src/Microsoft.Restier.Providers.EntityFramework/Submit/ChangeSetInitializer.cs b/src/Microsoft.Restier.Providers.EntityFramework/Submit/ChangeSetInitializer.cs index 4918a3ac..8d48a455 100644 --- a/src/Microsoft.Restier.Providers.EntityFramework/Submit/ChangeSetInitializer.cs +++ b/src/Microsoft.Restier.Providers.EntityFramework/Submit/ChangeSetInitializer.cs @@ -254,6 +254,7 @@ private void SetValues(object instance, Type type, IReadOnlyDictionary - /// Gets or sets the name of the operation. - /// The default name is same as method name. - /// - public string Name { get; set; } - /// /// Gets or sets the namespace of the operation. /// The default value will be same as the namespace of entity type. @@ -32,7 +26,7 @@ public sealed class OperationAttribute : Attribute /// /// Gets or sets a value indicating whether the function is composable. - /// The default value is false + /// The default value is false. /// public bool IsComposable { get; set; } @@ -40,15 +34,16 @@ public sealed class OperationAttribute : Attribute /// Gets or sets a value indicating whether the operation is bound or not. /// If it is set to true, then no matter what's the first parameter, it will be considered as bound. /// If it is set to false, then no matter what's the first parameter, it will be considered as unbound. - /// The default value is false + /// The default value is false. /// public bool IsBound { get; set; } /// /// Gets or sets a value indicating whether the operation has side effects. - /// If a operation does not have side effect, it means it is a function. - /// If a operation has side effect, it means it is a action. - /// The default value is false + /// If an operation does not have side effect, it means it is a function, + /// and need to use HTTP Get to call function. + /// If an operation has side effect, it means it is an action, and need to use HTTP post to call action. + /// The default value is false. /// public bool HasSideEffects { get; set; } } diff --git a/src/Microsoft.Restier.Publishers.OData/Model/ResourceAttribute.cs b/src/Microsoft.Restier.Publishers.OData/Model/ResourceAttribute.cs index 5c79ce96..f8cbb199 100644 --- a/src/Microsoft.Restier.Publishers.OData/Model/ResourceAttribute.cs +++ b/src/Microsoft.Restier.Publishers.OData/Model/ResourceAttribute.cs @@ -7,15 +7,11 @@ namespace Microsoft.Restier.Publishers.OData.Model { /// /// Attribute that indicates a property is an entity set or singleton. + /// If the property type is IQueryable, it will be built as entity set or it will be built as singleton. /// The name will be same as property name. /// [AttributeUsage(AttributeTargets.Property)] public sealed class ResourceAttribute : Attribute { - /// - /// Gets or sets a value indicating whether it is singleton or entity set. - /// The default value is false means it is an entity set - /// - public bool IsSingleton { get; set; } } } diff --git a/src/Microsoft.Restier.Publishers.OData/Model/RestierModelExtender.cs b/src/Microsoft.Restier.Publishers.OData/Model/RestierModelExtender.cs index ee42aafd..af7126fc 100644 --- a/src/Microsoft.Restier.Publishers.OData/Model/RestierModelExtender.cs +++ b/src/Microsoft.Restier.Publishers.OData/Model/RestierModelExtender.cs @@ -186,9 +186,9 @@ private void BuildEntitySetsAndSingletons(EdmModel model) continue; } - bool isSingleton = resourceAttribute.IsSingleton; - if ((!isSingleton && !IsEntitySetProperty(property)) - || (isSingleton && !IsSingletonProperty(property))) + bool isEntitySet = IsEntitySetProperty(property); + bool isSingleton = IsSingletonProperty(property); + if (!isSingleton && !isEntitySet) { // This means property type is not IQueryable when indicating an entityset // or not non-generic type when indicating a singleton @@ -196,7 +196,7 @@ private void BuildEntitySetsAndSingletons(EdmModel model) } var propertyType = property.PropertyType; - if (!isSingleton) + if (isEntitySet) { propertyType = propertyType.GetGenericArguments()[0]; } @@ -209,7 +209,7 @@ private void BuildEntitySetsAndSingletons(EdmModel model) } var container = model.EnsureEntityContainer(this.targetType); - if (!isSingleton) + if (isEntitySet) { if (container.FindEntitySet(property.Name) == null) { diff --git a/src/Microsoft.Restier.Publishers.OData/Model/RestierOperationModelBuilder.cs b/src/Microsoft.Restier.Publishers.OData/Model/RestierOperationModelBuilder.cs index 7d599ae1..ae1b3a70 100644 --- a/src/Microsoft.Restier.Publishers.OData/Model/RestierOperationModelBuilder.cs +++ b/src/Microsoft.Restier.Publishers.OData/Model/RestierOperationModelBuilder.cs @@ -233,7 +233,7 @@ private class OperationMethodInfo public string Name { - get { return this.OperationAttribute.Name ?? this.Method.Name; } + get { return this.Method.Name; } } public string Namespace diff --git a/test/Microsoft.Restier.Core.Tests/PropertyBag.Tests.cs b/test/Microsoft.Restier.Core.Tests/PropertyBag.Tests.cs index d9e55bd0..fea3e3e8 100644 --- a/test/Microsoft.Restier.Core.Tests/PropertyBag.Tests.cs +++ b/test/Microsoft.Restier.Core.Tests/PropertyBag.Tests.cs @@ -26,7 +26,7 @@ public void PropertyBagManipulatesPropertiesCorrectly() Assert.Equal("Test", api.GetProperty("Test")); Assert.Equal("Test", api.GetProperty("Test")); - api.ClearProperty("Test"); + api.RemoveProperty("Test"); Assert.False(api.HasProperty("Test")); Assert.Null(api.GetProperty("Test")); Assert.Null(api.GetProperty("Test")); diff --git a/test/Microsoft.Restier.Publishers.OData.Test/Model/RestierModelExtender.Tests.cs b/test/Microsoft.Restier.Publishers.OData.Test/Model/RestierModelExtender.Tests.cs index a72c81c0..8414a3b7 100644 --- a/test/Microsoft.Restier.Publishers.OData.Test/Model/RestierModelExtender.Tests.cs +++ b/test/Microsoft.Restier.Publishers.OData.Test/Model/RestierModelExtender.Tests.cs @@ -196,7 +196,7 @@ public class ApiA : BaseApi { [Resource] public IQueryable People { get; set; } - [Resource(IsSingleton = true)] + [Resource] public Person Me { get; set; } public IQueryable Invisible { get; set; } @@ -236,7 +236,7 @@ public class ApiC : ApiB { [Resource] public new IQueryable Customers { get; set; } - [Resource(IsSingleton = true)] + [Resource] public new Customer Me { get; set; } public ApiC(IServiceProvider serviceProvider) : base(serviceProvider) @@ -301,11 +301,11 @@ public ApiG(IServiceProvider serviceProvider) : base(serviceProvider) public class ApiH : BaseApi { - [Resource(IsSingleton = true)] + [Resource] public Person Me { get; set; } [Resource] public IQueryable Customers { get; set; } - [Resource(IsSingleton = true)] + [Resource] public Customer Me2 { get; set; } public static new IServiceCollection ConfigureApi(Type apiType, IServiceCollection services) diff --git a/test/Microsoft.Restier.TestCommon/PublicApi.bsl b/test/Microsoft.Restier.TestCommon/PublicApi.bsl index 00fbb3d3..251229ea 100644 --- a/test/Microsoft.Restier.TestCommon/PublicApi.bsl +++ b/test/Microsoft.Restier.TestCommon/PublicApi.bsl @@ -25,11 +25,6 @@ public abstract class Microsoft.Restier.Core.ApiBase : IDisposable { ExtensionAttribute(), ] public sealed class Microsoft.Restier.Core.ApiBaseExtensions { - [ - ExtensionAttribute(), - ] - public static void ClearProperty (Microsoft.Restier.Core.ApiBase api, string name) - [ ExtensionAttribute(), ] @@ -87,6 +82,11 @@ public sealed class Microsoft.Restier.Core.ApiBaseExtensions { ] public static System.Threading.Tasks.Task`1[[Microsoft.Restier.Core.Query.QueryResult]] QueryAsync (Microsoft.Restier.Core.ApiBase api, Microsoft.Restier.Core.Query.QueryRequest request, params System.Threading.CancellationToken cancellationToken) + [ + ExtensionAttribute(), + ] + public static void RemoveProperty (Microsoft.Restier.Core.ApiBase api, string name) + [ ExtensionAttribute(), ] @@ -656,7 +656,6 @@ public sealed class Microsoft.Restier.Publishers.OData.Model.OperationAttribute bool HasSideEffects { [CompilerGeneratedAttribute(),]public get; [CompilerGeneratedAttribute(),]public set; } bool IsBound { [CompilerGeneratedAttribute(),]public get; [CompilerGeneratedAttribute(),]public set; } bool IsComposable { [CompilerGeneratedAttribute(),]public get; [CompilerGeneratedAttribute(),]public set; } - string Name { [CompilerGeneratedAttribute(),]public get; [CompilerGeneratedAttribute(),]public set; } string Namespace { [CompilerGeneratedAttribute(),]public get; [CompilerGeneratedAttribute(),]public set; } } @@ -665,7 +664,5 @@ AttributeUsageAttribute(), ] public sealed class Microsoft.Restier.Publishers.OData.Model.ResourceAttribute : System.Attribute, _Attribute { public ResourceAttribute () - - bool IsSingleton { [CompilerGeneratedAttribute(),]public get; [CompilerGeneratedAttribute(),]public set; } } diff --git a/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Api/TrippinApi.cs b/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Api/TrippinApi.cs index a44f1841..32600ada 100644 --- a/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Api/TrippinApi.cs +++ b/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Api/TrippinApi.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations.Schema; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -18,6 +19,7 @@ using Microsoft.Restier.Providers.EntityFramework; using Microsoft.Restier.Publishers.OData.Model; using System.Reflection; +using System.Runtime.Serialization; using System.Web.OData.Builder; using Microsoft.OData.Edm.Vocabularies; @@ -25,12 +27,14 @@ namespace Microsoft.OData.Service.Sample.Trippin.Api { public class TrippinApi : EntityFrameworkApi { + [NotMapped] + [IgnoreDataMember] public TrippinModel ModelContext { get { return DbContext; } } - [Resource(IsSingleton = true)] + [Resource] public Person Me { get @@ -85,7 +89,7 @@ public IQueryable PeopleWithAge1 } } - [Resource(IsSingleton = true)] + [Resource] public PersonWithAge PeopleWithAgeMe { get diff --git a/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Microsoft.OData.Service.Sample.Trippin.csproj b/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Microsoft.OData.Service.Sample.Trippin.csproj index c3079674..c229bce3 100644 --- a/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Microsoft.OData.Service.Sample.Trippin.csproj +++ b/test/ODataEndToEnd/Microsoft.OData.Service.Sample.Trippin/Microsoft.OData.Service.Sample.Trippin.csproj @@ -146,6 +146,7 @@ ..\..\..\packages\Microsoft.AspNet.WebApi.Client.5.2.3\lib\net45\System.Net.Http.Formatting.dll True + diff --git a/test/ODataEndToEnd/Microsoft.OData.Service.Sample.TrippinInMemory/Api/TrippinApi.cs b/test/ODataEndToEnd/Microsoft.OData.Service.Sample.TrippinInMemory/Api/TrippinApi.cs index 21796a0c..f93b224c 100644 --- a/test/ODataEndToEnd/Microsoft.OData.Service.Sample.TrippinInMemory/Api/TrippinApi.cs +++ b/test/ODataEndToEnd/Microsoft.OData.Service.Sample.TrippinInMemory/Api/TrippinApi.cs @@ -70,7 +70,7 @@ public IQueryable NewComePeople } } - [Resource(IsSingleton = true)] + [Resource] public Person Me { get