anavarro9731 / datastore Goto Github PK

This may also be true on Updates and Deletes.
You need to make sure that once you queue the command, changes to the original object are not considered.

When new Aggregates are created using DataStoreCreateCapabilities.Create<T>(), the Created, CreatedAsMillisecondsEpochTime, Modified and ModifiedAsMillisecondsEpochTime properties are set to DateTime.MinValue.

The issue here appears within WalkGraphAndUpdateEntityMeta(), it's checking to see if the properties are null & setting them to DateTime.UtcNow if they are null but since these properties are now non-nullable, they are being left at default (DateTime.MinValue):

else if (p.Name == nameof(IEntity.Created))
{
    //set created datetime if this is null
    if ((DateTime?)p.GetValue(current, null) == null)
    {
        p.SetValue(current, DateTime.UtcNow, null);
    }
}
else if (p.Name == nameof(IEntity.CreatedAsMillisecondsEpochTime))
{
    //set created datetime if this is null
    if (p.GetValue(current, null) == null)
    {
        p.SetValue(current, DateTime.UtcNow.ConvertToSecondsEpochTime(), null);
    }
}

The error message thrown for when restricted properties are modified does not include the newly restricted properties "Modified" & "ModifiedAsMillisecondsEpochTime". It currently reads:

"Cannot change restricted properties [id, schema, Created, CreatedAsMillisecondsEpochTime on entity "

But should be updated to be:

"Cannot change restricted properties [id, schema, Created, CreatedAsMillisecondsEpochTime, Modified, ModifiedAsMillisecondsEpochTime] on entity "

DataStoreQueryCapabilities.Read and DataStoreQueryCapabilities.ReadActive do not reapply the predicate to the result set after EventReplay has modified the result set by calling ApplyAggregateEvents()

This can be replicated by adding the following line into the WhenCallingCreateWithoutCommitting.ItShouldReflectTheChangeInFutureQueriesFromTheSameSession() unit test:

Assert.Empty(this.testHarness.DataStore.ReadActive<Car>(car => car.Make == "Ford").Result);

If the amount specified by Take is greater than a single maxtakesize, then extra results are returned. To reproduce in WhenCallingReadWithBigSkipAndTake:

this.carsFromDatabaseWithFilter4 = this.testHarness.DataStore.WithoutEventReplay.Read<Car, WithoutReplayOptions<Car>>(car => car.Make == "Volvo", o => o.Skip(100).Take(1500)).Result;

[Fact]
        public void ItShouldReturnTheCorrectAmountOfVolvosWhenTakeIsLessThanAmountOfVolvosAvailableAfterSKip()
        {
            Assert.True(this.testHarness.DataStore.ExecutedOperations.All(e => e is AggregatesQueriedOperation<Car>));
            Assert.Equal(2202, this.carsInDatabase.Count());
            Assert.Equal(1500, this.carsFromDatabaseWithFilter4.Count()); //this fails and due to 2000 items being returned
        }

When DataStoreQueryCapabilities.Read and DataStoreQueryCapabilities.ReadActive query for results they first retrieve results from the DbConnection that match the provided predicate.

If an QueuedUpdateOperation that updates an Aggregate so that it now matches the predicate has not been committed yet, then this Aggregate will not be included in the result set as part of EventReplay.ApplyAggregateEvents.

This can be replicated by adding the following line to WhenCallingUpdateByIdWithoutCommitting.ItShouldOnlyMakeTheChangesInSession:

Assert.Single(this.testHarness.DataStore.ReadActive<Car>(car => car.Make == "Ford").Result);

SUGGESTION: bool out on AddSkipAndTake(), AddOrderBy() to indicate if Skip, Take or OrderBy has actually been applied so the provider can take any appropriate action

Currently when UseVersionHistory is set to true in DataStoreOptions it will create aggregate history items for all aggregates using DataStore when it calls IncrementAggregateHistoryIfEnabled<T>.

If DataStoreOptions was extended to allow for either a white list or a blacklist of aggregate types for version history then it would prevent un-needed read and write for aggregates where the version history will never be used (e.g. in log items).

DataStoreCreateCapabilities.Create has a guard against items that are already preexisting with the same id:

bool existsAlready = (await this.DsConnection.Exists(new AggregateQueriedByIdOperation(methodName, model.id)));
            Guard.Against(existsAlready, "An item with the same ID already exists", Guid.Parse("cfe3ebc2-4677-432b-9ded-0ef498b9f59d"));

However it's checking for model.id instead of newObject.id, this can cause issues if model.id is not yet set and the created aggregate id is expected to be set by the call to ForceProperties() against newObject.

HardDelete throws an error when using Version History. HardDelete is failing if the aggregate does not exist and the Version History is activated.

Observation: TestDataCreator is deleting and recreating the aggregate with the same ID (Guid), this might create an issue if Version History already exists for that ID value.

DataStore.ReadActiveById<T>() should not throw InvalidOperationException (Sequence contains no elements) as it should be using SingleOrDefault()

When creating an aggregate with a specific id, and then calling Exists() prior to CommitChanges() will result in false being returned rather than true. This appears to be due to the previously executed events are not replayed.

Example:

testHarness.DataStore.Create(newCar).Wait();

\\ result should be true but comes across as false
var result = testHarness.DataStore.Exists(newCar.id).Result;