MajMcCloud / TelegramBotFramework

This is a context based application framework for the C# TelegramBot library.
https://www.t.me/tgbotbase
MIT License
149 stars 43 forks source link

Add PostgreSQL serializer #58

Closed Kataane closed 7 months ago

Kataane commented 7 months ago

Hi. A few more improvements.

I added serialization for states using PostgreSQL. Since I use this database a lot myself.

I'm eager to hear your thoughts and any suggestions you might have for further improvements.

Best wishes to Kataane.

MajMcCloud commented 7 months ago

Looks good so far. I will have another look at it tomorrow, and then add it into the branch. Did you tested this one with any test project ? (you don't have to provide one) If so, I can publish it as a nuget package tomorrow as well.

Thanks !

Kataane commented 7 months ago

Hi @MajMcCloud.

Yes, I've tested it. I can write unit tests if you need them. Well, and if something breaks, I am ready to help find and fix bugs :).

Regards, Kataane.

Kataane commented 7 months ago

Also, maybe add base initialization when creating PostgreSqlSerializer? Because the solution in the form of a separate file does not look very clear.

Kataane commented 7 months ago

It would be nice to have a single point of getting the name of fields when creating and saving a StateEntry. Something like this:

foreach (var (name, type, value) in StateEntry.GetFields())
{
    sessionCommand.Parameters.Add(new NpgsqlParameter(name, type) {Value = value });
}

Or

// Create a command
insertIntoSessionSql = "INSERT INTO " + TablePrefix + $"devices_sessions ({string.Join(',', StateEntry.GetFields().Select(f => f.Name))}) VALUES ({Enumerable.Range(0, StateEntry.GetFields().Length)})";

 // Execute the command
for (int i = 0; i < StateEntry.GetFields().Length; i++)
{
    var type = StateEntry.GetFields()[i].Type;
    var value = StateEntry.GetFields()[i].Value;
    sessionCommand.Parameters.Add(new NpgsqlParameter(i.ToString(), type) { Value = value });
}

Providing a single point of information about the data being stored.

MajMcCloud commented 7 months ago

@Kataane

Hey thanks for your quick feedback. I would say for now this is fine. Don't need a Unit test right now, we are missing them as well for other stuff haha...

About that single point, let us get back to this later. Thanks for now!

MajMcCloud commented 7 months ago

First nuget package is out!

Thanks mate !

MajMcCloud commented 7 months ago

@Kataane Someone in our telegram group mentioned that the serializer does not work correctly. He said the LoadSessionStates(IStateMachine statemachine) raises a NullReferenceException exception.

Maybe you can have a look. I will check it as well.

PS: maybe you can join https://t.me/tgbotbase

Edit: I checked it and it worked fine. Not sure what his problem is.