How to create QueryBuilder to work with concurrent requests

Question

I need assistance with the QueryBuilder that generates OData query. Following is my implementation and it approach has couple of issues

• If user forgets to SetRootTable then it will cause serious issues.

• Task.WhenAll since it is a single QueryBuilder instance and queries gets corrupted.

Please suggest alternatives to be able generate OData queries through easy to use methods instead of having a long string and be able to unit test the Service class.

public interface IQueryBuilder
{
 IQueryBuilder SetRootTable(string rootTableName);
 IQueryBuilder Select(string selectExpression);
 IQueryBuilder Filter(string filterExpression);
 
 string Build();
}
public class QueryBuilder: IQueryBuilder
{
 private StringBuilder query;
 public QueryBuilder()
 {
 query = new StringBuilder();
 }
 public IODataQueryBuilder SetRootTable(string rootTableName)
 {
 query.Clear();
 query.Append(rootTableName);
 return this;
 }
 public IODataQueryBuilder Select(string selectExpression)
 {
 if (!string.IsNullOrWhiteSpace(selectExpression))
 {
 AppendSeparator();
 query.Append($"$select={selectExpression}");
 }
 return this;
 }
 public IODataQueryBuilder Filter(string filterExpression)
 {
 if (!string.IsNullOrWhiteSpace(filterExpression))
 {
 AppendSeparator();
 query.Append($"$filter={filterExpression}");
 }
 return this;
 }
 
 public string Build()
 {
 return query.ToString();
 }
 private void AppendSeparator()
 {
 if (query.ToString().Contains("?"))
 {
 query.Append("&");
 }
 else
 {
 query.Append("?");
 }
 }
}
public class CustomerService
{
 public CustomerService(IDBService dbService, IQueryBuilder queryBuilder) {
 this.dbService = dbService;
 this.queryBuilder = queryBuilder;
 }
 public List<Customers> GetCustomers() {
 .....
 // will this create a problem since query in QueryBuilder is shared
 await Task.WhenAll(customers.Select(async c => c.Verified = await IsVerified(c.customerId))); 
 ...
 }
 public async Task<bool> IsVerified(int customerId)
 {
 var query = queryBuilder
 .SetRootTable("Customers")
 .Select("id,name")
 .Filter($"customerid eq {customerId}").Build();
 return Convert.ToInt32(await dbService.GetResponseAsync(query)) > 0;
 }
}

• amon: This would be easy to solve if you get rid of IQueryBuilder (do you really need dependency injection for this class??), create a new query builder for every query instead of reusing the same object, and provide the root table name via the constructor. Example usage: new QueryBuilder("Customers").Select("id,name").Build().
• Sunny: Dependency injection is just for unit testing purpose.
• Sunny: @amon Will returning "this" cause any issues if create using "new QueryBuilder("Customers")"

Answer 1

Amon's comment is along the right track here, but this answer is built on your response to it:

"Dependency injection is just for unit testing purpose."

You've sort of inverted the problem here. The way in which you've chosen to inject it forces your hand to reuse the one injected instance of that query builder.

You can achieve both amon's suggestion (not reusing the query builder) and your goal (dependency injection) by injecting a factory instead of a single instance.

public interface IQueryBuilderFactory
{
 IQueryBuilder Get();
}
public class QueryBuilderFactory : IQueryBuilderFactory
{
 public IQueryBuilder Get() => new QueryBuilder();
}

You don't register the IQueryBuilder in your DI container, you instead register the IQueryBuilderFactory. This allows you to do something along the lines of:

public class CustomerService
{
 public CustomerService(IDBService dbService, IQueryBuilderFactory queryBuilderFactory) 
 {
 this.dbService = dbService;
 this.queryBuilderFactory = queryBuilderFactory;
 }
 public List<Customers> GetCustomers() 
 {
 .....
 await Task.WhenAll(customers.Select(async c => c.Verified = await IsVerified(c.customerId))); 
 ...
 }
 public async Task<bool> IsVerified(int customerId)
 {
 var queryBuilder = this.queryBuilderFactory.Get();
 var query = queryBuilder
 .SetRootTable("Customers")
 .Select("id,name")
 .Filter($"customerid eq {customerId}").Build();
 return Convert.ToInt32(await dbService.GetResponseAsync(query)) > 0;
 }
}

Notice how every call to IsVerified will request its very own instance of an IQueryBuilder, thereby sidestepping the issue of concurrent usage of the same instance.

---

The above is the direct answer to your question, and I think it's a good tool to have in your belt. However, I think there's a better fix here.

Currently, you are firing a DB request for each individual customer. It would be significantly better if you designed a query that would take in a collection of customer IDs and queried all of them at the same time.

Without doing so, you will get horrible performance when you need to verify a larger amount of customers. If you optimize this, the performance will be mostly the same regardless of how many customers need to be verified.

• Sunny: Thank you for introduction of Factory pattern. Based on Amon's comment, I tried to write new tests with "new QueryBuilder(query)" and I was able to test the IsVerified without the interface. For example, I am checking dbService.GetReseponseAsync is called with the composed query from "new QueryBuilder(query)". Do you think it will suffice without introducing interface.
• Flater: @Sunny: I generally advocate for interface-class separation as a matter of good practice. If at some point you wish to mock this factory, you'll want it to be able to return a mocked querybuilder (one that does what you need it to do for the test's purpose) instead of a real one. Without an interface, this is hard to accomplish. It's IMHO not worth contemplating as it is trivially simple in most IDEs to generate an interface based on an existing class, so the effort of introducing the interface is negligible when compared to the good practice benefits it gives you.