5
\$\begingroup\$

I've been researching ways to reduce response size on requests in my rest API without adding lots of calls to get back just the ID and single properties. What I've ended up attempting is creating the ability for the API consumer to specify the fields they want back from a default response model and have the response body send back only those fields out of the default model. This way, ideally, they get only exactly what they need.

An example request would look like:

api/characters/2?fields=id,displayname,mainImageUrl

Not specifying any fields would bring back the full default model as the response body.

This is a Web API 2 project with Entity Framework 6 using C# 6. I'm using AutoMapper to go from Entity object to Dto where necessary. What I have right now functions as I wanted for the most part, but I'm sure there are areas that need to be improved.

Specifically, I believe the use of joins in my Linq in general could be more generic as well as the amount of reflection in DtoBuilder.Assemble(), but am unsure of how to improve here. Any advice on these areas and others is much appreciated.

Gist link to IMetadataService

MetadataService.cs

This is the entry point from a Controller. It's injected into the controller's ctor via IoC (IMetadataService).

public interface IMetadataService
{ /*link above*/  }

public class MetadataService : BaseService, IMetadataService
{
    public MetadataService(IApplicationDbContext db)
        : base(db)
    { }

    public dynamic GetWithMovesOnEntity<TEntity, TDto>(int id, string fields = "")
        where TEntity : class, IMoveIdEntity
        where TDto : class
    {
        var dto = (from entity in Db.Set<TEntity>()
                   join joinEntity in Db.Moves
                       on entity.MoveId equals joinEntity.Id
                   where entity.Id == id
                   select entity).ProjectTo<TDto>()
                     .SingleOrDefault();

        return BuildContentResponse<TDto, TDto>(dto, fields);
    }

    public dynamic GetWithMoves<TEntity, TDto>(int id, string fields = "") 
        where TEntity : class, IMoveIdEntity 
        where TDto : class
    {
        var dto = (from entity in Db.Set<TEntity>()
                   join joinEntity in Db.Moves
                       on entity.MoveId equals joinEntity.Id
                   where entity.MoveId == id
                   select entity).ProjectTo<TDto>()
                     .SingleOrDefault();

        return BuildContentResponse<TDto, TDto>(dto, fields);
    }

    public dynamic GetFromEntity<TEntity, TDto>(TEntity entity, string fields = "") 
        where TEntity : class, IMoveIdEntity 
        where TDto : class
    {
        return BuildContentResponse<TEntity, TDto>(entity, fields);
    }

    public IEnumerable<dynamic> GetAllWithMoves<TEntity, TDto>(string fields = "") 
        where TEntity : class, IMoveIdEntity 
        where TDto : class
    {
        var dto = (from entity in Db.Set<TEntity>()
                   join joinEntity in Db.Moves
                       on entity.MoveId equals joinEntity.Id
                   select entity).ProjectTo<TDto>();

        return BuildContentResponseMultiple<TDto, TDto>(dto, fields);
    }

    public IEnumerable<dynamic> GetAllForOwnerId<TJoinEntity, TPrimaryEntity, TDto>(int id, string fields = "")
        where TJoinEntity : class, IMoveEntity 
        where TPrimaryEntity : class, IMoveIdEntity
        where TDto : class
    {
        var entities = (from entity in Db.Set<TPrimaryEntity>()
                        join ret in Db.Set<TJoinEntity>()
                            on entity.MoveId equals ret.Id
                        where ret.OwnerId == id
                        select entity).ProjectTo<TDto>();

        return BuildContentResponseMultiple<TDto, TDto>(entities, fields);
    }

    public dynamic Get<TEntity, TDto>(int id, string fields = "")
        where TEntity : class, IEntity
        where TDto : class
    {
        var entity = Db.Set<TEntity>().Find(id);
        return BuildContentResponse<TEntity, TDto>(entity, fields);
    }

    public dynamic Get<TEntity, TDto>(Expression<Func<TEntity, bool>> where, string fields = "")
        where TEntity : class, IEntity
        where TDto : class
    {
        var entity = Db.Set<TEntity>().First(where);
        return BuildContentResponse<TEntity, TDto>(entity, fields);
    }

    public IEnumerable<dynamic> GetAll<TEntity, TDto>(Expression<Func<TEntity, bool>> where, string fields = "")
        where TEntity : class, IEntity
        where TDto : class
    {
        var entities = Db.Set<TEntity>().Where(where);
        return BuildContentResponseMultiple<TEntity, TDto>(entities, fields);
    }

    /// <summary>
    /// Get all entity data of a specific type.
    /// </summary>
    /// <param name="fields"></param>
    /// <returns></returns>
    public virtual IEnumerable<dynamic> GetAll<TEntity, TDto>(string fields = "")
        where TEntity : class, IEntity
        where TDto : class
    {
        return BuildContentResponseMultiple<TEntity, TDto>(Db.Set<TEntity>(), fields);
    }

    /// <summary>
    /// Update an existing entity.
    /// </summary>
    /// <param name="id"></param>
    /// <param name="dto"></param>
    public void Update<TEntity, TDto>(int id, TDto dto)
        where TEntity : class, IEntity
        where TDto : class
    {
        Guard.VerifyObjectNotNull(dto, nameof(dto));
        UpdateEntity<TEntity, TDto>(id, dto);
    }

    /// <summary>
    /// Add an entity.
    /// </summary>
    /// <param name="dto"></param>
    /// <returns></returns>
    public TDto Add<TEntity, TDto>(TDto dto)
        where TEntity : class, IEntity
        where TDto : class
    {
        Guard.VerifyObjectNotNull(dto, nameof(dto));
        return AddEntity<TEntity, TDto>(dto);
    }

    /// <summary>
    /// Delete an entity.
    /// </summary>
    /// <param name="id"></param>
    public void Delete<T>(int id)
        where T : class, IEntity
    {
        DeleteEntity<T>(id);
    }
}

MetadataService calls into BaseService:

BaseService.cs

 public abstract class BaseService
    {
    protected readonly DtoBuilder DtoBuilder;
    protected readonly IApplicationDbContext Db;

    protected BaseService(IApplicationDbContext db)
    {
        Guard.VerifyObjectNotNull(db, nameof(db));
        Db = db;
        DtoBuilder = new DtoBuilder();
    }

    protected void UpdateEntity<TEntity, TDto>(int id, TDto dto)
        where TEntity : class, IEntity
        where TDto : class
    {
        var entity = Db.Set<TEntity>().Find(id);

        if (entity == null)
        { throw new NullReferenceException($"Unable to find entity of {typeof(TEntity).Name} with id = {id}"); }

        entity = Mapper.Map(dto, entity);
        entity.LastModified = DateTime.Now;
        Db.Entry(entity).State = EntityState.Modified;

        Db.SaveChanges();
    }

    protected TDto AddEntity<TEntity, TDto>(TDto dto)
        where TEntity : class, IEntity
        where TDto : class
    {
        var entity = Mapper.Map<TDto, TEntity>(dto);
        entity.LastModified = DateTime.Now;

        Db.Set<TEntity>().Add(entity);
        Db.SaveChanges();

        var newDto = Mapper.Map<TEntity, TDto>(entity);
        return newDto;
    }

    protected void DeleteEntity<T>(int id)
        where T : class, IEntity
    {
        var entity = Db.Set<T>().Find(id);

        if (entity == null)
        { throw new NullReferenceException($"Unable to find entity of {typeof(T).Name} with id = {id}"); }

        Db.Set<T>().Remove(entity);
        Db.SaveChanges();
    }

    protected bool EntityExists<T>(int id)
        where T : class, IEntity
    {
        return Db.Set<T>().Count(e => e.Id == id) > 0;
    }

    /// <summary>
    /// Returns a content based response that is either a custom <see cref="ExpandoObject"/> or an
    /// existing DTO depending on the passed in fields.
    /// </summary>
    /// <typeparam name="TEntity"></typeparam>
    /// <typeparam name="TDto"></typeparam>
    /// <param name="entity"></param>
    /// <param name="fields"></param>
    /// <returns></returns>
    protected dynamic BuildContentResponse<TEntity, TDto>(TEntity entity, string fields)
        where TEntity : class
    {
        if (entity == null)
        {
            return null;
        }

        return new DtoBuilder().Build<TEntity, TDto>(entity, fields);
    }

    protected IQueryable<dynamic> BuildContentResponseMultiple<TEntity, TDto>(IQueryable<TEntity> entities,
        string fields)
        where TEntity : class
        where TDto : class
    {
        if (entities == null)
        {
            return null;
        }

        var entitiesList = entities.ToList(); //note: this evaluates the result set fully!

        var builder = new DtoBuilder();

        var whereIterator = entitiesList.Select(entity => builder.Build<TEntity, TDto>(entity, fields));

        var retVal = whereIterator.AsQueryable();
        return retVal;
    }
}

Which then calls into the lower level DtoBuilder class:

DtoBuilder.cs

public class DtoBuilder
{
    private const BindingFlags Flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.IgnoreCase;

    /// <summary>
    /// Build up an <see cref="ExpandoObject"/> consisting of the specified field values
    /// as they exist on the passed in <typeparamref name="TEntity"/>.
    /// </summary>
    /// <typeparam name="TEntity"></typeparam>
    /// <typeparam name="TDto"></typeparam>
    /// <param name="entity"></param>
    /// <param name="fieldsRaw"></param>
    /// <returns></returns>
    public dynamic Build<TEntity, TDto>(TEntity entity, string fieldsRaw)
    {
        //Guard.VerifyStringIsNotNullOrEmpty(fieldsRaw, nameof(fieldsRaw));
        Guard.VerifyObjectNotNull(entity, nameof(entity));

        var splitValues = SplitValues(fieldsRaw);

        return Assemble<TEntity, TDto>(entity, splitValues);
    }

    private IEnumerable<string> SplitValues(string fieldsRaw)
    {
        return fieldsRaw.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
    }

    private dynamic Assemble<TEntity, TDto>(TEntity entity, IEnumerable<string> requestedFieldNames)
    {
        Guard.VerifyObjectNotNull(requestedFieldNames, nameof(requestedFieldNames));

        var fieldsNamesList = requestedFieldNames.ToList();

        //if no field names exist add all public instance ones for a 'default' dto object
        if (!fieldsNamesList.Any())
        {
            var props = typeof(TDto).GetProperties(Flags);
            fieldsNamesList.AddRange(props.Select(p => p.Name));
        }

        var customDto = new Dictionary<string, object>();

        foreach (var field in fieldsNamesList)
        {
            var propInfo = entity.GetType().GetProperty(field, Flags);

            if (propInfo != null)
            {
                //if null make empty so result is more web friendly
                var value = propInfo.GetValue(entity) ?? string.Empty;

                customDto.Add(propInfo.Name, value);
            }
        }

        dynamic resultObj = customDto.ToDynamicObject();
        return resultObj;
    }
}
\$\endgroup\$
3
\$\begingroup\$

Possible problem

Assume one would pass a request like

api/characters/2?fields=id,displayname,mainImageUrl,displayname

then the Assemble<TEntity, TDto>() method which is called by the DtoBuilder.Build() method would blow in your face because displayname would exists twice in the fieldsNamesList and therefor the call to customDto.Add(propInfo.Name, value); would throw an ArgumentException because the key already exists in the dictionary.

MetadataService

This class is well written and structured and easy to read. The only thing I don't like is the name where for the passed in Expression<Func<TEntity, bool>> in the overloaded Get<TEntity, TDto>() and GetAll<TEntity, TDto>() methods.

IMO where is a little bit abstract and hurts my eyes a little bit. Maybe whereCondition would be better ?


BaseService

Cleraly at the top of the class we see

protected readonly DtoBuilder DtoBuilder;
protected readonly IApplicationDbContext Db;

protected BaseService(IApplicationDbContext db)
{
    Guard.VerifyObjectNotNull(db, nameof(db));
    Db = db;
    DtoBuilder = new DtoBuilder();
}  

which is all good, you are using readonly , sure one could question the PascalCase casing of the protected fields, but otherwise this is ok.

But if we take a closer look at your class, we see that the DtoBuilder DtoBuilder isn't used anywhere in that class but only in the constructor. If you use a DtoBuilder object you are creating a new one each time you need it (BuildContentResponse and BuildContentResponseMultiple).

So if you would use the class level DtoBuilder you could limit the usage of reflection by caching the PropertyInfo's in a Dictionary<Type, Dictionary<string,PropertyInfo>> by first checking if the desired TDto is in the dictionary and if not you could add a new Dcitionary<string,PropertyInfo> containing all the PropertyInfo's of that type. Something along these lines

private readonly Dictionary<Type, Dictionary<string, PropertyInfo>> cachedPropertyInfos = new Dictionary<Type, Dictionary<string, PropertyInfo>>();
private Dictionary<string, PropertyInfo> FetchProperties<TDto>()
{
    var type = typeof(TDto);
    Dictionary<string, PropertyInfo> typeProperties = null;
    if (cachedPropertyInfos.TryGetValue(type, out typeProperties))
    {
        return typeProperties;
    }

    var properties = type.GetProperties(Flags);

    typeProperties = properties.ToDictionary(p => p.Name.ToLowerInvariant(), p => p);

    cachedPropertyInfos.Add(type, typeProperties);

    return typeProperties;
}  

and used in the Assemble<TEntity, TDto>() method like so

private dynamic Assemble<TEntity, TDto>(TEntity entity, IEnumerable<string> requestedFieldNames)
{
    Guard.VerifyObjectNotNull(requestedFieldNames, nameof(requestedFieldNames));

    var fieldsNamesList = requestedFieldNames.Select(f => f.ToLowerInvariant()).Distinct().ToList();

    Dictionary<string, PropertyInfo> typeProperties = FetchProperties<TDto>();

    //if no field names exist add all public instance ones for a 'default' dto object
    if (fieldsNamesList.Count == 0)
    {
        fieldsNamesList.AddRange(typeProperties.Keys);
    }

    var customDto = new Dictionary<string, object>();

    foreach (var field in fieldsNamesList)
    {
        PropertyInfo propInfo = null;

        if (typeProperties.TryGetValue(field, out propInfo))
        {
            //if null make empty so result is more web friendly
            var value = propInfo.GetValue(entity) ?? string.Empty;

            customDto.Add(propInfo.Name, value);
        }
    }

    return customDto.ToDynamicObject();
}

There are a few things I have changed in that method:

  • added a call to the new FetchProperties<TDto>() method
  • restricted the fieldsNamesList to not having any duplicate names in it
  • used fieldsNamesList.Count == 0 condition instead of !fieldsNamesList.Any() because it already is a List<string> so we can just evaluate the Count property which doesn't involve getting an Enumerator and a call to MoveNext() under the hood of the Any() method.
  • removed the variable dynamic resultObj
\$\endgroup\$
  • \$\begingroup\$ Thanks for pointing out the duplicate key bug. Wrote a test and fixed. I'm now calling Count on fieldNamesList instead of Any(). I'll have to check to see if I'm doing this in other places as well (and be more cognizant of it). Is there a benefit to calling .ToLowerInvariant() on items instead of passing in a StringComparer to the Dictionary ctor()? E.g new Dictionary<TKey, TValue>(StringComparer.InvariantCultureIgnoreCase). \$\endgroup\$ – Jordan Aug 3 '16 at 12:09
  • \$\begingroup\$ As the Dictionary will be created by Enumerable.ToDictionary() you would need to use the overloaded version which takes the IEqualityComparer to get rid of the ToLowerInvariant which might be the best idea. \$\endgroup\$ – Heslacher Aug 3 '16 at 12:14
  • \$\begingroup\$ Ah, I see. Thank you for your comments! This method of caching looks very promising. \$\endgroup\$ – Jordan Aug 3 '16 at 13:28
1
\$\begingroup\$

I'm not a fan of this:

protected readonly DtoBuilder DtoBuilder;
protected readonly IApplicationDbContext Db;

The reason is two-fold:

Quoting from that second link:

As soon as a member is more accessible than private, you are making guarantees to other classes about how that member will behave. Since a field is totally uncontrolled, putting it "out in the wild" opens your class and classes that inherit from or interact with your class to higher bug risk. There is no way to know when a field changes, no way to control who or what changes it.

IMHO once you use protected a field becomes a property, and a property should have a getter/setter.

\$\endgroup\$
  • \$\begingroup\$ Ok, that makes sense. I'll be changing those to be readonly properties and will look to see if I do that anywhere else. Thanks. \$\endgroup\$ – Jordan Aug 3 '16 at 11:36
1
\$\begingroup\$

This will be a very short answer because I will just suggest you an alternative method to filter the properties.

I guess you use json.net for serialization. If so, you could write your own contract resolver and actually super easy filter the properties.

Here's an example from the IContractResolver documentation where only properties that start with an A are serialized:

public class DynamicContractResolver : DefaultContractResolver
{
    private readonly char _startingWithChar;

    public DynamicContractResolver(char startingWithChar)
    {
        _startingWithChar = startingWithChar;
    }

    protected override IList<JsonProperty> CreateProperties(Type type, MemberSerialization memberSerialization)
    {
        IList<JsonProperty> properties = base.CreateProperties(type, memberSerialization);

        // only serializer properties that start with the specified character
        properties =
            properties.Where(p => p.PropertyName.StartsWith(_startingWithChar.ToString())).ToList();

        return properties;
    }
}

public class Book
{
    public string BookName { get; set; }
    public decimal BookPrice { get; set; }
    public string AuthorName { get; set; }
    public int AuthorAge { get; set; }
    public string AuthorCountry { get; set; }
}

Usage:

string startingWithA = JsonConvert.SerializeObject(book, Formatting.Indented,
    new JsonSerializerSettings { ContractResolver = new DynamicContractResolver('A') });
\$\endgroup\$
  • \$\begingroup\$ Hmm, this is very tempting. My only potential issue is that it locks me to json at this lower level while replacing the DtoBuilder type. \$\endgroup\$ – Jordan Aug 3 '16 at 11:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.