Skip to content

Commit

Permalink
Remove 'Advanced' property from AsyncTenantedDocumentSession. It ca…
Browse files Browse the repository at this point in the history
…used adding additional logic:

 - ExistsAsync()
 - SaveChangesAsync(boolean)
 - Patch(...) & PatchWithoutValidation(...)
  • Loading branch information
AKlaus committed Aug 19, 2021
1 parent c0466c5 commit b0d6b56
Show file tree
Hide file tree
Showing 14 changed files with 218 additions and 100 deletions.
97 changes: 79 additions & 18 deletions back-end/Database/Infrastructure/AsyncTenantedDocumentSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -22,9 +23,6 @@ public class AsyncTenantedDocumentSession : IAsyncTenantedDocumentSession
/// <inheritdoc />
public bool ThrowExceptionOnWrongTenant { get; }

/// <inheritdoc />
public IAsyncAdvancedSessionOperations Advanced => DbSession.Advanced;

#region Fields [PRIVATE] ----------------------------------------------

private readonly IDocumentStore _documentStore;
Expand All @@ -43,7 +41,7 @@ public class AsyncTenantedDocumentSession : IAsyncTenantedDocumentSession
/// </summary>
/// <remarks>
/// Use it in testing to avoid running on stale indexes.
/// Note that when deferred patches need to be applied then <see cref="SaveChangesAsync"/> will wait for indexes anyway
/// Note that when deferred patches need to be applied then <see cref="IAsyncDocumentSession.SaveChangesAsync"/> will wait for indexes anyway
/// </remarks>
private readonly TimeSpan? _waitingForIndexesAfterSave;

Expand Down Expand Up @@ -94,13 +92,25 @@ public void Delete<T>(T entity) where T: notnull
}

/// <inheritdoc />
public async Task SaveChangesAsync(CancellationToken token = default)
public async Task<bool> SaveChangesAsync(CancellationToken token = default)
{
if (HasChanges())
{
await DbSession.SaveChangesAsync(token);
await SendDeferredPatchByQueryOperationsAsync(token);
}
if (!HasChanges())

This comment has been minimized.

Copy link
@ayende

ayende Aug 19, 2021

Member

FWIW, you are forcing us to run changes detection twice here. Do you really need the bool result?
Otherwise, just call it always.

This comment has been minimized.

Copy link
@AKlaus

AKlaus Aug 20, 2021

Author Collaborator

Is it an expensive check?
Returning a bool here is more "nice to have" than required. Can be removed if it hurts performance.

This comment has been minimized.

Copy link
@ayende

ayende Aug 20, 2021

Member

It is O(N) to the number of docs in the session. Not usually expensive, no.

return false;

await DbSession.SaveChangesAsync(token);
await SendDeferredPatchByQueryOperationsAsync(token);
return true;
}

/// <inheritdoc />
public async Task<bool> SaveChangesAsync(bool clearCache, CancellationToken token = default)
{
var saved = await SaveChangesAsync(token);
if (clearCache)
// Clear all cached entities

This comment has been minimized.

Copy link
@ayende

ayende Aug 19, 2021

Member

what is the scenario for this?

This comment has been minimized.

Copy link
@AKlaus

AKlaus Aug 20, 2021

Author Collaborator

It's used only in tests

DbSession.Advanced.Clear();

return saved;
}

#region StoreAsync [PUBLIC] -------------------------------------------
Expand Down Expand Up @@ -199,16 +209,67 @@ public IRavenQueryable<T> Query<T>(string? indexName = null, string? collectionN
#endregion Query [PUBLIC] ---------------------------------------------

/// <inheritdoc />
public bool HasChanges()
{
// Use direct property, so we don't open a new session too early
return _dbSession?.Advanced.HasChanges == true
// if a deferred patch query exist
public bool HasChanges() =>
// Usages of the direct property avoids opening a new session too early
_dbSession?.Advanced.HasChanges == true
// a deferred patch query might exist
|| _deferredPatchQueries.Any()
// if there is a PATCH request in the underlining session
// Note, the below has been added to 'Advanced.HasChanges()' in v5.2
|| _dbSession?.Advanced is InMemoryDocumentSessionOperations { DeferredCommandsCount: > 0 };

/// <inheritdoc />
public string GetFullId<T>(string shortId) where T : IEntity
=> DbSession.Advanced.GetFullId<T>(shortId);

#region 'Patch' & 'Exists' for individual records [PUBLIC, PRIVATE] ---

/// <inheritdoc />
public void PatchWithoutValidation<TEntity, TItem>(
string shortId,
Expression<Func<TEntity, IEnumerable<TItem>>> path,
Expression<Func<JavaScriptArray<TItem>, object>> arrayAdder
) where TEntity : IEntity
{
// Consider adding tenant validation in here
DbSession.Advanced.Patch(GetFullId<TEntity>(shortId), path, arrayAdder);
}

/// <inheritdoc />
public async Task<bool> Patch<TEntity, TProp>(string shortId, Expression<Func<TEntity, TProp>> path, TProp value) where TEntity : IEntity
{
var fullId = GetFullId<TEntity>(shortId);
if (!IsNotTenantedType<TEntity>() && !await TenantedEntityExistsAsync<TEntity>(fullId))
{
ThrowArgumentExceptionIfRequired();
return false;
}

DbSession.Advanced.Patch(fullId, path, value);
return true;
}

/// <inheritdoc />
public Task<bool> ExistsAsync<TEntity>(string shortId, CancellationToken token = default) where TEntity : IEntity
{
var fullId = GetFullId<TEntity>(shortId);
return IsNotTenantedType<TEntity>()
? DbSession.Advanced.ExistsAsync(fullId, token)
: TenantedEntityExistsAsync<TEntity>(fullId, token);
}

/// <summary>
/// Validate if a tenanted record exists
/// </summary>
private async Task<bool> TenantedEntityExistsAsync<TEntity>(string fullId, CancellationToken token = default) where TEntity : IEntity
{
var recordsTenantId =
await ( from e in Query<TEntity>()
where e.Id == fullId // a condition to filter on tenant gets added in Query<T>()
select e.Id).SingleOrDefaultAsync(token);
return recordsTenantId != null;
}
#endregion / 'Patch' & 'Exists' for individual records [PUBLIC, PRIVATE]

#region Processing Deferred Path Queries [PUBLIC, PRIVATE] ------------

/// <inheritdoc/>
Expand Down Expand Up @@ -241,10 +302,10 @@ private async Task SendDeferredPatchByQueryOperationsAsync(CancellationToken tok
// Wait as long as it's required. It's better to fail (e.g. timeout on API response) rather than update on stale indexes
queryIndex.WaitForNonStaleResultsTimeout = TimeSpan.MaxValue;
queryIndex.WaitForNonStaleResults = true;
var sentOperation = await Advanced.DocumentStore.Operations.SendAsync(new PatchByQueryOperation(queryIndex), Advanced.SessionInfo, token);
var sentOperation = await DbSession.Advanced.DocumentStore.Operations.SendAsync(new PatchByQueryOperation(queryIndex), DbSession.Advanced.SessionInfo, token);

if (_waitingForIndexesAfterSave.HasValue)
// Waiting can come in handy during tests
// Waiting comes in handy in the tests
await sentOperation.WaitForCompletionAsync(_waitingForIndexesAfterSave.Value);

if (token.IsCancellationRequested)
Expand Down
96 changes: 77 additions & 19 deletions back-end/Database/Infrastructure/IAsyncTenantedDocumentSession.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Linq.Expressions;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -20,30 +21,13 @@ namespace Raven.Yabt.Database.Infrastructure
/// </remarks>
public interface IAsyncTenantedDocumentSession : IDisposable
{
/// <summary>
/// Direct access to <seealso cref="IAsyncDocumentSession.Advanced"/> property
/// </summary>
IAsyncAdvancedSessionOperations Advanced { get; }

/// <summary>
/// Flag defining the behaviour on requesting a record of a wrong tenant.
/// <value>true</value> - would throw an exception. <value>false</value> - silent return of NULL.
/// </summary>
bool ThrowExceptionOnWrongTenant { get; }

/// <summary>
/// Check if there are any changes to save or run any patch requests
/// An extension of the <see cref="IAdvancedDocumentSessionOperations.HasChanges"/> property usually accessed from <seealso cref="IAsyncDocumentSession.Advanced"/>
/// </summary>
bool HasChanges();

/// <summary>
/// Add a RavenDB patch request for executing after calling <see cref="IAsyncDocumentSession.SaveChangesAsync"/>
/// </summary>
/// <remarks>
/// Operation against indexes can't be performed in the same transaction. This method offloads the operation to the Server to run
/// </remarks>
void AddDeferredPatchQuery(IndexQuery patchQuery);
#region IAsyncDocumentSession methods ---------------------------------

/// <summary>
/// Extension of the <seealso cref="IAsyncDocumentSession.CountersFor(object)"/> method to get counters for the entity.
Expand All @@ -65,7 +49,17 @@ public interface IAsyncTenantedDocumentSession : IDisposable
/// Saves all the pending changes to the server.
/// Propagation of the <seealso cref="IAsyncDocumentSession.SaveChangesAsync"/>
/// </summary>
Task SaveChangesAsync(CancellationToken token = default);
/// <param name="token"> Cancellation token [Optional] </param>
/// <returns> True - changes were saved, False - no changes detected </returns>
Task<bool> SaveChangesAsync(CancellationToken token = default);

/// <summary>
/// Saves all the pending changes to the server.
/// </summary>
/// <param name="clearCache"> Clears session's cache on saving </param>
/// <param name="token"> Cancellation token [Optional] </param>
/// <returns> <value>true</value> - changes were saved, <value>false</value> - no changes detected </returns>
Task<bool> SaveChangesAsync(bool clearCache, CancellationToken token = default);

/// <summary>
/// Extension of the <seealso cref="IAsyncDocumentSession.StoreAsync(object, CancellationToken)"/> method to stores entity in session.
Expand Down Expand Up @@ -125,5 +119,69 @@ public interface IAsyncTenantedDocumentSession : IDisposable
/// <param name="collectionName"> Name of the collection (mutually exclusive with indexName) </param>
/// <param name="isMapReduce"> Whether we are querying a map/reduce index (modify how we treat identifier properties) </param>
IRavenQueryable<T> Query<T>(string? indexName = null, string? collectionName = null, bool isMapReduce = false);

#endregion / IAsyncDocumentSession methods ----------------------------

#region Patch requests (aka Set based operations) ---------------------

/// <summary>
/// Add a RavenDB patch request for executing after calling <see cref="IAsyncDocumentSession.SaveChangesAsync"/>
/// </summary>
/// <remarks>
/// Operation against indexes can't be performed in the same transaction. This method offloads the operation to the Server to run
/// </remarks>
void AddDeferredPatchQuery(IndexQuery patchQuery);

/// <summary>
/// Updates properties on a record without loading it and validation of the correct tenant
/// </summary>
/// <remarks>
/// Propagates to <see cref="IAsyncAdvancedSessionOperations.Patch{T,U}(string, Expression{Func{T, IEnumerable{U}}},Expression{Func{JavaScriptArray{U},object}})"/> method
/// </remarks>
void PatchWithoutValidation<TEntity, TItem>(
string shortId,
Expression<Func<TEntity, IEnumerable<TItem>>> path,
Expression<Func<JavaScriptArray<TItem>, object>> arrayAdder) where TEntity : IEntity;

/// <summary>
/// Updates properties on a record with validation of the correct tenant
/// </summary>
/// <returns>
/// True if the patch request was applied to the record. False otherwise
/// </returns>
/// <remarks>
/// Propagates to <see cref="IAsyncAdvancedSessionOperations.Patch{T,U}(string,Expression{Func{T,U}},U)"/> method
/// </remarks>
Task<bool> Patch<TEntity, TProp>(string shortId, Expression<Func<TEntity, TProp>> path, TProp value) where TEntity : IEntity;

#endregion / Patch requests (aka Set based operations) ----------------

#region Auxilliary methods --------------------------------------------

/// <summary>
/// Check if document exists
/// </summary>
/// <param name="shortId"> Short document ID </param>
/// <param name="token"> Cancellation token [Optional] </param>
/// <remarks>
/// Propagates to <see cref="IAsyncAdvancedSessionOperations.ExistsAsync(string, CancellationToken)"/> method
/// </remarks>
Task<bool> ExistsAsync<TEntity>(string shortId, CancellationToken token = default) where TEntity : IEntity;

/// <summary>
/// Check if there are any changes to save or run any patch requests
/// An extension of the <see cref="IAdvancedDocumentSessionOperations.HasChanges"/> property usually accessed from <seealso cref="IAsyncDocumentSession.Advanced"/>
/// </summary>
bool HasChanges();

/// <summary>
/// Gets full document ID for a given entity (e.g. for '1-A' returns 'users/1-A')
/// </summary>
/// <typeparam name="T"> The entity type (e.g. class `Users`) </typeparam>
/// <param name="shortId"> The short ID (e.g. '1-A') </param>
/// <returns> A full ID (e.g. 'users/1-A') </returns>
string GetFullId<T>(string shortId) where T : IEntity;

#endregion / Auxilliary methods ---------------------------------------
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public async Task<IDomainResult<BacklogItemReference>> Delete(string shortId)
if (ticket.RelatedItems.Any())
foreach (var item in ticket.RelatedItems)
{
DbSession.Advanced.Patch<BacklogItem, BacklogItemRelatedItem>(GetFullId(item.RelatedTo.Id!),
DbSession.PatchWithoutValidation<BacklogItem, BacklogItemRelatedItem>(item.RelatedTo.Id!,
x => x.RelatedItems,
items => items.RemoveAll(i => i.RelatedTo.Id == shortId));
}
Expand All @@ -95,22 +95,16 @@ public async Task<IDomainResult<BacklogItemReference>> Delete(string shortId)

public async Task<IDomainResult> SetState(string backlogItemId, BacklogItemState newState)
{
var fullId = GetFullId(backlogItemId);
if (!await DbSession.Advanced.ExistsAsync(fullId))
if (!await DbSession.Patch<BacklogItem, BacklogItemState>(backlogItemId, x => x.State, newState))
return IDomainResult.NotFound();

DbSession.Advanced.Patch<BacklogItem, BacklogItemState>(fullId, x => x.State, newState);

await AddHistoryRecordPatch(backlogItemId, $"Changed status to {newState.ToString()}");

return IDomainResult.Success();
}

public async Task<IDomainResult> AssignToUser(string backlogItemId, string? userShortenId)
{
var fullId = GetFullId(backlogItemId);
if (!await DbSession.Advanced.ExistsAsync(fullId))
return IDomainResult.NotFound("The Backlog Item not found");

UserReference? userRef = null;
if (userShortenId != null)
{
Expand All @@ -119,7 +113,9 @@ public async Task<IDomainResult> AssignToUser(string backlogItemId, string? user
return DomainResult.NotFound("The user not found");
}

DbSession.Advanced.Patch<BacklogItem, UserReference?>(fullId, x => x.Assignee, userRef);
if (!await DbSession.Patch<BacklogItem, UserReference?>(backlogItemId, x => x.Assignee, userRef))
return IDomainResult.NotFound("The Backlog Item not found");

await AddHistoryRecordPatch(backlogItemId, userRef == null ? "Removed assigned user" : $"Assigned user '{userRef.MentionedName}'");

return IDomainResult.Success();
Expand All @@ -133,8 +129,8 @@ private async Task UpdateRelatedItems<T>(T dto, BacklogItemReference ticketRef)
{
if (link.ActionType == ListActionType.Add)
{
DbSession.Advanced.Patch<BacklogItem, BacklogItemRelatedItem>(
GetFullId(link.BacklogItemId),
DbSession.PatchWithoutValidation<BacklogItem, BacklogItemRelatedItem>(
link.BacklogItemId,
x => x.RelatedItems,
items => items.Add(
new BacklogItemRelatedItem
Expand All @@ -149,8 +145,8 @@ private async Task UpdateRelatedItems<T>(T dto, BacklogItemReference ticketRef)
{
var relatedId = ticketRef.Id!;
var relationType = link.RelationType.GetMirroredType();
DbSession.Advanced.Patch<BacklogItem, BacklogItemRelatedItem>(
GetFullId(link.BacklogItemId),
DbSession.PatchWithoutValidation<BacklogItem, BacklogItemRelatedItem>(
link.BacklogItemId,
x => x.RelatedItems,
items => items.RemoveAll(
i => i.RelatedTo.Id! == relatedId && i.LinkType == relationType));
Expand All @@ -163,8 +159,8 @@ private async Task UpdateRelatedItems<T>(T dto, BacklogItemReference ticketRef)
private async Task AddHistoryRecordPatch(string backlogItemId, string message)
{
var userRef = await _userResolver.GetCurrentUserReference();
DbSession.Advanced.Patch<BacklogItem, BacklogItemHistoryRecord>(
GetFullId(backlogItemId),
DbSession.PatchWithoutValidation<BacklogItem, BacklogItemHistoryRecord>(
backlogItemId,
x => x.ModifiedBy,
items => items.Add(new BacklogItemHistoryRecord(userRef, message)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void ClearUserId(string userId)
// Add the patch to a collection
DbSession.AddDeferredPatchQuery(query);
}

public void UpdateReferences(UserReference newUserReference)
{
if (string.IsNullOrEmpty(newUserReference.Id))
Expand Down Expand Up @@ -86,7 +86,7 @@ public void UpdateReferences(UserReference newUserReference)
QueryParameters = new Parameters
{
{ "userId", newUserReference.Id },
{ "newMention", newUserReference!.MentionedName },
{ "newMention", newUserReference.MentionedName },
}
};

Expand Down
2 changes: 1 addition & 1 deletion back-end/Domain/Common/BaseService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ protected BaseService(IAsyncTenantedDocumentSession dbSession): base(dbSession)
protected string GetFullId(string id)
=> id.Contains('/')
? id // Assume it's already a full ID with a prefix
: DbSession.Advanced.GetFullId<TEntity>(id);
: DbSession.GetFullId<TEntity>(id);
}

public abstract class BaseDbService
Expand Down
Loading

0 comments on commit b0d6b56

Please sign in to comment.