C#•3y ago

Database - Relationship returns empty set even though records are present [Answered]

I think I'm probably missing something stupid obvious... In my batch model, I have

private readonly List<InContactDataStatus> _queueItems;

public IReadOnlyList<IncontactDataStatus> DownloadQueueItems => _queueItems.AsReadOnly();

private readonly List<InContactDataStatus> _queueItems;

public IReadOnlyList<IncontactDataStatus> DownloadQueueItems => _queueItems.AsReadOnly();

I had set up a relationship as below, which did set up a foreign key relationship to the batch as expected.

// entity config
builder.HasMany(it => it.DownloadQueueItems).WithOne(it => it.Batch);

// migration
table.ForeignKey(
    name: "FK_IC_DataStatus_IC_Batch_BatchId",
    column: x => x.BatchId,
    principalTable: "IC_Batch",
    principalColumn: "Id",
    onDelete: ReferentialAction.Cascade);

// entity config
builder.HasMany(it => it.DownloadQueueItems).WithOne(it => it.Batch);

// migration
table.ForeignKey(
    name: "FK_IC_DataStatus_IC_Batch_BatchId",
    column: x => x.BatchId,
    principalTable: "IC_Batch",
    principalColumn: "Id",
    onDelete: ReferentialAction.Cascade);

When processing a batch, I get the collection of IncontactDataStatuses to manipulate them, but the call to batch.DownloadQueueItems returns an empty list. I did see something about setting up the access mode, but apparently using the field and ignoring the property is default.

35 Replies

jcotton42•3y ago

EF doesn't populate fields I don't think And even if it did, you need to make sure you're using Include in your queries for related data

reacher•3y ago

How would EF know that a read-only property named DownloadQueueItems is connected to a private field named _queueItems, I'm surprised EF isn't throwing an exception at runtime with this configuration And yes an explicit Include is also needed in the query (it would have been nice to see the code for the query itself)

Foxtrek_64OP•3y ago

Here is the current query

var failedQueueItems = batch.DownloadQueueItems.Where
    (
        it => it.Status is QueueItemState.Failed or QueueItemState.Abandoned
    );

var failedQueueItems = batch.DownloadQueueItems.Where
    (
        it => it.Status is QueueItemState.Failed or QueueItemState.Abandoned
    );

My understanding is that EF Core by default will use field access mode for all properties. At least, that was what I gathered when I read the docs on backing fields https://docs.microsoft.com/en-us/ef/core/modeling/backing-field?tabs=data-annotations

Backing Fields - EF Core

Configuring backing fields for properties in an Entity Framework Core model

reacher•3y ago

That's not an EF query? That's just LINQ

Foxtrek_64OP•3y ago

Ah, perhaps I misread though. It looked like it was providing the sort of default, built-in code, but I apparently do need to configure an explicit field binding

reacher•3y ago

You can also just use IReadOnlyList directly, EF Core supports that Instead of having a backing field

Foxtrek_64OP•3y ago

That would simplify things a lot actually

reacher•3y ago

I also don't know if you can use Include on a getter method when EF Core is configured to just use a private backing field I've never tried that but it seems a bit messy

Foxtrek_64OP•3y ago

I'll try it like that with the model modified just to have a IReadOnlyList property with no backing field. If it doesn't work I'll change to a proper query with include

reacher•3y ago

You still need the include You always need Include Unless you're doing a projection

Foxtrek_64OP•3y ago

var batch = _pluginContext.Batches.Include(it => it.DownloadQueueItems).FirstOrDefault
(
    x =>
    x.StartDate == startDate &&
    x.EndDate == endDate
);

var batch = _pluginContext.Batches.Include(it => it.DownloadQueueItems).FirstOrDefault
(
    x =>
    x.StartDate == startDate &&
    x.EndDate == endDate
);

I think that should probably do it there That batch entity gets passed around from that point onwards

reacher•3y ago

Why not async?

Foxtrek_64OP•3y ago

(If it returns null the next step is creating a new one, which has its download queue items generated and inserted via the constructor) Probably just not seeing it could be async at the time. I've updated it to be so Now to find out if inserting new records into the table that backs download queue items will update the readonly collection

foreach (var status in failedQueueItems)
{
    var newStatus = new InContactDataStatus(status.DownloadType)
    {
        Batch = status.Batch
    };
    status.RetryItem(newStatus);
    changed = true;
}

foreach (var status in failedQueueItems)
{
    var newStatus = new InContactDataStatus(status.DownloadType)
    {
        Batch = status.Batch
    };
    status.RetryItem(newStatus);
    changed = true;
}

status.RetryItem() sets the finished time for the current item if it's not set, sets the status to Retry, and assigns the provided newStatus to a property of type InContactDataStatus configured with a HasOne relationship

public void RetryItem(TSuperseding supersedingItem)
{
    if (Status != QueueItemState.Failed)
    {
        throw new InvalidOperationException("Cannot retry an item which has not failed.");
    }

    ProcessingEnd ??= DateTimeOffset.UtcNow;
    Status = QueueItemState.Retried;
    Retry = supersedingItem;
}

public void RetryItem(TSuperseding supersedingItem)
{
    if (Status != QueueItemState.Failed)
    {
        throw new InvalidOperationException("Cannot retry an item which has not failed.");
    }

    ProcessingEnd ??= DateTimeOffset.UtcNow;
    Status = QueueItemState.Retried;
    Retry = supersedingItem;
}

This should result in the newStatus item being inserted into the database (code to actually save omitted), but I don't know if that'll update the readonly list in the batch model.

reacher•3y ago

I think you have to reload it At least I hope so

Foxtrek_64OP•3y ago

Is there a specific method to reload a specific item or do I just perform a new query and search for the updated batch by id?

reacher•3y ago

You can explicitly load a navigation property but I would recommend just reloading it the same way I don't recommend getting too fancy with EF, just keep it simple and it's easier to not get into weird situations

Foxtrek_64OP•3y ago

I can just pull a new instance of the batch by id. That seems to be simple enough.

reacher•3y ago

Yeah that's what I'd recommend

D.Mentia•3y ago

Why is it a readonly collection to begin with? The usual strategy is to start with a tracked parent entity (the batch), then just add the new Status to the batch's collection, and save. The batch you have remains up to date and can be iterated, because it's what updated the db

reacher•3y ago

Good point. I like immutability, but it doesn't work great with EF entities. And I also generally don't send my EF entities loose out into the rest of my system because of that mutability. Even untracked entities. I usually just return a read-only interface of it, or map to another type The read-only interface is a trick I've used quite a lot, and it's been great

D.Mentia•3y ago

Read-only interface is a good idea. I generally end up with the EF model being just, a blank class extending a base model... which I also sometimes use as a generic DTO, as much as I know that's a bad idea (I just hate spamming specific models). I, too, have had nightmares dealing with EF entities being sent everywhere. My biggest issue with it is not knowing if the entity you're working on in any given method is already attached/tracked or not.
I like the idea of lots of small, non-saving endpoints that take entities, so you can chain operations and Save at the end in one transaction instead of multiple, and everything else takes the base class. And anything that receives an entity as a param can safely assume the entity is already tracked

jcotton42•3y ago

... generic DTO? What? How does that even work?

D.Mentia•3y ago

like, being lazy and providing some UpdateTrackingNumber endpoint, which takes in a full non-EF BaseEntity. Leaving it up to the caller to know that I'm going to ignore all the values except ID and TrackingNumber But if they're calling UpdateTrackingNumber and expecting IsAdmin=true to work, that's on them 😛

jcotton42•3y ago

🤢

D.Mentia•3y ago

Yeahhh.... I know.... but honestly, it saves so much clutter from random models everywhere

jcotton42•3y ago

Records are your friend Declare them next to the endpoint using them

D.Mentia•3y ago

it's not just the model declaration, it's newing them up everytime you make a function call. If you have some BaseEntity and want to do 3 things with it, so you new up 3 separate DTOs and copy the thing you need from the entity over to each one, that's awkward. I like just setting the values I need and making 3 calls with the entity... or rather, the base class that the EF Entity extends, because I don't like EF entities being passed around if they aren't being tracked

reacher•3y ago

I have tracking disabled by default, and only enable tracking within one method where I perform an update, and I never return tracked entities

D.Mentia•3y ago

To each their own. I don't like that idea because it means you are always competing performance with modularity. If you want to do 3 things to an entity, you can either call 3 endpoints, and do 3 db operations, or write a whole new endpoint that duplicates the logic of the other 3 in one operation

reacher•3y ago

I mean entities aren't tracked across endpoint calls So I'm not sure what you mean there

D.Mentia•3y ago

By passing around tracked entities (in the case of these very low level modules, not sending them anywhere else), you can call 3 functions and then save to the database, one transaction They are as long as the context is the same That's actually half of my nightmare about passing around Entities 😛 lots of unexpected issues when you try to track something that's already tracked and never got disposed because it just got passed around everywhere And it's rare that every function in a service makes a new db context; usually the service has one, and the service is expected to be created fresh each time you make a call. If you happen to get an instance of that service, and call more than one method in a row, you have potential problems

reacher•3y ago

Sure, you can pass entities to helper methods that work on them, but yeah you need to be careful with that and keep the scope narrow And I'm never releasing tracked entities out into my system except for passing to some helper methods Specifically designed for DB updates

D.Mentia•3y ago

Yes. I wish that was the case in my work repo 😛 Apparently they explicitly chose to avoid DTOs and extra models, removing them years ago to just use the entity everywhere. They got angry when I suggested we not do that But anyway, I will stop hijacking a thread to rant about work now

Foxtrek_64OP•3y ago

I can change it to just be a list instead of a readonly list. I suppose it would simplify it Especially since when I last touched it this morning I was getting some errors about it being unable to iterate a readonly collection (which made no sense to me)

Accord•3y ago

✅ This post has been marked as answered!

Gaming

Programming

Database - Relationship returns empty set even though records are present [Answered]

Did you find this page helpful?