go-fed / apcore

Golang ActivityPub Server Framework
GNU Affero General Public License v3.0
105 stars 10 forks source link

Why tightly coupled to a sql db? #71

Open prologic opened 3 years ago

prologic commented 3 years ago

Why have you made the decision to tightly couple the framework to a SQL database? An implementor not only has to satisfy the interface requirements of app.Application but also Database and Txbuidler.

This makes it quite hard for implementers to satisfy the interface without also doing a lot of "unnecessary" work (I fail to see how SQL has anything to do with ActivityPub).

Are there any future plans to simplify the interface requirements and remove the tight coupling on SQL?

cjslep commented 3 years ago

Great question. You may want https://github.com/go-fed/activity/tree/master/pub instead (github.com/go-fed/activity/pub). Its Database concept is abstracted away from any implemented details, as it was intended to be a purer implementation of ActivityPub in library form.

In contrast, APCore is geared to be a server framework. It is intended to be a quicker way to bootstrap a traditional single-server application from nothing, where developers can focus more on delivering the vocabulary & side effects, web service, and frontend. As such, there are more practical and opinionated decisions that have to be made, and the out-of-the-box database/sql was the lowest-energy choice. I haven't thought about it much since then. Note that perhaps what is needed is better interface separation: app.Application is the only one to be implemented by client code. app.Database and app.Framework, app.Router, etc are all provided to the Client Application, and so app.Database is not a client burden to implement in this case -- but it is for the purer go-fed/activity/pub. Everything that isn't app.Application (and plain structs app.Software + app.Paths) is the APCore API the client should call into in its app.Application implementation.

If you see a way to abstract database/sql and sibling competing practical choices entirely away, while still allowing the core to function in a configuration-driven manner, I'm very much welcome to hear your ideas.

Note that APCore is still under active development without having an initial release, and as such I do not consider it ready for actual use. I am in parallel attempting to be my own client and use it to bootstrap an application. So it is a great time to entertain better ideas.

(Note I've edited this a few times, as I've internalized your inquiry)

prologic commented 3 years ago

All very good points! I suspect as such. 👌

I just really hate SQL database of all kinds really, they're overkill for what you often need, cumbersome to setup and hard to maintain properly.

If we abstract away app.Datdabase to be a set of interfaces for what the rest of the framework needs I think this could be a good compromise. For example:

type ActoerLookuper {
  ActorLookup(id string) (*Actor, error)
}

type Database interface {
  ActoerLookuper
}

Just as a contrived example, I don't know terribly too much about the activity pub vocab/terms yet or the codebase. But this way it allows me as a developer perhaps interested in exploring activitypub to quickly prototype up a server and just implementing the required specific interfaces rather than a whole SQL database.

Does this make sense? And the default implementation can still use SQL under the hood, as it will already satisfy the interfaces if you make them match (more or less). I suspect refactoring will be required somewhat.

prologic commented 3 years ago

In other words I prefer the abstraction to be a store.Store like for example many of my projects such as yarn

cjslep commented 3 years ago

I've been thinking on this the past day. Let me get some thoughts down -- you may have already thought this through, though hopefully it isn't too repetitive.

Expanding on above -- github.com/go-fed/activity/pub has pub.Database as an abstraction for the client to implement. The pub library says "this is the subset of the application database that pub needs". In that way, we can implicitly think of a pub.Database implementation in a real application. It will have pub.Database's list as one set of methods, and {App Database Methods} as implicitly another set of methods, to fulfill the particulars of that application logic. This seems like an odd category to make, but the below leverages both these distinctions.

The apcore framework conceptually says "hey, I'll implement pub.Database so that apcore users can leverage my implementation of it". However, that leaves the problem of how to let the application specify the {App Database Methods} from before. Today, I actually expose this to the application via the apcore.Database, which is a generic way of saying "here Application, do what you want with this generic storage". apcore.Database is provided to the client, in stark contrast of pub. However, internally to apcore there is still an {apcore impl of pub.Database}. That also means that pub.Database and apcore.Database, while similarly named, are conceptually doing very different things.

Today, it happens that {apcore impl of pub.Database} is tied to database/sql, and therefore the database/sql concept leaks out into the apcore.Database. That's what motivates the original inquiry: it would be great if {apcore impl of pub.Database} was not tied to database/sql, and therefore it did not leak into apcore.Database. Plus, no more requirement to run SQL database software.

The question becomes: how to approach this?


A constraint I've imposed on myself is: I would like for the details of "which" and "what kind" of actual database to connect to, to be configuration driven. That made the database/sql pick an obvious first naive choice: the driver package allows me to statically link in all types of sql "for free", and I would just have to theoretically adjust the underlying SQL queries, and then "job done" on supporting N+1 datastores. The actual apcore clients will never need to implement these driver implementations to do the apcore+pub glue, they will continue being provided the apcore.Database interface to use as their generic datastore.

Preserving this constraint means that we need a database/sql-driver relationship, but at a higher level. database/sql itself would conceptually be a "driver", next to non-sql solutions. Plus, we would need a non-sql "driver" actually implemented to make use of these new concepts.


That's about as far as I've gotten in defining the problem in context of existing constraints. I don't have a concrete technical proposal at the moment.

To close, I'd like to revisit your proposed example:

type ActoerLookuper {
  ActorLookup(id string) (*Actor, error)
}

type Database interface {
  ActoerLookuper
}

with the above context, it is clear that this proposed interface is a "driver" -- the glue between pub+apcore -- and not actually the genericized interface provided to apcore clients. While it is likely the first person to use a non-sql implementation will be the one to write the implementation of this interface, that bit won't live in the client code, but in apcore -- only to be executed if configured this way.

Once we've:

  1. Created an abstraction for a "driver" (in the larger-than-sql sense)
  2. Modified app.Database to also be decoupled from SQL concepts.

Then we'd need:

  1. A no-sql version of that "driver" in apcore
  2. A client to use apcore and verify things are working happily. :)

I would love to hear your thoughts on all the above.