Versioning service layer with inheritance

Question

I have a .net core 3.1 api and I want to version my controllers and I think some structure for versioning on service layer like below

    public interface IVersionableObject { }
    
    public class GetDataV1 : IVersionableObject { }
    
    public class PostDataV1 : IVersionableObject { }
    
    public class GetDataV2 : IVersionableObject { }
    
    public class PostDataV2 : IVersionableObject { }
    
    public class ListItemV1 : IVersionableObject { }

    public class MobileAppServiceV1
    {
        public virtual async Task<IVersionableObject> Get()
        {
            return new GetDataV1();
        }

    public virtual async Task<IVersionableObject> Post()
    {
        return new PostDataV1();
    }

    public virtual async Task<IVersionableObject> ListItems()
    {
        return new ListItemV1();
    }
}

public class MobileAppServiceV2 : MobileAppServiceV1
{
    public override async Task<IVersionableObject> Get()
    {
        return new GetDataV2();
    }

    public override async Task<IVersionableObject> Post()
    {
        return new PostDataV2();
    }

    [Obsolete("This method is not available for after V1" , true)]
    public async Task<IVersionableObject> ListItems()
    {
        throw new NotSupportedException("This method is not available for after V1");
    }
}

Lets check Controller

Controller for V1

    [ApiVersion("1.0")]
    [Route("api/{v:apiVersion}/values")]
    public class ValuesControllerV1 : ControllerBase
    {
        private readonly MobileAppServiceV1 _mobileAppServiceV1;

    public ValuesControllerV1()
    {
        _mobileAppServiceV1 = new MobileAppServiceV1();
    }

    [HttpGet]
    public async Task<IActionResult> Get()
    {
        return Ok(await _mobileAppServiceV1.Get());
    }

    [HttpGet("listItem")]
    public async Task<IActionResult> ListItems()
    {
        return Ok(await _mobileAppServiceV1.ListItems());
    }

    [HttpPost]
    public async Task<IActionResult> Post([FromBody] string value)
    {
        return Ok(await _mobileAppServiceV1.Post());
    }

}

Controller for V2

    [ApiVersion("2.0")]
    [Route("api/{v:apiVersion}/values")]
    public class ValuesControllerV2 : ControllerBase
    {
        private readonly MobileAppServiceV2 _mobileAppServiceV2;

        public ValuesControllerV2()
        {
            _mobileAppServiceV2 = new MobileAppServiceV2();
        }

        [HttpGet]
        public async Task<IActionResult> Get()
        {
            return Ok(await _mobileAppServiceV2.Get());
        }

        [HttpPost]
        public async Task<IActionResult> Post([FromBody] string value)
        {
            return Ok(await _mobileAppServiceV2.Post());
        }

    }

For example ListItems method removed on v2 , I avoid to use ListItem method on v2 with Obselete attribute.

Finally I think structure something like this and I try to show it with sample code.Can you give some idea about this is good structure or not for versioning service layer on web api? I am open to all suggestions.

Answer 1

While you can certainly go this direction, it's not one I recommend. Inheritance is not the right way to think about the problem in my opinion. HTTP has no concept of inheritance. There are a number of problems and challenges to making it work. If your goal is to share common code, then you have several other options such as:

• Inherit protected methods that are not actions; expose them with public actions as appropriate
• Delegate as much of the non-HTTP logic to code outside of controllers
• Use extension methods or other types of extensions to share internal controller logic

The [Obsolete] attribute will not do what you hope for it to do. While it's true it will result in a compilation error as shown, why not just delete the method? The only edge case is if you are inheriting across multiple assemblies, which is even more complex. If completely removing the original code is not an option, then a better approach is to decorate obsolete methods with [NonAction] so that it is no longer visible to ASP.NET.

Variation 1

Use protected methods to share logic.

[ApiController]
public abstract class ApiController : ControllerBase
{
 protected async virtual Task<IActionResult> GetAll(CancellationToken cancellationToken)
 {
    // TODO: implementation
    await Task.Yield();
    return Ok();
 }

 protected async virtual Task<IActionResult> GetOne(int id, CancellationToken cancellationToken)
 {
    // TODO: implementation
    await Task.Yield();
    return Ok();
 }
}

[ApiVersion("1.0")]
[Route("[controller]")]
public class MobileController : ApiController
{
   [HttpGet("list")]
   public Task<IActionResult> Get(CancellationToken cancellationToken) =>
     GetAll(cancellationToken);

   [HttpGet("{id}")]
   public Task<IActionResult> Get(int id, CancellationToken cancellationToken) =>
     GetOne(id, cancellationToken);
}

[ApiVersion("2.0")]
[Route("[controller]")]
public class Mobile2Controller : ApiController
{
   [HttpGet("list")]
   public Task<IActionResult> Get(CancellationToken cancellationToken) =>
     GetAll(cancellationToken);

   [HttpGet("{id:int}")] // new route constraint, but could be alt implementation
   public Task<IActionResult> Get(int id, CancellationToken cancellationToken) =>
     GetOne(id, cancellationToken);
}

Variation 2

Move non-API logic out of the controller.

public interface IRepository<T>
{
    IAsyncEnumerable<T> GetAll(CancellationToken cancellationToken);
    Task<T> GetOne(int id, CancellationToken cancellationToken);
}

// TODO: implement IRepository<T>

// NOTE: a generic base class 'could' be used for common logic

[ApiVersion("1.0")]
[ApiController]
[Route("[controller]")]
public class MobileController : ControllerBase
{
   readonly IRepository<MobileApp> repository;

   public MobileController(IRepository<MobileApp> repository) =>
     this.repository = repository;

   [HttpGet("list")]
   public IAsyncEnumerable<MobileApp> Get(CancellationToken cancellationToken) =>
     repository.GetAll(cancellationToken);

   [HttpGet("{id}")]
   public async Task<IActionResult> Get(int id, CancellationToken cancellationToken)
   {
     var model = await repository.GetOne(id, cancellationToken);

     if (model == null)
     {
         return NotFound();
     }

     return Ok(model);
   }
}

[ApiVersion("2.0")]
[ApiController]
[Route("[controller]")]
public class Mobile2Controller : ControllerBase
{
   readonly IRepository<MobileApp2> repository;

   public Mobile2Controller(IRepository<MobileApp2> repository) =>
     this.repository = repository;

   [HttpGet("list")]
   public IAsyncEnumerable<MobileApp2> Get(CancellationToken cancellationToken) =>
     repository.GetAll(cancellationToken);

   [HttpGet("{id}")]
   public async Task<IActionResult> Get(int id, CancellationToken cancellationToken)
   {
     var model = await repository.GetOne(id, cancellationToken);

     if (model == null)
     {
         return NotFound();
     }

     return Ok(model);
   }
}

Variant 3

Use attributes to ignore old methods. I don't recommend this appropriate as it makes it messy to maintain over time.

[ApiVersion("1.0")]
[ApiController]
[Route("[controller]")]
public class MobileController : ControllerBase
{
   [HttpGet("list")]
   public virtual async Task<IActionResult> Get(CancellationToken cancellationToken)
   {
      await Task.Yield();
      return Ok();
   }

   [HttpGet("{id}")]
   public virtual async Task<IActionResult> Get(int id, CancellationToken cancellationToken)
   {
      await Task.Yield();
      return Ok();
   }
}

[ApiVersion("2.0")]
[Route("[controller]")]
public class Mobile2Controller : MobileController
{
   [NonAction] // exclude method as action
   public override Task<IActionResult> Get(int id, CancellationToken cancellationToken) =>
       Task.FromResult<IActionResult>(Ok());

   [HttpGet("{id:guid}")]
   public virtual async Task<IActionResult> GetV2(Guid id, CancellationToken cancellationToken)
   {
      await Task.Yield();
      return Ok();
   }
}

Conclusion

These are just a few possibilities, but there are others. Inheritance tends to make things unclear and harder to manage over time. You need to know/remember which attributes are inherited and which aren't. When looking at the source or debugging, you will likely jump across multiple files. It may not even be clear where to set a breakpoint.

You should be defining a well-known versioning policy upfront. A common policy is N-2 versions. If you adhere to that policy, then the idea of duplicating the source of controllers isn't all that bad (e.g. 3 times). As demonstrated, there are numerous techniques that you can use to reduce code duplication within the controllers.