lewster32 / corporallancot

A Discord bot primarily for recording, searching and retrieving notes and quotes
MIT License
1 stars 2 forks source link

DB Abstraction improvement #19

Closed Bidthedog closed 4 years ago

Bidthedog commented 4 years ago

More changes to come - opened PR for visibility.

Bidthedog commented 4 years ago

@lewster32 I haven't yet decided on how DbAdapter classes will execute the SQL - ideally we should keep all SQL / DSL syntax to the MariaDbAdapter class, but the methods I've implemented so far (execute(), read(), delete() etc) would essentially make it a type of ORM, making the potential parameter combinations infinite.

An alternative solution is to keep all SQL in MariaDbAdapter, but have specific methods (insertNote(), getRandomNote() etc) as simple pass-through calls from NotesActionPersistenceHandler. I think this is a good solution, as it keeps the SQL abstracted away from the NotesActionPersistenceHandler. The benefit of this is that we can strip out the persistence later and replace it with anything else by creating a new class that inherits from dbAdapter, then modifying one file - container.js.

Bidthedog commented 4 years ago

@lewster32 I've now implemented a repository type pattern for the action persistence handlers. I really wanted to move the persistence DSL (i.e. SQL) into a single class, but the MariaDbAdapter would become too cluttered, so the SQL now exists in the repositories.

I also wanted to avoid standard CRUD based operations in the repositories, so they have very specific method names and parameters.

Bidthedog commented 4 years ago

This is now ready to go - there are some additional things that need to be done, but we can consider #6 and #9 complete, with more work done on #2 (ongoing).

I'd like this to be reviewed and put into the core codebase so we can build from it.

Once the logging is done and the notes are sanitised (both with you @lewster32), I'm happy to push this to my DO box and put v1.0.0 live in the SQS channel.