Skip to content
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

[Feature request] Allow interface inheritance to extend service #62

Closed
mateusz-pietrzak opened this issue Feb 24, 2020 · 8 comments
Closed

Comments

@mateusz-pietrzak
Copy link

Would it be possible to have [ServiceContract] interface inherit another generic interface in order to reduce code duplication for simple CRUD services and allow generic reusage in components? Based on this example:

    [ServiceContract(Name = "MyApp.TestDataService")]
    public interface ITestDataService : IDataService<TestDto>
    {

    }

    public interface IDataService<TDto> where TDto : class, new()
    {
        ValueTask<List<TDto>> GetAllAsync(CallContext context = default);
        ValueTask<TDto> GetByIdAsync(IdMessage message, CallContext context = default);
        ValueTask<TDto> AddAsync(TDto dto, CallContext context = default);
        ValueTask<TDto> UpdateAsync(TDto dto, CallContext context = default);
        ValueTask DeleteAsync(TDto dto, CallContext context = default);
    }

    [ProtoContract]
    public class TestDto
    {
        [ProtoMember(1)]
        public int Id { get; set; }

        [ProtoMember(2)]
        public string Name { get; set; }
    }

    [ProtoContract]
    public class IdMessage
    {
        [ProtoMember(1)]
        public int Id { get; set; }
    }

If interface that service marked with [ServiceContract] inherits (in this case IDataService) does not implement [ServiceContract] then it's methods could be incorporated into outmost service (ITestDataService) with it's address (in this case MyApp.TestDataService). Alternatively introduce new attribute, something like [ServiceContractExtension] which would allow this functionality without breaking the way it works at the moment. Currently those methods would raise NotSupportedException since IDataService is not marked with [ServiceContract].

@gustav3d
Copy link

I was a bout to post a bout this issue too ! I'm happy to see im not alone in needing this:)

@TieSKey
Copy link

TieSKey commented Feb 26, 2020

Since the library's code checks the Attribute by name you could implement your own ServiceContractAttribute class and enable inheritance. I tested it while trying to make it work in Unity3D since for some reason I couldn't import the required dll.
Ofc I wouldn't recommend such approach unless u are super sure and willing to face future consequences.

@mateusz-pietrzak
Copy link
Author

mateusz-pietrzak commented Feb 26, 2020

@TieSKey Not that simple I'm afraid. Nevertheless, I was able to fork it and make it work for me by adding few lines in ProxyEmitter:

...
bool isService = binderConfig.Binder.IsServiceContract(iType, out var serviceName);

if (!isService)
{
      isService = binderConfig.Binder.IsServiceContract(typeof(TService), out serviceName);
 }
...

and ServerBinder:

...

var immediateServices = serviceContracts
      .Except(serviceContracts.SelectMany(t => t.GetInterfaces()))
      .Where(t => binderConfiguration.Binder.IsServiceContract(t, out _));

...

if (!binderConfiguration.Binder.IsServiceContract(serviceContract, out serviceName))
{
      var parentService = immediateServices.FirstOrDefault(s => serviceContract.IsAssignableFrom(s));

      if (parentService is null)
      {
            continue;
      }
      else
      {
             binderConfiguration.Binder.IsServiceContract(parentService, out serviceName);
      }
}

...

I wonder if this kind of functionality could be baked in, perhaps as opt-in via attribute.

@jonorogers
Copy link

+1 for this, this would help cut down a lot on duplicate code

@mateusz-pietrzak
Copy link
Author

Sorry for quick bump, but I would like to know if that it's something that might be explored in the future. This library seems perfect for my usage as it allows for easy DataAnnotations interceptor validation on both Server side and Blazor WASM (as you cannot easily add attributes to messages generated from standard GRPC lib).

I'm currently using version 1.0.22 with mentioned above changes that allow for very simple inheritance (I'm aware that all services in "inheritance chain" will be propagated to the topmost service, but that's still better than nothing for my use case) and it's working quite fine.

However I would like to know if that kind of inheritance might be included in the future releases in order to make sure that I can avoid patching every single release to enable it.

@mgravell
Copy link
Member

mgravell commented Mar 9, 2020

@mateusz-pietrzak yes, I think we'll definitely get something along these lines into the lib; I just need to find time to think through all the edges and corners here, etc

@periapsistech
Copy link

Any updates on this?

@mgravell
Copy link
Member

This is supported via [SubService], which would be applied to the inner interface, i.e. IDataService<TDto>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants