search

neo-project / neo-modules

MIT License
60 stars 100 forks source link

Add Rest Web Server `WIP` #810

Closed cschuchardt88 closed 12 months ago

cschuchardt88 commented 1 year ago

Adding Restful services to Neo N3 ecosystem. This uses MVC model without the view aspect. I currently don't know how much easier I can make this plugin. When it comes to flexibility and freedom. This system was design with usability. To allow the freedom of making what you want with plugins and will have a fault tolerant model. Also this model can allow upgrades, if Neo ever decides to make neo-cli have a built in webserver that handles RPC, Rest, WSS; this would be the code to do so. The already built plugins that are using this RestServer Plugin would not need to be changed.

This plugin is test ready. I'm not trying to rush it and understand it takes a lot of time. But I would love some feedback on this plugin. If you decide to not use this plugin. By all means this plugin will not end here.

The goal here is to have its client be dependency free of the Neo.dll and all its dependencies. Of course you can still add them if need be. Like Neo.vm.dll for building scripts to send via Rest service call. I want to have this client and this plugins endpoints be available in other parts of the dotnet ecosystem. Just to name a few Mono.Net, .NET Maui and Unity. This will help the Neo user base have no limitations on whatever framework or technology they desire to use.

As for maintainability, I'm not going anywhere; anytime soon and I have tons of time on my hands to write document, answer questions and update code. You have to start somewhere; so let's come together!

Yes I know it seemed fast; for me to make this plugin, But I really started at the begin of the year working on it and learning the neo-cli. 😸 So this is 5 months worth of learning and reading source code. 3 months of programming the code. My background is architecture design of systems.

See https://github.com/neo-project/neo/issues/1004

Documentation

This RestServer Documentation explains the working of the RestServer plugin. Note still a work in progress.

RestServer Addon Plugins This Example Plugin Documentation explains how to make plugins for the RestServer.

Downloads

Swagger UI

RestClient Code Example

using RestClient;

using var httpClient = new HttpClient();
var restClient = new RestWebClient("http://127.0.0.1:10339/", httpClient);

try
{
    var block = await restClient.GetBlockAsync(0).ConfigureAwait(false);
    Console.WriteLine("Block Hash: {0}", block.Hash);
}
catch (ApiException<ErrorModel> ex)
{
    Console.WriteLine(ex.Result.Message);
}
Console.WriteLine("Please ENTER to exit.");
Console.ReadLine();

Key Features

Change Log

Testing Progress

Neo-Cli Commands

image

You can just add Swagger OpenAPI 2.0 to any project by

Step1

image

Step 2

image

Step 3

image
cschuchardt88 commented 1 year ago

Before i make client can someone make sure we are going to use this?

vncoelho commented 1 year ago

@cschuchardt88, It is a mechanism that many developers requested to us in the past, as far as I can remember. In my opinion, many people would use it. I would like to hear as well from other developers from the Python team that already did something similar in the past: @ixje @lllwvlvwlll

One point that worries me a little bit is about the maintenance and security of some calls, how could we make it more generic related to the current RPC Server plugin or RPC Client? That would gives us less burden for keeping this plugin up to date.

shargon commented 1 year ago

Swagger can be added to the list

cschuchardt88 commented 1 year ago

The RestServer implements the same things for Kestrel as RpcServer does. Plus newer features. Everything is configurable and can be turned off. For added security you can enable basic authentication and/or https, with the ability to use a forward proxy. Also you have the ability to disable controllers and by default we disable WalletController. So if anyone want to make this plugin be public on a public node. We don't want people trying to open wallets or search around on the node for one. This uses middleware to block the controllers from ever being loaded. "DisableControllers": [ "WalletController" ]. Trying to access disabled controller routes with end in a 404 response from the server.

This is very generic but may appear not to be. Most the code is Model/Json converters for serializing the Models to JSON and JSON to Objects. These include UInt160, UInt256, StackItem, Transactions and Blocks to just name a few. If the application breaks I have Middleware set to not leak sensitive information to the end user. Server will return generic responses like see below. In the event of a crash of the application from any controller will result in a generic exception json response. To ensure applications that are consuming this RestServer will not have inconsistent responses for crashes or errors. Also any exception that is thrown will also result in error response like see below. So instead of returning your own response in a plugin for example with your own error. You would just throw an exception and the result will be 400 response with see below as example

{
    "code": 1001,
    "name": "ParameterFormatException",
    "message": "Parameter hash is in an invalid format."
}

if you have a look at theses files RestServerPlugin.cs, RestServerSettings.cs and RestWebServer.cs they can tell the whole story of what's going on and give you the idea of what is happening and the working of the application.

@shargon yes I can add it.

Edit: I added swagger to the RestServer per @shargon

cschuchardt88 commented 1 year ago

Swagger can be added to the list

Is this what you were looking for? See Attachment of results. I don't use swagger, but i have implemented it for you. I have implemented it in the pass before, but just don't like using it. Swagger Test.pdf

Edit: also do you care if i use xunit testing framework instead of microsofts. Also would like a fully featured wallet api?

cschuchardt88 commented 1 year ago

@vncoelho currently the state of plugin is sound. The burden would add to new features like something in Kestrel server Or even maybe change swagger settings (which can be an easy task). Other than that the plugins get automatically loaded and no reference needs to be referenced back to this assembly. All plugins are independent from one another. Top post i posted has a link to my repo with an example of a "dummy example plugin" for adding to this Rest server.

I designed this plugin to have no restrictions for other plugins. To allow them do what they want. For example Route, Route parameters, parameters type, erroring, throwing exceptions and more. the code uses generic error middleware for responses. So other client that program against this api won't crash or get something unexpected results. Like an developer exception page. All default settings for RestServer have been removed and i set everything manually. If you like reading code have a look at RestWebServer.cs. Reusability was thought of in the design pattern of this plugin. No dependency injection is required as well.

WorkFlow The workflow is a simple one, it follows the premise of MVC without the views. Just simple "API Controllers". You also have the ability to disable controllers and everything is configurable. No dependency reference to this assembly for plugins or soon to come RestClient. So users can easily add RestClient into other dotnet frameworks like dotnet Maui and Unity. RestClient will not require dependency on Neo.dll. Everything is server side.

Need more information or if i wasn't clear enough let me know.

cschuchardt88 commented 1 year ago

Typing up all the service descriptions for each controller for Swagger

image

@shargon is this how you were envisioning it? Also you do know each plugin would have to do this and update the RestServer.xml file for Swagger right? If you don't already know, Swagger uses code comments to do so, unless you use Swagger's Annotations.

Edit: For the xml file I think I have it figured out. For other plugins.

cschuchardt88 commented 1 year ago

RestClient is here at my Repo. Reason being is that I tried adding the client to this pull request but the build props won't let you remove the neo.dll package.

The reason is that we want a DotNet Standard 2.1 client for support in Unity, Xamarin, Mono and all other dotnet frameworks.

cschuchardt88 commented 1 year ago

This is Ready for testing. Check top post for RestServer Plugin binaries (Only has necessary files).

Just need input on the Swagger endpoint descriptions. For Example when you get the exception It will show error "If anything is invalid or request crashes." but you are able to get the real error see example in Top Post of RestClient Example. It's just the way Open API client works in visual studio, so we need to update that and all other output messages of any kind.

cschuchardt88 commented 1 year ago

@shargon Swagger has limitations with polymorphism classes. There is no workarounds for this problem. Here is the problem that i am having. The document that is create by swagger and the client don't know how to make classes abstract. One example of this is StackItem's and Signer's class see below. What the client does is generate inheritance of the classes and doesn't mark them as abstract class or abstract properties. In the case of the example below it will think that the return is ONLY AndConditionModel and this is no way to fix this currently due to the specification of Open API. Making a custom client defeats the purpose of using Swagger. So i removed swagger and started making another RestServer version that doesn't use swagger.

Example

public abstract class WitnessConditionModel
{
    public abstract WitnessConditionType Type { get; }
}

public class AndConditionModel : WitnessConditionModel
{
    public WitnessConditionModel[] Expressions { get; set; }
    public override WitnessConditionType Type => WitnessConditionType.And;
}

public class OrConditionModel : WitnessConditionModel
{
    public WitnessConditionModel[] Expressions { get; set; }
    public override WitnessConditionType Type => WitnessConditionType.Or;
}

public class GroupConditionModel : WitnessConditionModel
{
    public string Group { get; set; }
    public override WitnessConditionType Type => WitnessConditionType.Group;
}
shargon commented 1 year ago

You can use options.UseOneOfForPolymorphism();

cschuchardt88 commented 1 year ago

@shargon there is currently a bug https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/2702 with it and i did use a workaround 

options.UseOneOfForPolymorphism();
options.SelectSubTypesUsing(baseType =>
{
    if (baseType == typeof(WitnessConditionModel))
    {
        return new[]
        {
                typeof(AndConditionModel),
                typeof(OrConditionModel),
                typeof(GroupConditionModel),
                // ...
            };
    }

    return Enumerable.Empty<Type>();
});

But the problem is the Open API client won't generate the right classes as abstract or properties as abstract because it doesn't know. Because the spec doesn't say. Now we can customize the classes to abstract but that defeats the purpose.

Edit: i researched this for a day, but all i could come up with is there is a limitation with polymorphism and abstract classes

cschuchardt88 commented 1 year ago

@shargon Open API will generates classes like this see below and only have one return type of the class, in this case OrConditionModel is the only one returned by the function out of all the Conditions see below. But the classes are partial and can be edited. we can do that with our client, but other end users importing this swagger.json file won't know. We might get spam with issues about it.

WitnessConditionModel andcon = await restClient.TestAsync().ConfigureAwait(false);
OrConditionModel orcon = (OrConditionModel)andcon;
[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "13.20.0.0 (NJsonSchema v10.9.0.0 (Newtonsoft.Json v13.0.0.0))")]
public partial class WitnessConditionModel
{
    [Newtonsoft.Json.JsonProperty("type", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
    public WitnessConditionType Type { get; set; }

}
[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "13.20.0.0 (NJsonSchema v10.9.0.0 (Newtonsoft.Json v13.0.0.0))")]
public partial class OrConditionModel : WitnessConditionModel
{
    [Newtonsoft.Json.JsonProperty("expressions", Required = Newtonsoft.Json.Required.Default, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
    public System.Collections.Generic.ICollection<AndConditionModel> Expressions { get; set; }

    [Newtonsoft.Json.JsonProperty("type", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
    public WitnessConditionType Type { get; set; }

}

image

shargon commented 1 year ago

@shargon Open API will generates classes like this see below and only have one return type of the class, in this case OrConditionModel is the only one returned by the function out of all the Conditions see below. But the classes are partial and can be edited. we can do that with our client, but other end users importing this swagger.json file won't know. We might get spam with issues about it.

WitnessConditionModel andcon = await restClient.TestAsync().ConfigureAwait(false);
OrConditionModel orcon = (OrConditionModel)andcon;
[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "13.20.0.0 (NJsonSchema v10.9.0.0 (Newtonsoft.Json v13.0.0.0))")]
public partial class WitnessConditionModel
{
    [Newtonsoft.Json.JsonProperty("type", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
    public WitnessConditionType Type { get; set; }

}
[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "13.20.0.0 (NJsonSchema v10.9.0.0 (Newtonsoft.Json v13.0.0.0))")]
public partial class OrConditionModel : WitnessConditionModel
{
    [Newtonsoft.Json.JsonProperty("expressions", Required = Newtonsoft.Json.Required.Default, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
    public System.Collections.Generic.ICollection<AndConditionModel> Expressions { get; set; }

    [Newtonsoft.Json.JsonProperty("type", Required = Newtonsoft.Json.Required.DisallowNull, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
    public WitnessConditionType Type { get; set; }

}

It's strange that only one class appears, but I think that is openApi problem, it's well defined in the json

cschuchardt88 commented 1 year ago

I did make an issue about it on their github https://github.com/RicoSuter/NSwag/issues/4506#issue-1876376051

Jim8y commented 1 year ago

What a beautiful and elegent coding style.

cschuchardt88 commented 1 year ago

This is at the point where you can try it out and see if it meets your requirements. Everything is up for discussion. If you can't use it either that's fine too.

Some questions i have:

  1. Is it something you can use?
  2. Is it something you can maintain?
  3. Missing any features?
  4. What will it take to get moving forward in a release?

But i do think this is a good start to get it moving out the door for merger.

Jim8y commented 1 year ago

too long to review every single line, but functioning ok, maybe we can have it at first, then optimize it little by little. I personally love and welcome new features added.

Jim8y commented 12 months ago

@cschuchardt88 do you mind to provide a readme for this plugin? i know others dont have it, but we will add readme for every plugin in the future. Mainly about what it is, what it does, how to use it, extra. basically what you have in the description of this pr

Jim8y commented 12 months ago

Merge since its a plugin, people dont install it if they dont like it.

roman-khimov commented 12 months ago

@Liaojinghui, you're rushing too fast with this, IMO. People will install it, people will try building things on it. I'm not sure it's tested enough and stable enough for them to do that. And notice that it could've been a separate plugin for some time exactly for these experiments. Now it's an official plugin from the Neo project, people have higher expectations of those.

cschuchardt88 commented 12 months ago

@roman-khimov I use it all the time. So It pretty stable. Its maybe lacking features. But all the basic and some advanced are all there.

shargon commented 12 months ago

@Liaojinghui, you're rushing too fast with this, IMO. People will install it, people will try building things on it. I'm not sure it's tested enough and stable enough for them to do that. And notice that it could've been a separate plugin for some time exactly for these experiments. Now it's an official plugin from the Neo project, people have higher expectations of those.

Agree and reverted (https://github.com/neo-project/neo-modules/commit/83372022c984c71ddbb2b0df64a83e16c6cfc1dc). Too many files without a deep review.

Jim8y commented 12 months ago

@Liaojinghui, you're rushing too fast with this, IMO. People will install it, people will try building things on it. I'm not sure it's tested enough and stable enough for them to do that. And notice that it could've been a separate plugin for some time exactly for these experiments. Now it's an official plugin from the Neo project, people have higher expectations of those.

Agree, but only if someone review it. I dont see how this can introduce more bugs than there has already be. I dont understand why you still stop merge and expect some one to review it while a pr has being there for 3 months and no one to review. I have checked the code and run it, no abvious issues found.

shargon commented 12 months ago

@roman-khimov I use it all the time. So It pretty stable. Its maybe lacking features. But all the basic and some advanced are all there.

Agree, but only if someone review it. I dont see how this can introduce more bugs than there has already be. I dont understand why you still stop merge and expect some one to review it while a pr has being there for 3 months and no one to review.

I understand, but understand that you have to review the code before merging, the more changes, the more complicated and time-consuming it requires, imagine (which I don't think is the case) that you add a line to leak the private key, all the code must be reviewed by at least two core developers, and tested by the great @superboyiii team.

Small pr will be reviewed faster than longer ones.

Jim8y commented 12 months ago

Yes, the same reason why so many prs stay there for many years without anyone care about them.

shargon commented 12 months ago

Yes, the same reason why so many prs stay there for many years without anyone care about them.

The solution is not merge all of them.

shargon commented 12 months ago

@cschuchardt88 please open a new PR with these changes.

Jim8y commented 12 months ago

Not all of them, but something i have reviewed, and no one else review it after months. We can wait, but should not be endless waiting.

roman-khimov commented 12 months ago

That's why roadmaps and plans matter. Set some target, like "this PR should be dealt with by this time" and work towards that goal. Not planned? Likely not critical for now then (sure, life is complicated and sometimes it is). Add it to some milestone in the future, but keep real concentration on the nearest goals. We can't solve all problems at once and we can't risk destabilizing the system by accepting everything without proper reviews and tests. Going one problem after another with intermediate stable releases is more viable.