Skip to content

Migrate to gobuffalo/pop #1730

@aeneasr

Description

@aeneasr

Is your feature request related to a problem? Please describe.

We're currently using sqlx with a lot of custom queries. This is problematic because the code is very verbose. For example, we have lots of helper structs such as this one whose only purpose is to provide arguments for NamedExec which in turn alows us to do

https://github.com/ory/hydra/blob/master/client/manager_sql.go#L346

instead of manually going over all the fields and putting them in a map[string]interface{} for example. For more info on sqlx check here: https://github.com/jmoiron/sqlx#usage

Unfortunately, sqlx does not work with INSERT or UPDATE statements which is why we have things like this ( https://github.com/ory/hydra/blob/master/consent/sql_helper.go#L53 ) which are then being used with some strings.Join to generate the SQL insert and update statements (e.g. https://github.com/ory/hydra/blob/master/consent/manager_sql.go#L368-L369 )

On top of that, our in-memory manager is basically a duplication of the SQL adapter with some locking. We could however use SQLite with an in-memory database instead (as we do in Kratos) which would mean that the consistency checks and everything else work fine and that you can actually run Hydra without any external dependencies for a small test environment based on SQLite.

I hope/know that by switching to gobuffalo/pop, we can reduce the code a lot in verbosity, lines of code, and improve readability and testability.

For that, I would like ORY Hydra to implement the persitence/persister model of ORY Kratos. This also means that we have to move the migrations from the model in Hydra (here, every module is responsible for its own migrations) to one package and also to gobuffalo/fizz.

If we do that, we need to make sure that:

  1. Old migrations do not break. This means that we have to translate the old migration tables (e.g. https://github.com/ory/hydra/blob/master/consent/manager_sql.go#L55) into something that fizz understands.
  2. If applied with new migrations, the resulting schema is 1:1 the same for all databases

Additional context

This is a lot of work. I also think that we can refactor the tests a bit to make them more readable. But first, we should use pop. Use the same tests (except for maybe how they are being set up, but the contents should be the same), and in a next phase refactor the tests too.

Metadata

Metadata

Assignees

Labels

featNew feature or request.help wantedWe are looking for help on this one.

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions