Home > Software design >  my Get method return some fields as null while the GetById method returns everything
my Get method return some fields as null while the GetById method returns everything

Time:10-05

Currently working on my first web api and my first c# project, I've been stuck trying to implement the Get method.

In my project, an user can have a profession (Journalist, Developer, ...) and a profession field (Sports, Business, Website development, ...).

When I'm trying to get all the users, the profession (UserProfession) and profession field (UserProfessionField) attributes are returned as null. When I'm trying to return one user only by its id, those same attributes are not null anymore.

I have the following model for User:

namespace Sims.Models
{
    public partial class User
    {
        public User()
        {
            DataUsages = new HashSet<DataUsage>();
        }

        public long IdUser { get; set; }
        public int UserProfessionId { get; set; }
        public int UserProfessionFieldId { get; set; }
        public string? UserName { get; set; }
        public string? UserMail { get; set; }
        public string? UserCompany { get; set; }
        public byte[]? UserPicture { get; set; }

        public virtual Profession UserProfession { get; set; } = null!;
        public virtual ProfessionField UserProfessionField { get; set; } = null!;
        public virtual ICollection<DataUsage> DataUsages { get; set; }
    }
}

Here's the DTO I'm using for this model:

namespace sims.DTO
{
    public partial class UserDTO
    {
        public long IdUser { get; set; }
        public string? UserName { get; set; }
        public string? UserMail { get; set; }
        public string? UserCompany { get; set; }
        public virtual ProfessionDTO UserProfession { get; set; } = null!;
        public virtual ProfessionFieldDTO UserProfessionField { get; set; } = null!;
    }
}

Then there is my Get method and its result:

[HttpGet]
public async Task<ActionResult<IEnumerable<UserDTO>>> GetUsers()
{
    var users = _context.Users.ToList();
    var userDtos = new List<UserDTO>();
    foreach (var user in users)
    {
        userDtos.Add(new UserDTO 
        { 
            IdUser = user.UserProfessionId, 
            UserName = user.UserName, 
            UserCompany = user.UserCompany, 
            UserMail = user.UserMail, 
            UserProfession =  user.UserProfession == null ?
            null as ProfessionDTO : new ProfessionDTO
            {
                IdProfession = user.UserProfession.IdProfession,
                ProfessionName = user.UserProfession.ProfessionName
            },
            UserProfessionField = user.UserProfessionField == null ?
            null as ProfessionFieldDTO : new ProfessionFieldDTO
            {
                IdProfessionField = user.UserProfessionField.IdProfessionField,
                ProfessionFieldName = user.UserProfessionField.ProfessionFieldName
            }
        });
    }
    return userDtos;
}
[
  {
    "idUser": 2,
    "userName": "user_test",
    "userMail": "[email protected]",
    "userCompany": "TestCompany",
    "userProfession": null,
    "userProfessionField": null
  },
  {
    "idUser": 1,
    "userName": "roger_federer",
    "userMail": "[email protected]",
    "userCompany": null,
    "userProfession": null,
    "userProfessionField": null
  },
  {
    "idUser": 3,
    "userName": "rafa_nadal",
    "userMail": null,
    "userCompany": null,
    "userProfession": null,
    "userProfessionField": null
  }
]

And the GetById method and its result:

        [HttpGet("{id}")]
        public async Task<ActionResult<UserDTO>> GetUserById(long id)
        {
            UserDTO User = await _context.Users.Select(user => new UserDTO
            {
                IdUser = user.UserProfessionId,
                UserName = user.UserName,
                UserCompany = user.UserCompany,
                UserMail = user.UserMail,
                UserProfession = user.UserProfession == null ?
                    null as ProfessionDTO : new ProfessionDTO
                    {
                        IdProfession = user.UserProfession.IdProfession,
                        ProfessionName = user.UserProfession.ProfessionName
                    },
                UserProfessionField = user.UserProfessionField == null ?
                    null as ProfessionFieldDTO : new ProfessionFieldDTO
                    {
                        IdProfessionField = user.UserProfessionField.IdProfessionField,
                        ProfessionFieldName = user.UserProfessionField.ProfessionFieldName
                    }
            }).FirstOrDefaultAsync(user => user.IdUser == id);
            if(User == null)
            {
                return NotFound();
            }
            else
            {
                return User;
            }
        }
{
  "idUser": 1,
  "userName": "roger_federer",
  "userMail": "[email protected]",
  "userCompany": null,
  "userProfession": {
    "idProfession": 1,
    "professionName": "Journalist"
  },
  "userProfessionField": {
    "idProfessionField": 1,
    "professionFieldName": "Sports"
  }
}

Do you have any idea what could be the origin of this issue ?

CodePudding user response:

In your data model, UserProfession and UserProfessionField are stored in separate tables; for both there are so-called Navigation properties on your User class. When using Entity Framework, these Navigation properties can be eager and lazy loaded. The earlier fetches the values directly with the parent object, the latter fetches them on demand.

In your GetUsers method, the first statement loads all the data from the database using a ToList - this breaks the ability to lazy load. Only after this, the projection using the Select takes place, but at this time, the data cannot be lazy loaded anymore.

In contrast in GetUserById, the call to FirstOrDefaultAsync follows the projection with the Select. During the Select, by accessing the UserProfession(Field) property, you lazy load the corresponding data from the other tables.

This explains why the behavior is happening. In order to fix this, you should change from lazy loading to eager loading for performance reasons (if you load 20 users and load the dependent data for each of them, you have 41 requests whereas with eager loading, you only need one). You can use the Include method for this:

[HttpGet]
public async Task<ActionResult<IEnumerable<UserDTO>>> GetUsers()
{
  var users = _context.Users
    .Include(x => x.UserProfession)
    .Include(x => x.UserProfessionField);
  // ...

Above code includes the information in the UserProfession and UserProfessionField tables directly when loading the Users.

In addition, you can also change the GetUserById to also include the properties - though the performance problem is not as dramatic as with loading the list.

CodePudding user response:

I would suggest

  • You stop copy/pasting code around. Write a single function to transform a User database object into a User data transfer object and call it in both places.

  • Start listening to your compiler warnings. You are comparing things to null that acording to your database can never be null. You are assigning null to things that cannot be null by your definition. Why isn't your compiler throwing errors like crazy? Did you turn that off? Why? Listen to your compiler.

  • Test your code in isolation. Maybe with unit tests. If you cannot do that, at least ensure that you restart your whole app and your call under test is the first call made.

So when you have a single place where you transform a user object, with probably less code, then you can start looking for mistakes. Taking away the option of anything being cached, you should be able to find your mistake soon.

I don't know how your context is configured, but with the default configuration, you seem to be missing the .Includes for your navigation properties. They might be filled accidentially, because you called other code on the same context before. But even if that is the case, please take the three points above and fix them. This is no way to program.

  • Related