ava-innersource / Liquid-Application-Framework-1.0-deprecated

Liquid is a framework to speed up the development of microservices
MIT License
25 stars 14 forks source link

Remove or recreate the FileDB class #95

Open guilhermeluizsp opened 4 years ago

guilhermeluizsp commented 4 years ago

The FileDB class is a poor man's implementation of a database. Each entity is stored in its own Json file and the methods to retreive those entities are completely unoptimized.

Take a look at this method:

https://github.com/Avanade/Liquid-Application-Framework/blob/6997cddea3cca6af8c45d998a6d4713d63cde865/src/Liquid.OnPre/Databases/FileDB.cs#L113-L135

It reads the contents of every file within a folder (which entities are stored into). There's no cache nor asynchronous methods involved in this process.

There are crucial functions that depends on this implementation:

https://github.com/Avanade/Liquid-Application-Framework/blob/6997cddea3cca6af8c45d998a6d4713d63cde865/src/Liquid.OnPre/Databases/FileDB.cs#L178-L212

This could result in a horrendous performance bottleneck to the application.

Also, there's a lot of methods throwing NotImplementedException, which is a red flag, indicating that, in fact, it is not a correct database implementation.

https://github.com/Avanade/Liquid-Application-Framework/blob/6997cddea3cca6af8c45d998a6d4713d63cde865/src/Liquid.OnPre/Databases/FileDB.cs#L222-L484

Let's not reinvent the wheel. There are plenty of database implementations such as SQLite, LiteDB and others. Either we should recreate this class using one of them, or completely remove it, since it doesn't bring us any benefits.

bruno-brant commented 4 years ago

I think the greater discussion here is whether we need OnPre or not. I don't think it's necessary, and the current implementation is, as we can see from this and other issues, lacking.

We can start this discussion on a separate thread, what do you think @guilhermeluizsp?

guilhermeluizsp commented 4 years ago

@bruno-brant I have a similar view and agree that we should dive into a deeper discussion on a separate thread.

bruno-brant commented 4 years ago

For now, I'm voting we remove all classes that have no implementation at all. We should create issues to track the actual development of those classes.