trjhtr

I have never created a web service before. I followed most of this Pluralsight tutorial to give me an idea of how to create one using ASP.NET MVC along with .NET Core 2.0 and Entity Framework Core.

The goal of this web service is to provide users with data from a database. It doesn't really do anything other than filter data down to what was requested and then return that data.

Here is an example request body:

 "buildIds": [
 "BuildId.1",
 "BuildId.2"
 ],
 "cRs": [
 100,
 400
 ]

The buildIds property is what is used to get "CRs". The cRs property is used to filter these CRs down to a specific set. The cRs property can be omitted if the user doesn't want to filter by anything.

This question is somewhat two-fold: I would like to know if there are any other cases for which I should handle certain things coming in as requests and also what you think of my code overall.

Controller:

[Route("api/metabuildCRs")]
public class MetabuildCRsController : Controller

 private IQSARepository _repository;

 public MetabuildCRsController(IQSARepository repository)
 
 _repository = repository;
 

 [HttpPost] // POST is used here because you can't send a body with GET
 public IActionResult GetMetabuildCrs([FromBody] MetabuildCRsRequest model)
 

Request object:

public class MetabuildCRsRequest

 public IEnumerable<string> BuildIds get; set; 
 public IEnumerable<int> CRs get; set; 

Repository (service layer):

public class QSARepository : IQSARepository

 private QSAContext _context;

 public QSARepository(QSAContext context)
 
 _context = context;
 

 public IEnumerable<string> GetImageBuildsInProductBuild(string buildId)
 
 return _context.SoftwareProductBuildCompositions.Where(x => x.SoftwareProductBuildId == buildId)?.Select(y => y.SoftwareImageBuildId);
 

 public SoftwareImageBuild GetSoftwareImageBuild(string buildId)
 
 return _context.SoftwareImageBuilds.FirstOrDefault(sib => sib.SoftwareImageBuildId == buildId);
 

 public IEnumerable<VerifySourceJobDetail> GetJobDetailsForSoftwareImageBuild(string buildId)
 
 var crNumbers = (from job in _context.VerifySourceJobs
 join details in _context.VerifySourceJobDetails on job.Id equals details.VerifySourceJobId
 where job.SoftwareImageBuildId == buildId
 select details).Distinct();

 return crNumbers;
 

 public CRException GetCRException(int crNumber, string softwareImage)
 
 return _context.CRExceptions.FirstOrDefault(e => e.ChangeRequestNumber == crNumber && e.SoftwareImage == softwareImage);
 

 public PrismCRDocument GetBulletinInformationForCR(int crNumber)
 
 return _context.PrismCRDocuments.FirstOrDefault(b => b.ChangeRequestNumber == crNumber);
 

 public IEnumerable<int> GetCRsThatDependOnCR(int crNumber)
 
 return from r in _context.PrismCRRelationships
 where r.ChangeRequestNumber2 == crNumber && r.Relationship == "DependsOn"
 select r.ChangeRequestNumber1;
 

Controllers should be kept as lean as possible. Consider adding another layer of abstraction specific the controller in order to separate concerns.

public interface IMetabuildCRsService 
 List<MetabuildCR> GetMetabuildCrs(IEnumerable<string> BuildIds, IEnumerable<int> CRs = null);

Its implementation will encapsulate the core functionality currently being done in the controller.

public class DefaultMetabuildCRsService : IMetabuildCRsService 
 private readonly IQSARepository repository;

 public DefaultMetabuildCRsService(IQSARepository repository) 
 this.repository = repository;
 

 public List<MetabuildCR> GetMetabuildCrs(IEnumerable<string> BuildIds, IEnumerable<int> CRs = null)
 var metabuildCRs = new List<MetabuildCR>();
 foreach (var productBuildId in BuildIds) 
 var imageBuildIds = repository.GetImageBuildsInProductBuild(productBuildId);
 foreach (var imageBuildId in imageBuildIds) 
 var crNumbers = repository.GetJobDetailsForSoftwareImageBuild(imageBuildId)?
 .Select(jd => jd.ChangeRequestNumber)
 .Distinct();

 if (CRs != null && CRs.Count() > 0) 
 // filter down to only crs we care about
 crNumbers = crNumbers.Where(cr => CRs.Contains(cr));
 

 var imageBuild = repository.GetSoftwareImageBuild(imageBuildId);
 foreach (var crNumber in crNumbers) 
 var bulletinInfo = repository.GetBulletinInformationForCR(crNumber);
 var exception = repository.GetCRException(crNumber, imageBuildId);
 var dependentCRs = repository.GetCRsThatDependOnCR(crNumber);

 metabuildCRs.Add(new MetabuildCR 
 ChangeRequestNumber = crNumber,
 // Build Info
 SoftwareImageBuildId = imageBuildId,
 BuildDate = imageBuild.CrmbuildDate,
 // Exception Info
 RequestText = exception?.RequestText,
 RequestedBy = exception?.RequestedBy,
 RequestedOn = exception?.RequestedOn,
 ExpiresOn = exception?.ExpiresOn,
 JiraIssueKey = exception?.JiraIssueKey,
 ReasonCode = exception?.ReasonCode,
 ResponseBy = exception?.ResponseBy,
 ResponseText = exception?.ResponseText,
 ResponseOn = exception?.ResponseOn,
 ExemptionNotes = exception?.Notes,
 //Bulletin Info
 SecurityBulletinDcn = bulletinInfo?.SecurityBulletinDcn,
 DocumentType = bulletinInfo?.DocumentType,
 DocumentReleaseDate = bulletinInfo?.DocumentReleaseDate,

 DependentCRs = dependentCRs
 );
 
 
 
 return metabuildCRs;
 

This simplifies the controller to

[Route("api/metabuildCRs")]
public class MetabuildCRsController : Controller 
 private readonly IMetabuildCRsService service;

 public MetabuildCRsController(IMetabuildCRsService service) 
 this.service = service;
 

 [HttpPost]
 public IActionResult GetMetabuildCrs([FromBody] MetabuildCRsRequest model) 

If anything changes in the core functionality then there is no need to touch the controller as it is performing its Single Responsibility of handling requests.

The service can be modified independently of the controller. It can also be reused elsewhere if needed.

I am personally not a big fan of using underscore prefixes on variable names, so will notice that I removed them all.

As for your concern about additional functionality, they can be isolated to their own service abstraction and added to this controller or its own controller depending on your choice. Splitting functionality into small easy to maintain modules helps separate concerns within the application and allows the code to grow softly