forevolve / forevolve-framework Goto Github PK

Calling AddForEvolveOperationResults in Startup.ConfigureServices should load OperationResults and its dependencies; implicitly calling AddForEvolveErrorFactory instead of throwing the following exception:

InvalidOperationException: Unable to resolve service for type 'ForEvolve.AspNetCore.IErrorFactory' while attempting to activate 'ForEvolve.AspNetCore.DefaultOperationResultFactory'.

• Create and deploy an ARM template that creates an Azure storage resource to run integration testing against.
• Update the tests to use that source instead of the emulator (only during VSTS builds?)
• Create a fast-build (filter -Trait:"Dependency") and a full-build (no filter)
• Remove the use of SUPPORT_AZURE_EMULATOR and favor the use of traits

OperationResult.Failure(Exception) should support inner exceptions.

• When ex.InnerException != null add the exception to the error collection.

Reason: it is not strictly related to APIs.

When an IOperationResult is in the scope of a Controller (http request), it should automatically populate its ModelState with the error messages instead of doing it "manually" everytime.

• This should be an opt-out feature. It should support:
  • Global setting
  • Per controller on/off
  • Per action on/off
• Use IObjectModelValidator to populate the ModelState
• Remove ModelStateDictionaryExtensions.AddModelErrorFrom()?

Should be able to send from a specific email account.

Interface
Add the following method to IEmailSenderService:

Task SendEmailAsync(MailAddress email, string subject, string message);

Task SendEmailAsync(string senderEmail, string recipientEmail, string subject, string message);
Task SendEmailAsync(MailAddress senderEmail, string recipientEmail, string subject, string message);
Task SendEmailAsync(MailAddress senderEmail, MailAddress recipientEmail, string subject, string message);

Implementation (DefaultEmailSenderService)
In the case that a method with a senderEmail is called, replace EmailOptions.SenderEmailAddress by the specified senderEmail.

Since the pipeline can be configured anyway, this boolean only creates confusion.

See: https://github.com/ForEvolve/ForEvolve-Framework/blob/master/src/ForEvolve.Markdown/Markdig/MarkdigStartupExtensions.cs.

See https://github.com/dotnet/sourcelink/

This feature aim at testing operation execution order.

Example:

• Confirming that the mail has been rendered to HTML before it was sent
• After creating something, you must log the operation before moving to the next task.
• Or any complex logic that require a certain order.

Expected usage (draft)

As a consumer, I'd like to set up the code like this, allowing the declaration code to be written only once; but also allowing multiple declarations to support all use cases:

private readonly _operationRegister = new MoqOperationRegister();
//...
_someMock
    .SetupWithMonitoring(operationRegister, x => x.SomeCall(It.IsAny<string>())
    .Returns(true);
_someOtherMock
    .SetupWithMonitoring(operationRegister, x => x.SomeOtherCall());
//...

Then in the tests, I'd like something like this to verify the outcome:

// Arrange
var operationMonitor = _operationRegister .CreateMonitor();
//...

// Act
Act();

// Assert
Assert.Collection(
    operationMonitor.Operations,
    operation => Should_do_something_clever(
        operation, 
        expectedParameters: new { paramName = "some string" }, 
        expectedResult: true
    ),
    operation => Should_do_something_even_more_clever(operation)
);

The MoqOperationRegister instance should not be shared between tests run in parallel execution. The idea here is only to save time by declaring all the monitored operations once.

In the previous code sample, operation would be an instance of a class similar to this:

public class MonitoredOperation
{
    public MonitoredOperation(string name, dynamic value = null)
    {
        Name = name ?? throw new ArgumentNullException(nameof(name));
        Time = DateTime.UtcNow;
        Value = value;
    }

    public string Name { get; }
    public DateTime Time { get; }
    public dynamic Value { get; }
}

• Name is the operation name; this could be converted to something else than a string.
• Time is the execution time, allowing to monitor execution time as well.
• Value is a dynamic object holding the arguments as properties, allowing to test input as well as output and execution order.

See if it is possible to implement a non generic IFilterableTableStorageReader that support ReadAsync<TModel>(...) instead of the class level generic TModel.

Possible problems

Querying on multiple tables.

The ITableStorageSettings is usually injected in the class constructor; See how to solve this.

Add and implement a ITableStorageRepositoryFactory to help lower the number of dependencies injected in classes.

Early implementation/idea:

public interface ITableStorageRepositoryFactory
{
    ITableStorageRepository<TModel> CreateRepository<TModel>() where TModel : class, ITableEntity, new();
    IFilterableTableStorageReader<TModel> CreateReader<TModel>() where TModel : class, ITableEntity, new();
}

public class TableStorageRepositoryFactory : ITableStorageRepositoryFactory
{
    private readonly IServiceProvider _serviceProvider;

    public TableStorageRepositoryFactory(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider));
    }

    public ITableStorageRepository<TModel> CreateRepository<TModel>()
        where TModel : class, ITableEntity, new()
    {
        return _serviceProvider.GetService<ITableStorageRepository<TModel>>();
    }

    public IFilterableTableStorageReader<TModel> CreateReader<TModel>()
        where TModel : class, ITableEntity, new()
    {
        return _serviceProvider.GetService<IFilterableTableStorageReader<TModel>>();
    }
}

Describe the bug
When referencing ForEvolve.App the rasterize.js file (from PDF) seems to be copying itself at the project root and in the PhantomJs/Root/ directory.

To Reproduce
Steps to reproduce the behavior:

1. Create a project
2. Import the ForEvolve.App package
3. Look at the file tree (in the solution explorer)

Expected behavior
rasterize.js should only be present in the PhantomJs/Root/ directory.

Additional context
Version: ForEvolve.*-RC-1

Create TableQuery helpers like StartWith.

Maybe something chainable like that: query.Column("ColName").StartWith("String to check"); TBD.

See #9

• Create the filter in ForEvolve.OperationResults.AspNetCore (review this, maybe creating a standalone project would be best?)
• Allow the configuration of ExceptionTransformer that could map specific exception type(s) to an IOperationResult allowing customization of the exception convention pipeline. This could be based on, use, or update the concepts of IOperationResultStandardizer.
• The ExceptionTransformer should support returning any type, no just IOperationResult, making this compatible with any system (including, but not limited to, my old DynamicInternalServerError projcet).
• Add the filter to the AddForEvolveOperationResultFilters() extension method or create an AddForEvolveOperationResultApiFilters() method to differentiate API filters vs MVC filters (or both); TODO: review/rename these to be more generic and less "OperationResult-centric".

OperationResult-centric stuff could be part of another assembly, following an assembly structure similar to:

• ForEvolve.ExceptionFilters.Abstraction
• ForEvolve.ExceptionFilters.AspNetCore ( or ForEvolve.ExceptionFilters.AspNet)
• ForEvolve.ExceptionFilters.AspNetCore.OperationResults
• ForEvolve.ExceptionFilters.AspNetCore.DynamicInternalServerError (maybe?)
• ForEvolve.ExceptionFilters.MediatR (or ForEvolve.ExceptionFilters.VerticalSlice)
• ForEvolve.ExceptionFilters.MediatR.OperationResults
• ForEvolve.ExceptionFilters.MediatR.DynamicInternalServerError (maybe?)
• ...

Notes to self

• Use branch feature/application-insights
• Validate if the code should live in ForEvolve.Azure or not; based on the dependencies brought by ApplicationInsights.
  • ForEvolve.Azure must target netstandardX.Y, not netcoreappX.Y.
  • ApplicationInsights code could target netcoreappX.Y but should aim at netstandardX.Y.
• Port my transaction-like telemetry system.

TODO

• Complete all v1.0.0 tasks
  • #32
  • #33
• Cleanup ForEvolve.ApplicationInsights
  • Move ForEvolve.ApplicationInsights to ForEvolve.Azure\ApplicationInsights in a new branch feature/application-insights.
  • Delete the code from master.
  • Create an issue about implementing something useful there (like a sub-session system, see other project).
• Add a TraitAttribute to tests that use the Azure Storage Emulator
• Validate that all packages target netstandard2.0
• Create community files in the Toc repo
• Create community files in all repos; reference the Toc repo files for CODE_OF_CONDUCT and CONTRIBUTING; create ISSUE_TEMPLATEs
  • ForEvolve-Framework
  • ForEvolve.AspNetCore.Localization
• Cleanup project files
  • ForEvolve-Framework
  • ForEvolve.AspNetCore.Localization
• Review the ForEvolve.AspNetCore.UserIdFinder directory
• Review the ForEvolve.AspNetCore.Middleware directory
• Review the ForEvolve.AspNetCore.HttpRequestValueFinder directory; atm, the implementation only look into request headers.
  • Update services.TryAddSingleton<IHttpContextAccessor, HttpContextAccessor>(); to
    services.AddHttpContextAccessor() in ForEvolveAspNetCoreStartupExtensions.cs
  • Extract an AddHttpRequestValueFinder() extension method that encapsulate both adding IHttpRequestValueFinder elements and the IHttpContextAccessor; allowing to not load the IHttpContextAccessor if not used (for performance gain). Validate where IHttpContextAccessor is used in the project to se if it is possible.
• Remove the CodeCoveragedependency during release builds
• Update readme and create one readme per project
  • Include a PDF example. (clean and push ForEvolvePdfNuGetExperiment to a new ForEvolve.Samples repo)
  • Update all services.Add* to services.AddForEvolve* when required.
  • Update all services.Configure* to services.ConfigureForEvolve* when required.
  • Review ForEvolve.DynamicInternalServerError and ForEvolve.DynamicInternalServerError.Swagger README
  • Create/review markdown, pdf and xunit readme files.
• Deploy v1 to NuGet (this project packages)
  • Complete PR #28
  • Deploy an RC1 version
  • Run some tests
  • Fix bugs
  • Deploy the v1.0.0 final version

• Extract the ForEvolve.Pdf.PhantomJs.Dependencies project to its own repository to keep this one lighter.
• Update the README file (this repo)
• Update the README file (dependencies repo)
• Create a VSTS build definition that creates the NuGet package
• Create a VSTS release definition that deploys the NuGet packages to both MyGet and nuget.org

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

Here are some notes about the test server development and the XUnit test project.
See #10

Goal

Create an easy to use, modular, programmable, per test (or per test class), in-memory mock server.

Ideas

• Move the code away from the XUnit project and create its own project. Target a later version (v1.1.0?).
• Create a non-framework specific project for shared code and extract all non-XUnit code there (if possible without too much effort).
• Keep the XUnit specific code in the XUnit project.
• Depending of the final package separation, create one or more meta-packages; ex.: ForEvolve.TestApp and ForEvolve.TestApp.XUnit.

Update

Update or remove classes like HttpTestServerBuilder and HttpTestServer and use the new asp.net 2.1 Microsoft.AspNetCore.Mvc.Testing instead as a fondation.

Check if it is possible to support both .net core 2.0 and 2.1 without too much effort. If it isnt possible, target 2.1 and implement support for 2.0 later.

Todo

Split the remaining elements of this issue after cleaning up #10

Allow easy testing of IOperationResult with default failure message.

Examples:

result.AssertSuccess(); // Failure message: The operation should be sucessful.
result.AssertFailure(); // Failure: The operation should not be sucessful.
result.AssertFailure(
    error => Assert.Equal("Some expected error message 1", error.Message),
    error => Assert.Equal("Some expected error message 2", error.Message),
    error => Assert.Equal("Some expected error message ...", error.Message),
    error => Assert.Equal("Some expected error message N", error.Message)
);
// Failure: The operation should not be sucessful.
// Collection failure: report (do not handle) Assert.Collection() error

When doing:

var x = new EmailOptions();

x.Smtp should not be null.

Ideas:

• Add a method or an extension method to IOperationResult like string GetMessage() or IEnumerable<string> GetMessages() to ease the access to messages.
• Add a string Message { get; } property on IMessage that link to the appropriate dictionary key; this must not be serialized.
• Other?

Implemented diverse type transformation methods and extensions instead.

For example, find ans use a IMessage of the MyBadMessage type:

if (!result.Succeeded && result.Messages.Contains<MyBadMessage>())
{
    // Do something with that!
    MyBadMessage message = result.Messages.GetSingle<MyBadMessage>();
    if (message.SomeProp == "Toto!")
    {
        // ...
    }
    // ...
}

Or, find all ArgumentNullException:

 IEnumerable<ArgumentNullException> exceptions = result.Messages.GetExceptionsOfType<ArgumentNullException>();
// Do something with "exceptions"