search

ardalis / Specification

Base class with tests for adding specifications to a DDD model
MIT License
1.83k stars 238 forks source link

Including nullable/optional navigation property causes only non-nullable connected objects to be returned #356

Closed jeffreymonroe closed 10 months ago

jeffreymonroe commented 10 months ago

This is a bit of a head scratcher. Using the following specification with includes: 

    // Define the return
    ISpecificationBuilder<Entities.AuditTrail, ListSievedModel> 
      select = Query.Select(
      x => new ListSievedModel
      {
          Id = x.Id,
          Created = x.Created,
          Modified = x.Modified,
          State = x.State,
          OperationType = x.OperationType,
          ForEntity = x.ForEntity,
          Username = x.Username,
          TrackingKey = x.TrackingKey,
          RelStateName = x.RelState.ShortName,
          RelOperationTypeName = x.RelOperationType.ShortName,
          RelForEntity = x.RelForEntity,
          RelForEntityTitle = (x.RelForEntity == null) ? string.Empty : x.RelForEntity.ToString(),
          RelForEntityType = (x.RelForEntity != null) ? x.RelForEntity.EntityType : string.Empty
      }); 

    // Add Includes
    Query.IgnoreAutoIncludes();
    Query.Include(s => s.RelState);
    Query.Include(s => s.RelOperationType);
    Query.Include(s => s.RelForEntity).AsSplitQuery();

Creates two different data sets when repository.CountAsync and repository.ListAsync are run. CountAsync returns all projected objects regardless of whether s.RelForEntity is null or not null. ListAsync only returns the not-null s.RelForEntity projections.

The result is as follows from a web api:

{ "pagedInfo": { "pageNumber": 1, "pageSize": 20, "totalPages": 2, "totalRecords": 24 }, "value": [ { "id": "3739c90a-47b2-4836-808f-65a57e18db20", "created": "0001-01-01T08:00:00Z", "modified": "0001-01-01T08:00:00Z", "state": 5, "operationType": 2, "forEntity": "014b8736-1749-49f7-8e78-cd8b42a8c3e9", "username": "", "trackingKey": "014b8736-1749-49f7-8e78-cd8b42a8c3e9", "relStateName": "Enabled", "relOperationTypeName": "Insert", "relForEntityTitle": "Four score and 7 hyars on 7/26/2023 8:16:55 PM ", "relForEntityType": "Vantedge.Portfolio.Domain.Entities.Comment" }, { "id": "9ebc8c10-67f3-4ffe-a08f-aa7a512ce35c", "created": "0001-01-01T08:00:00Z", "modified": "0001-01-01T08:00:00Z", "state": 5, "operationType": 2, "forEntity": "1e921c37-127b-43d3-ae59-a60a2fafa4ff", "username": "", "trackingKey": "1e921c37-127b-43d3-ae59-a60a2fafa4ff", "relStateName": "Enabled", "relOperationTypeName": "Insert", "relForEntityTitle": "This is a test on 7/26/2023 8:10:46 PM ", "relForEntityType": "Vantedge.Portfolio.Domain.Entities.Comment" }, { "id": "e39ff253-4c45-4488-b831-ad45d54a79a4", "created": "0001-01-01T08:00:00Z", "modified": "0001-01-01T08:00:00Z", "state": 5, "operationType": 3, "forEntity": "f8a41f37-f4ff-4404-ad31-15105a349ac6", "username": "", "trackingKey": "f8a41f37-f4ff-4404-ad31-15105a349ac6", "relStateName": "Enabled", "relOperationTypeName": "Update", "relForEntityTitle": "TTTTTEEEEESSSSSST", "relForEntityType": "Vantedge.Portfolio.Domain.Entities.Market" }, { "id": "f0bc4bee-2ea5-42c4-9d81-e95d6b5bab23", "created": "0001-01-01T08:00:00Z", "modified": "0001-01-01T08:00:00Z", "state": 5, "operationType": 2, "forEntity": "f8a41f37-f4ff-4404-ad31-15105a349ac6", "username": "", "trackingKey": "f8a41f37-f4ff-4404-ad31-15105a349ac6", "relStateName": "Enabled", "relOperationTypeName": "Insert", "relForEntityTitle": "TTTTTEEEEESSSSSST", "relForEntityType": "Vantedge.Portfolio.Domain.Entities.Market" }, { "id": "99047ee3-6a11-4b8c-9502-fea8bb8fffa6", "created": "0001-01-01T08:00:00Z", "modified": "0001-01-01T08:00:00Z", "state": 5, "operationType": 2, "forEntity": "dd02898a-72c6-456c-a972-5db9a327bcf5", "username": "", "trackingKey": "dd02898a-72c6-456c-a972-5db9a327bcf5", "relStateName": "Enabled", "relOperationTypeName": "Insert", "relForEntityTitle": "Keep this on 7/26/2023 8:17:24 PM ", "relForEntityType": "Vantedge.Portfolio.Domain.Entities.Comment" } ], "valueType": "System.Collections.Generic.List`1[[Vantedge.Portfolio.Api.Endpoints.AuditTrails.CommandsAndResults+PagedResult, Vantedge.Portfolio.Api, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], System.Private.CoreLib, Version=7.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e", "status": 0, "isSuccess": true, "successMessage": "", "correlationId": "", "errors": [], "validationErrors": [] }

Notice "totalRecords": 24 which is resolved from CountAsync . The actual data is from ListAsync which uses the exact same specification. The missing results are the null s.RelForEntity projections.

Any help would be much appreciated.

Cheers.

fiseni commented 10 months ago

Hey @jeffreymonroe

I don't see it in your example, but you have Take/Skip somewhere right? The CountAsync method in the built-in repository is a bit special, it doesn't include all evaluators. For instance, Take and Skip are omitted. Actually, all evaluators with IsCriteriaEvaluator false are omitted. This was a community request and it makes the pagination code simpler. You don't have to create two specs, one without pagination, and another one with pagination. Instead, you can use the same specification for counting all records, and also getting the list of records per page. Let's assume this specification

public class CustomerSpec : Specification<Customer>
{
    public CustomerSpec()
    {
        Query.Where(x => x.Age > 20)
            .Skip(10)
            .Take(10);
    }
}

If you call CountAsync using this spec, you'll get all customers satisfying the criteria. And if you call ListAsync with the same spec, will return only the second 10 records.

Omitted evaluators:

Note: Not particularly related to this issue, but I want to mention the following. Whenever you use projection (the Select statement), EF ignores all your Include statements. The logic behind it is that you anyway not returning entities back, but some projections, so all explicit Includes are ignored. If there is navigation usage inside your Select statement, EF automatically will add the join statements in the generated query.

jeffreymonroe commented 10 months ago

Hi @fiseni

Thanks for the thorough explanation. I thought this was the case and I do agree with using the same specification for CountAsyncand ListAsync. I am using a modified version of Sieve (https://github.com/Biarity/Sieve) which can be utilized with Specification. The Skip/Take are handled in Sieve.

In regards to optional navigation properties, I continue to have issues when including an optional navigation property inside a projection. See below:

      // Projection
       ISpecificationBuilder<Entities.AuditTrail, ListSievedModel> select = Query.Select(
          x => new ListSievedModel
          {
              Id = x.Id,
              Created = x.Created,
              Modified = x.Modified,
              State = x.State,
              OperationType = x.OperationType,
              ForEntity = x.ForEntity,
              Username = x.Username,
              TrackingKey = x.TrackingKey,
              RelStateName = x.RelState.ShortName,
              RelOperationTypeName = x.RelOperationType.ShortName,
              // RelForEntity = (x.RelForEntity == null) ? null : x.RelForEntity
              //RelForEntityTitle = (x.RelForEntity == null) ? string.Empty : x.RelForEntity.ToString(),
              //RelForEntityType = (x.RelForEntity != null) ? x.RelForEntity.EntityType : string.Empty
          }); ;

        // Add Includes
        Query.IgnoreAutoIncludes();
        //Query.Include(s => s.RelState);
        //Query.Include(s => s.RelOperationType);
        //Query.Include(s => s.RelForEntity).AsSplitQuery();

        Query.AsNoTracking().ApplySieve(sieveProcessor, sieveModel);

This code will return all projections where s.RelForEntity is null or not null. When I uncomment RelForEntity = (x.RelForEntity == null) ? null : x.RelForEntity I only get projections where s.RelForEntity is not null.

I suspect this is not a Specification issue, but something more general in EF. I was hoping you would have some insight.

Cheers and thanks again.

jeffreymonroe commented 10 months ago

After setting this aside and then reviewing, I was able to fix this issue. Indeed it had nothing to do with Specification and everything to do with the EF configuration for the two related objects. I had to set the navigational property as nullable then define the relationship as IsRequired(false).

Cheers and I will mark this as closed.