Skip to content

Commit

Permalink
Merge pull request #122 from Encamina/@hramos/FixWarnings/Encamina.En…
Browse files Browse the repository at this point in the history
…marcha.SemanticKernel

Fixed some warnings in Encamina.Enmarcha.SemanticKernel
  • Loading branch information
HugoRamosEs authored Jun 10, 2024
2 parents b71efcb + 73e1e25 commit 80e53f7
Show file tree
Hide file tree
Showing 20 changed files with 104 additions and 70 deletions.
14 changes: 9 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ Previous classification is not required if changes are simple or all belong to t
### Minor Changes

- Added `MaxTokensOutput` property in `ModelInfo`.
- Added `SaveChatMessagesHistoryBatchAsync` in `ChatHistoryProvider`.
- Fixed some warnings in:
- `Encamina.Enmarcha.Bot`
- `Encamina.Enmarcha.Core`
- `Encamina.Enmarcha.Data`
- `Encamina.Enmarcha.Email`
- `Encamina.Enmarcha.Entities`
- `Encamina.Enmarcha.SemanticKernel`

## [8.1.6]

Expand Down Expand Up @@ -83,13 +91,9 @@ Previous classification is not required if changes are simple or all belong to t
- Class `SlidePptxDocumentConnector` is now `public` instead of `internal`.
- Added `UseAzureActiveDirectoryAuthentication` and `TokenCredentialsOptions` properties in `AzureOpenAIOptions`.
- Added `RequiredIfAttribute` to validate properties based on the value of another property.
- Fixed warnings CS8603 and CS8025 in:
- Fixed some warnings in:
- `Encamina.Enmarcha.AI`.
- `Encamina.Enmarcha.AspNet`
- `Encamina.Enmarcha.Bot`
- `Encamina.Enmarcha.Core`
- `Encamina.Enmarcha.Data`
- `Encamina.Enmarcha.Email`
- Corrected a typo in the Spanish error message in `ResponseMessages.es.resx` from "ha encontrar" to "ha encontrado".

## [8.1.5]
Expand Down
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

<PropertyGroup>
<VersionPrefix>8.1.7</VersionPrefix>
<VersionSuffix>preview-05</VersionSuffix>
<VersionSuffix>preview-06</VersionSuffix>
</PropertyGroup>

<!--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public interface IServiceFactory<out T> : IDisposable where T : class
/// otherwise <see langword="false"/> and don't throw any exception.
/// </param>
/// <returns>A valid instance of the service if found.</returns>
T ById<TId>(TId serviceId, bool throwIfNotFound);
T? ById<TId>(TId serviceId, bool throwIfNotFound);

/// <summary>
/// Gets a service by its name.
Expand All @@ -42,7 +42,7 @@ public interface IServiceFactory<out T> : IDisposable where T : class
/// otherwise <see langword="false"/> and don't throw any exception.
/// </param>
/// <returns>A valid instance of the service if found.</returns>
T ByName(string serviceName, bool throwIfNotFound);
T? ByName(string serviceName, bool throwIfNotFound);

/// <summary>
/// Gets a service by its type.
Expand All @@ -64,5 +64,5 @@ public interface IServiceFactory<out T> : IDisposable where T : class
/// otherwise <see langword="false"/> and don't throw any exception.
/// </param>
/// <returns>A valid instance of the service if found.</returns>
T ByType(Type serviceType, bool throwIfNotFound);
T? ByType(Type serviceType, bool throwIfNotFound);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ public abstract class IdentifiableBase<T> : IIdentifiable<T>
public virtual T Id { get; init; }

Check warning on line 13 in src/Encamina.Enmarcha.Entities.Abstractions/IdentifiableBase{T}.cs

View workflow job for this annotation

GitHub Actions / CI

Non-nullable property 'Id' must contain a non-null value when exiting constructor. Consider declaring the property as nullable.

/// <inheritdoc/>
object IIdentifiable.Id => Id;
object IIdentifiable.Id => Id!;
}
16 changes: 8 additions & 8 deletions src/Encamina.Enmarcha.Entities/ServiceFactory{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,28 @@ public ServiceFactory(IServiceScope serviceScope)
}

/// <inheritdoc/>
public virtual T ById<TId>(TId serviceId) => ById(serviceId, true);
public virtual T ById<TId>(TId serviceId) => ById(serviceId, true)!;

/// <inheritdoc/>
public virtual T ById<TId>(TId serviceId, bool throwIfNotFound)
public virtual T? ById<TId>(TId serviceId, bool throwIfNotFound)
{
return ProcessService(GetService<T>(s => s is IIdentifiable<TId> identifiable && identifiable.Id.Equals(serviceId)), throwIfNotFound, Resources.ExceptionMessages.InvalidServiceId, nameof(serviceId), typeof(T), serviceId);
}

/// <inheritdoc/>
public virtual T ByName(string serviceName) => ByName(serviceName, true);
public virtual T ByName(string serviceName) => ByName(serviceName, true)!;

/// <inheritdoc/>
public virtual T ByName(string serviceName, bool throwIfNotFound)
public virtual T? ByName(string serviceName, bool throwIfNotFound)
{
return ProcessService(GetService<T>(s => s is INameable nameable && nameable.Name.Equals(serviceName)), throwIfNotFound, Resources.ExceptionMessages.InvalidServiceName, nameof(serviceName), typeof(T), serviceName);
}

/// <inheritdoc/>
public virtual T ByType(Type serviceType) => ByType(serviceType, true);
public virtual T ByType(Type serviceType) => ByType(serviceType, true)!;

/// <inheritdoc/>
public virtual T ByType(Type serviceType, bool throwIfNotFound)
public virtual T? ByType(Type serviceType, bool throwIfNotFound)
{
return ProcessService(GetService<T>(s => s.GetType() == serviceType), throwIfNotFound, Resources.ExceptionMessages.InvaidServiceType, nameof(serviceType), serviceType);
}
Expand All @@ -67,7 +67,7 @@ public void Dispose()
/// This method disposes the <see cref="IServiceScope"/> used by this factory.
/// </remarks>
/// <param name="disposing">
/// A flag value indicating whether this instance is being disponsed or not, to prevent redundant calls.
/// A flag value indicating whether this instance is being disposed or not, to prevent redundant calls.
/// </param>
protected virtual void Dispose(bool disposing)
{
Expand All @@ -82,7 +82,7 @@ protected virtual void Dispose(bool disposing)
}
}

private static T ProcessService(T service, bool throwIfNotFound, string exceptionMessage, string parameterName, params object[] exceptionMessageParameters)
private static T? ProcessService(T service, bool throwIfNotFound, string exceptionMessage, string parameterName, params object[] exceptionMessageParameters)
{
return service == null && throwIfNotFound
? throw new ArgumentException(string.Format(exceptionMessage, exceptionMessageParameters), parameterName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public static string ReadValueFromRequestHeader(this HttpContext httpContext, st
Guard.IsNotNull(headerName);

return httpContext.Request.Headers.TryGetValue(headerName, out var headerValue) && headerValue.Any()
? headerValue[0].TrimAndAsNullIfEmpty() ?? defaultValue
? headerValue[0]?.TrimAndAsNullIfEmpty() ?? defaultValue
: defaultValue;
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.SemanticKernel.ChatCompletion;
using Microsoft.SemanticKernel;
using Microsoft.SemanticKernel.ChatCompletion;

namespace Encamina.Enmarcha.SemanticKernel.Abstractions;

Expand Down Expand Up @@ -34,4 +35,13 @@ public interface IChatHistoryProvider
/// <param name="cancellationToken">A cancellation token that can be used to receive notice of cancellation.</param>
/// <returns>A <see cref="Task"/> that on completion indicates the asynchronous operation has executed.</returns>
Task SaveChatMessagesHistoryAsync(string indexerId, string roleName, string message, CancellationToken cancellationToken);

/// <summary>
/// Save a set of messages in the chat history.
/// </summary>
/// <param name="indexerId">The unique identifier of the chat history indexer.</param>
/// <param name="messages">The list of messages to be saved.</param>
/// <param name="cancellationToken">A cancellation token that can be used to receive notice of cancellation.</param>
/// <returns>A <see cref="Task"/> that on completion indicates the asynchronous operation has executed.</returns>
Task SaveChatMessagesHistoryBatchAsync(string indexerId, IEnumerable<ChatMessageContent> messages, CancellationToken cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Task UpsertMemoryAsync(
string collectionName,
IEnumerable<string> chunks,
Kernel kernel,
IDictionary<string, string> metadata = null,
IDictionary<string, string>? metadata = null,
CancellationToken cancellationToken = default);

/// <summary>
Expand All @@ -63,7 +63,7 @@ Task UpsertMemoryAsync(
/// <param name="collectionName">Name of the collection where the content will be retrieved.</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>
Task<MemoryContent> GetMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);
Task<MemoryContent?> GetMemoryAsync(string memoryId, string collectionName, CancellationToken cancellationToken);

/// <summary>
/// Checks if the memory exists in a collection.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public void AppendText(Stream stream, string text)
// Intentionally not implemented to comply with the Liskov Substitution Principle
}

private static string ApplyStyles(string value, bool bold, bool italic)
private static string? ApplyStyles(string? value, bool bold, bool italic)
{
if (string.IsNullOrWhiteSpace(value))
{
Expand Down Expand Up @@ -157,7 +157,7 @@ private static string ApplyStyles(string value, bool bold, bool italic)
return value;
}

private string GetCellTextValue(Cell cell)
private string? GetCellTextValue(Cell cell)
{
// Get the cell value with or without formatting
var cellValue = WithFormattedValues ? cell.FormattedText : cell.Text;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private static void AddTocContent(List<TocItem> tocItems, IReadOnlyCollection<Pa
// It is the last title. Extract the text to the end.

var lastPage = pages.MaxBy(p => p.Number);
var lastIndex = lastPage.Content.Length;
var lastIndex = lastPage!.Content.Length;

tocContent = GetTocContent(pages, currentPage, currentIndex, lastPage, lastIndex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ public virtual void AddDocumentConnector(string fileExtension, IDocumentConnecto
/// <inheritdoc/>
public virtual IDocumentConnector GetDocumentConnector(string fileExtension)
{
return GetDocumentConnector(fileExtension, true);
return GetDocumentConnector(fileExtension, true)!;
}

/// <inheritdoc/>
public IDocumentConnector GetDocumentConnector(string fileExtension, bool throwException)
public IDocumentConnector? GetDocumentConnector(string fileExtension, bool throwException)
{
if (documentConnectors.TryGetValue(fileExtension.ToUpperInvariant(), out var value))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ internal static class CellTypeExtensions
/// <param name="cell">The cell to get the value from.</param>
/// <param name="workbookPart">The workbook part that contains the cell.</param>
/// <returns>A tuple containing the cell value and a boolean indicating whether the cell contains text.</returns>
public static (string Value, bool IsText) GetCellValue(this CellType cell, WorkbookPart workbookPart)
public static (string? Value, bool IsText) GetCellValue(this CellType cell, WorkbookPart workbookPart)
{
if (cell == null)
{
Expand All @@ -77,7 +77,7 @@ public static (string Value, bool IsText) GetCellValue(this CellType cell, Workb
/// <param name="cell">The cell to get the value from.</param>
/// <param name="workbookPart">The workbook part that contains the cell.</param>
/// <returns>The raw text value of the cell.</returns>
public static string GetCellTextValue(this CellType cell, WorkbookPart workbookPart)
public static string? GetCellTextValue(this CellType cell, WorkbookPart workbookPart)
{
if (cell == null)
{
Expand All @@ -103,7 +103,7 @@ public static string GetCellTextValue(this CellType cell, WorkbookPart workbookP
/// <param name="cell">The cell to get the value from.</param>
/// <param name="workbookPart">The workbook part that contains the cell.</param>
/// <returns>The formatted text value of the cell.</returns>
public static string GetCellFormattedTextValue(this CellType cell, WorkbookPart workbookPart)
public static string? GetCellFormattedTextValue(this CellType cell, WorkbookPart workbookPart)
{
if (cell == null)
{
Expand All @@ -114,11 +114,11 @@ public static string GetCellFormattedTextValue(this CellType cell, WorkbookPart
&& double.TryParse(cell.CellValue?.InnerText, out var number)
&& cell.StyleIndex != null && workbookPart.WorkbookStylesPart?.Stylesheet is { CellFormats: not null, NumberingFormats: not null })
{
var cellFormat = workbookPart.WorkbookStylesPart.Stylesheet.CellFormats.ElementAtOrDefault((int)cell.StyleIndex.Value) as CellFormat;
var cellFormat = workbookPart.WorkbookStylesPart.Stylesheet.CellFormats?.ElementAtOrDefault((int)cell.StyleIndex.Value) as CellFormat;

if (cellFormat?.NumberFormatId != null)
{
string format = workbookPart.WorkbookStylesPart.Stylesheet.NumberingFormats.Elements<NumberingFormat>()
string? format = workbookPart.WorkbookStylesPart.Stylesheet.NumberingFormats?.Elements<NumberingFormat>()
.FirstOrDefault(i => i.NumberFormatId == cellFormat.NumberFormatId)?
.FormatCode;

Expand All @@ -127,7 +127,7 @@ public static string GetCellFormattedTextValue(this CellType cell, WorkbookPart
format = defaultFormat;
}

return ConvertToExcelFormat(number, format);
return ConvertToExcelFormat(number, format!);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public interface IDocumentConnectorProvider
/// <exception cref="InvalidOperationException">
/// If the <paramref name="fileExtension"/> is not supported or no suitable <see cref="IDocumentConnector"/> instance for it can be found.
/// </exception>
IDocumentConnector GetDocumentConnector(string fileExtension, bool throwException);
IDocumentConnector? GetDocumentConnector(string fileExtension, bool throwException);

/// <summary>
/// Determines whether a specified file extension is supported.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ internal class Cell
/// <summary>
/// Gets the text in the cell.
/// </summary>
public string Text { get; private init; }
public string? Text { get; private init; }

/// <summary>
/// Gets the formatted text in the cell.
/// </summary>
public string FormattedText { get; private init; }
public string? FormattedText { get; private init; }

/// <summary>
/// Gets a value indicating whether the text in the cell is null, empty or white space.
Expand All @@ -27,7 +27,7 @@ internal class Cell
/// <summary>
/// Gets the reference of the cell. Such us A1, B2, etc.
/// </summary>
public string Reference { get; private init; }
public string? Reference { get; private init; }

/// <summary>
/// Gets a value indicating whether the text in the cell is bold.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal class Worksheet
/// <summary>
/// Gets the name of the worksheet.
/// </summary>
public string Name { get; private init; }
public string? Name { get; private init; }

/// <summary>
/// Gets a value indicating whether the worksheet is hidden.
Expand Down Expand Up @@ -150,7 +150,7 @@ private static IReadOnlyList<int> GetHiddenRows(DocumentFormat.OpenXml.Spreadshe
{
return worksheet.Descendants<Row>()
.Where((r) => r.Hidden != null && r.Hidden.Value && r.RowIndex?.Value is not null)
.Select(r => (int)r.RowIndex.Value)
.Select(r => (int)r.RowIndex!.Value)
.ToList();
}

Expand All @@ -163,7 +163,7 @@ private static IReadOnlyList<int> GetHiddenColumns(DocumentFormat.OpenXml.Spread
{
return worksheet.Descendants<Column>()
.Where(c => c.Hidden != null && c.Hidden.Value && c.Min != null && c.Max != null)
.SelectMany(c => Enumerable.Range((int)c.Min.Value, (int)c.Max.Value - (int)c.Min.Value + 1))
.SelectMany(c => Enumerable.Range((int)c.Min!.Value, (int)c.Max!.Value - (int)c.Min.Value + 1))
.ToList();
}

Expand Down Expand Up @@ -304,8 +304,8 @@ private static List<List<Cell>> RemoveEmptyColumnsFromRows(List<List<Cell>> rows
/// <returns>A list of rows, each containing a list of cells, representing the range of cells with text.</returns>
private static List<List<Cell>> GetOnlyCellsRangeWithText(IList<List<Cell>> rows)
{
var firstNonEmptyRowIndex = rows.IndexOf(rows.FirstOrDefault(r => r.Exists(c => !c.IsNullOrWhiteSpace)));
var lastNonEmptyRowIndex = rows.IndexOf(rows.LastOrDefault(r => r.Exists(c => !c.IsNullOrWhiteSpace)));
var firstNonEmptyRowIndex = rows.IndexOf(rows.FirstOrDefault(r => r.Exists(c => !c.IsNullOrWhiteSpace))!);
var lastNonEmptyRowIndex = rows.IndexOf(rows.LastOrDefault(r => r.Exists(c => !c.IsNullOrWhiteSpace))!);

var columnsRange = Enumerable.Range(0, rows[0].Count).ToList();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
using Encamina.Enmarcha.SemanticKernel.Plugins.Chat.Plugins;

using Microsoft.Extensions.Options;

using Microsoft.SemanticKernel;
using Microsoft.SemanticKernel.ChatCompletion;

namespace Encamina.Enmarcha.SemanticKernel.Plugins.Chat;
Expand Down Expand Up @@ -113,4 +113,17 @@ await chatMessagesHistoryRepository.AddAsync(new ChatMessageHistoryRecord()
TimestampUtc = DateTime.UtcNow,
}, cancellationToken);
}

/// <inheritdoc/>
public async Task SaveChatMessagesHistoryBatchAsync(string indexerId, IEnumerable<ChatMessageContent> messages, CancellationToken cancellationToken)
{
await chatMessagesHistoryRepository.AddBatchAsync(messages.Select(message => new ChatMessageHistoryRecord()
{
Id = Guid.NewGuid().ToString(),
IndexerId = indexerId,
RoleName = message.Role.ToString(),
Message = message.Content!,
TimestampUtc = DateTime.UtcNow,
}), cancellationToken);
}
}
Loading

0 comments on commit 80e53f7

Please sign in to comment.