Merge pull request #49 from Encamina/@rliberoff/improvements_on_ephem…

…aral_memory_handler

Various improvements in IMemoryStore, IMemoryManager and IMemoryStoreHandler.

CHANGELOG.md

@@ -18,8 +18,14 @@ Previous classification is not required if changes are simple or all belong to t
 
 ## [8.1.2]
 
+### Braking Changes
+- Replace dependency with `IMemoryStore` for `IMemoryManager` in abstract class `MemoryStoreHandlerBase`. This affects internal types like the `EphemeralMemoryStoreHandler`.
+- Removed visibility modifiers in `IMemoryManager` interface.
+
 ### Major change
 - Method `GetDocumentConnector` in `DocumentContentExtractorBase` is now `public` instead of `protected`.
+- New `MemoryManager` property of type `IMemoryManager` in `IMemoryStoreHandler` interface to get read-only access to the underlaying memory manager.
+- New `MemoryStore` property of type `IMemoryStore` in `IMemoryManager` interface to get read-only access to the underlaying memory store.
 
 ### Minor Changes
 - Properties `CollectionNamePostfix` and `CollectionNamePrefix` from `MemoryStoreHandlerBase` are now `virtual` instead of `abstract`.

Directory.Build.props

@@ -17,7 +17,7 @@
 
   <PropertyGroup>
     <VersionPrefix>8.1.2</VersionPrefix>
-    <VersionSuffix>preview-3</VersionSuffix>
+    <VersionSuffix>preview-4</VersionSuffix>
   </PropertyGroup>
 
   <!--

src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryManager.cs

@@ -1,4 +1,6 @@
-using Microsoft.SemanticKernel.Memory;
+// Ignore Spelling: Upsert
+
+using Microsoft.SemanticKernel.Memory;
 
 namespace Encamina.Enmarcha.SemanticKernel.Abstractions;
 
@@ -7,6 +9,11 @@ namespace Encamina.Enmarcha.SemanticKernel.Abstractions;
 /// </summary>
 public interface IMemoryManager
 {
+    /// <summary>
+    /// Gets the instance of the memory store manage by this manager.
+    /// </summary>
+    IMemoryStore MemoryStore { get; }
+
     /// <summary>
     /// Upserts the memory content into a collection.
     /// </summary>
@@ -16,7 +23,7 @@ public interface IMemoryManager
     /// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
     /// <param name="metadata">Metadata of the memory.</param>
     /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
-    public Task UpsertMemoryAsync(string memoryId, string collectionName, IEnumerable<string> chunks, CancellationToken cancellationToken, IDictionary<string, string> metadata = null);
+    Task UpsertMemoryAsync(string memoryId, string collectionName, IEnumerable<string> chunks, CancellationToken cancellationToken, IDictionary<string, string> metadata = null);
 
     /// <summary>
     /// Deletes the memory content from a collection.
@@ -25,7 +32,7 @@ public interface IMemoryManager
     /// <param name="collectionName">Name of the collection from where the content will be deleted.</param>
     /// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
     /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
-    public Task DeleteMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);
+    Task DeleteMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);
 
     /// <summary>
     /// Gets the memory content from a collection.
@@ -34,15 +41,15 @@ public interface IMemoryManager
     /// <param name="collectionName">Name of the collection where the content will be saved.</param>
     /// <param name="cancellationToken">Cancellation token to cancel the operation.</param>
     /// <returns>A <see cref="Task"/> containing the <see cref="MemoryContent"/>, or <see langword="null"/> if the content could not be found.</returns>
-    public Task<MemoryContent> GetMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);
+    Task<MemoryContent> GetMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);
 
     /// <summary>
     /// Upserts a batch of memory contents into a collection.
     /// </summary>
     /// <param name="collectionName">Name of the collection where the content will be saved.</param>
     /// <param name="memoryContents">
     /// Dictionary with the memory contents to upsert.
-    /// The <c>key</c> in the dictionary must containt a unique identifier for the content of the memory,
+    /// The <c>key</c> in the dictionary must contain a unique identifier for the content of the memory,
     /// and the <c>value</c> of the dictionary must provide the memory content (chunks and metadata).
     /// </param>
     /// <param name="cancellationToken">Cancellation token to cancel the operation.</param>

src/Encamina.Enmarcha.SemanticKernel.Abstractions/IMemoryStoreHandler.cs

@@ -3,8 +3,8 @@
 namespace Encamina.Enmarcha.SemanticKernel.Abstractions;
 
 /// <summary>
-/// Represents types that helps working with memory stores (i.e., <see cref="IMemoryStore"/>), when sometimes
-/// it is necessary to centrally manage how some operations or actions are done.
+/// Represents types that helps working with memory stores (i.e., <see cref="IMemoryStore"/>) through a memory handler (<see cref="IMemoryManager"/>),
+/// when sometimes it is necessary to centrally manage how some operations or actions are done.
 /// </summary>
 public interface IMemoryStoreHandler
 {
@@ -18,6 +18,11 @@ public interface IMemoryStoreHandler
     /// </summary>
     string CollectionNamePrefix { get; init; }
 
+    /// <summary>
+    /// Gets the <see cref="IMemoryManager"/> that manages the memory stored handled by this instance.
+    /// </summary>
+    IMemoryManager MemoryManager { get; }
+
     /// <summary>
     /// Gets the name of a collection from its unique identifier.
     /// </summary>

src/Encamina.Enmarcha.SemanticKernel.Abstractions/MemoryStoreHandlerBase.cs

@@ -14,14 +14,15 @@ public abstract class MemoryStoreHandlerBase : IMemoryStoreHandler
     /// <summary>
     /// Initializes a new instance of the <see cref="MemoryStoreHandlerBase"/> class.
     /// </summary>
-    /// <param name="memoryStore">The <see cref="IMemoryStore"/> to handle.</param>
-#pragma warning disable SKEXP0003 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
-    protected MemoryStoreHandlerBase(IMemoryStore memoryStore)
-#pragma warning restore SKEXP0003 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
+    /// <param name="memoryManager">A valid instance of <see cref="IMemoryManager"/> that manages the memory store handled by this instance.</param>
+    protected MemoryStoreHandlerBase(IMemoryManager memoryManager)
     {
-        MemoryStore = memoryStore;
+        MemoryManager = memoryManager;
     }
 
+    /// <inheritdoc/>
+    public virtual IMemoryManager MemoryManager { get; }
+
     /// <inheritdoc/>
     public virtual string CollectionNamePostfix { get; init; }
 
@@ -33,11 +34,6 @@ protected MemoryStoreHandlerBase(IMemoryStore memoryStore)
     /// </summary>
     protected IDictionary<string, MemoryStoreCollection> MemoryStoreCollectionInfo { get; } = new ConcurrentDictionary<string, MemoryStoreCollection>();
 
-    /// <summary>
-    /// Gets the current <see cref="IMemoryStore"/> handled by this instance.
-    /// </summary>
-    protected IMemoryStore MemoryStore { get; }
-
     /// <inheritdoc/>
     public virtual async Task<string> GetCollectionNameAsync(string collectionId, CancellationToken cancellationToken)
     {
@@ -55,9 +51,9 @@ public virtual async Task<string> GetCollectionNameAsync(string collectionId, Ca
             LastAccessUtc = DateTime.UtcNow,
         };
 
-        if (!await MemoryStore.DoesCollectionExistAsync(collectionName, cancellationToken))
+        if (!await MemoryManager.MemoryStore.DoesCollectionExistAsync(collectionName, cancellationToken))
         {
-            await MemoryStore.CreateCollectionAsync(collectionName, cancellationToken);
+            await MemoryManager.MemoryStore.CreateCollectionAsync(collectionName, cancellationToken);
         }
 
         return MemoryStoreCollectionInfo[collectionId].CollectionName;

src/Encamina.Enmarcha.SemanticKernel/EphemeralMemoryStoreHandler.cs

@@ -20,11 +20,11 @@ internal sealed class EphemeralMemoryStoreHandler : MemoryStoreHandlerBase
     /// <summary>
     /// Initializes a new instance of the <see cref="EphemeralMemoryStoreHandler"/> class.
     /// </summary>
-    /// <param name="memoryStore">A valid instance of an <see cref="IMemoryStore"/> handled by this memory store handler.</param>
+    /// <param name="memoryManager">A valid instance of <see cref="IMemoryManager"/> that manages the memory store handled by this instance.</param>
     /// <param name="sessionManagementOptions">Configuration options for this memory store handler.</param>
     /// <param name="logger">A logger for this memory store handler.</param>
-    public EphemeralMemoryStoreHandler(IMemoryStore memoryStore, IOptionsMonitor<EphemeralMemoryStoreHandlerOptions> sessionManagementOptions, ILogger<EphemeralMemoryStoreHandler> logger)
-        : base(memoryStore)
+    public EphemeralMemoryStoreHandler(IMemoryManager memoryManager, IOptionsMonitor<EphemeralMemoryStoreHandlerOptions> sessionManagementOptions, ILogger<EphemeralMemoryStoreHandler> logger)
+        : base(memoryManager)
     {
         this.logger = logger;
 
@@ -54,7 +54,7 @@ private async Task RemoveOutdatedCollectionsAsync(CancellationToken cancellation
                 foreach (var memoryStoreInfo in MemoryStoreCollectionInfo.Where(i => i.Value.LastAccessUtc < date).ToList())
                 {
                     MemoryStoreCollectionInfo.Remove(memoryStoreInfo.Key);
-                    await MemoryStore.DeleteCollectionAsync(memoryStoreInfo.Value.CollectionName, cancellationToken);
+                    await MemoryManager.MemoryStore.DeleteCollectionAsync(memoryStoreInfo.Value.CollectionName, cancellationToken);
                 }
 
                 Thread.Sleep(TimeSpan.FromMinutes(options.InactivePollingTimeMinutes));

src/Encamina.Enmarcha.SemanticKernel/MemoryManager.cs

@@ -16,21 +16,30 @@ namespace Encamina.Enmarcha.SemanticKernel;
 /// <summary>
 /// Manager that provides some CRUD operations over memories with multiple chunks that need to be managed by an <see cref="IMemoryStore"/>, using batch operations.
 /// </summary>
-/// <remarks>
-/// Initializes a new instance of the <see cref="MemoryManager"/> class.
-/// </remarks>
-/// <param name="kernel">
-/// A valid instance of <see cref="Kernel"/>, used to get the configured text embeddings generation service (<see cref="ITextEmbeddingGenerationService"/>) required by this manager.
-/// </param>
-/// <param name="memoryStore">A valid instance of a <see cref="IMemoryStore"/> to manage.</param>
-public class MemoryManager(Kernel kernel, IMemoryStore memoryStore) : IMemoryManager
+public class MemoryManager : IMemoryManager
 {
     private const string ChunkSize = @"chunkSize";
 
-    private readonly Kernel kernel = kernel;
+    private readonly Kernel kernel;
 
-    private readonly ILogger logger = kernel.LoggerFactory.CreateLogger<MemoryManager>();
-    private readonly IMemoryStore memoryStore = memoryStore;
+    private readonly ILogger logger;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="MemoryManager"/> class.
+    /// </summary>
+    /// <param name="kernel">
+    /// A valid instance of <see cref="Kernel"/>, used to get the configured text embeddings generation service (<see cref="ITextEmbeddingGenerationService"/>) required by this manager.
+    /// </param>
+    /// <param name="memoryStore">A valid instance of a <see cref="IMemoryStore"/> to manage.</param>
+    public MemoryManager(Kernel kernel, IMemoryStore memoryStore)
+    {
+        this.kernel = kernel;
+        logger = kernel.LoggerFactory.CreateLogger<MemoryManager>();
+        MemoryStore = memoryStore;
+    }
+
+    /// <inheritdoc/>
+    public IMemoryStore MemoryStore { get; init; }
 
     /// <inheritdoc/>
     public virtual async Task UpsertMemoryAsync(string memoryId, string collectionName, IEnumerable<string> chunks, CancellationToken cancellationToken, IDictionary<string, string> metadata = null)
@@ -66,7 +75,7 @@ public virtual async Task<MemoryContent> GetMemoryAsync(string memoryId, string
             return null;
         }
 
-        var memoryRecords = await memoryStore.GetBatchAsync(collectionName, Enumerable.Range(0, chunkSize).Select(i => BuildMemoryIdentifier(memoryId, i)), cancellationToken: cancellationToken)
+        var memoryRecords = await MemoryStore.GetBatchAsync(collectionName, Enumerable.Range(0, chunkSize).Select(i => BuildMemoryIdentifier(memoryId, i)), cancellationToken: cancellationToken)
                                              .ToListAsync(cancellationToken);
 
         return new MemoryContent
@@ -100,7 +109,7 @@ public virtual async IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string co
             }
         }
 
-        var memoryRecordsUniqueIdentifiers = memoryStore.UpsertBatchAsync(collectionName, memoryRecords, cancellationToken);
+        var memoryRecordsUniqueIdentifiers = MemoryStore.UpsertBatchAsync(collectionName, memoryRecords, cancellationToken);
 
         await foreach (var item in memoryRecordsUniqueIdentifiers)
         {
@@ -113,7 +122,7 @@ public virtual async IAsyncEnumerable<string> BatchUpsertMemoriesAsync(string co
 
     private async Task<int> GetChunkSize(string memoryId, string collectionName, CancellationToken cancellationToken)
     {
-        var fistMemoryChunk = await memoryStore.GetAsync(collectionName, BuildMemoryIdentifier(memoryId, 0), cancellationToken: cancellationToken);
+        var fistMemoryChunk = await MemoryStore.GetAsync(collectionName, BuildMemoryIdentifier(memoryId, 0), cancellationToken: cancellationToken);
 
         if (fistMemoryChunk == null)
         {
@@ -127,7 +136,7 @@ private async Task<int> GetChunkSize(string memoryId, string collectionName, Can
 
     private async Task DeleteMemoryAsync(string memoryId, string collectionName, int chunkSize, CancellationToken cancellationToken)
     {
-        await memoryStore.RemoveBatchAsync(collectionName, Enumerable.Range(0, chunkSize).Select(i => BuildMemoryIdentifier(memoryId, i)), cancellationToken);
+        await MemoryStore.RemoveBatchAsync(collectionName, Enumerable.Range(0, chunkSize).Select(i => BuildMemoryIdentifier(memoryId, i)), cancellationToken);
     }
 
     private async Task SaveChunks(string memoryid, string collectionName, IEnumerable<string> chunks, IDictionary<string, string> metadata, CancellationToken cancellationToken)
@@ -149,6 +158,6 @@ private async Task SaveChunks(string memoryid, string collectionName, IEnumerabl
             memoryRecords.Add(MemoryRecord.LocalRecord(BuildMemoryIdentifier(memoryid, i), chunk, null, embedding, metadataJson));
         }
 
-        await memoryStore.UpsertBatchAsync(collectionName, memoryRecords, cancellationToken).ToListAsync(cancellationToken: cancellationToken);
+        await MemoryStore.UpsertBatchAsync(collectionName, memoryRecords, cancellationToken).ToListAsync(cancellationToken: cancellationToken);
     }
 }