Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Player serialization from-and to SQL database #496

Open
wants to merge 49 commits into
base: kotlin-experiments
Choose a base branch
from

Conversation

sinoz
Copy link

@sinoz sinoz commented May 29, 2020

Solving #175

sinoz added 30 commits May 29, 2020 22:52
@sinoz sinoz marked this pull request as ready for review May 30, 2020 22:33
@garyttierney garyttierney requested review from Major- and garyttierney and removed request for Major- June 8, 2020 21:31
game/build.gradle Outdated Show resolved Hide resolved
game/src/main/java/org/apollo/game/account/Email.java Outdated Show resolved Hide resolved
FROM postgres:11-alpine

# And then copy over the apollo.sql script into the container's entrypoint
COPY apollo.sql /docker-entrypoint-initdb.d/init.sql
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to start publishing custom PostgreSQL Docker images unless we need to compile native extensions. Instead of using the containers init scripts functionality we should be using something like Flyway to run migrations on the configured database.

database/start-db.sh Outdated Show resolved Hide resolved
}
}

private void putRank(Connection connection, Email email, PrivilegeLevel rank) throws SQLException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the amount of manual interaction with PreparedStatements and data mapping we have to do in here. I'd prefer if we used something like Spring's JDBCTemplate or jOOQ to handle splitting out the data mapping code (into spring RowMappers or jOOQ Converters), common JDBC errors, and replace positional parameters with named parameters.

* The associated queries that are ran by this serializer.
*/
private static final String
GET_ACCOUNT_QUERY = "SELECT password_hash, rank FROM get_account(?::text)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these queries implemented as functions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because majority of them are slightly more complex than the usual one liners involving joins and such. Java doesn't support text blocks until 13 and we're on 11 now so that'd get a bit messy. Do you have a better idea?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the only reason is to cut down on the size of string literals than we can store the queries in a kotlin file (which does support multi-line raw literals). The game module already has the Kotlin compiler enabled so this won't be an issue.

settings.gradle Outdated Show resolved Hide resolved
database/README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,506 @@
CREATE EXTENSION citext;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we've got this schema into something that can manage it (i.e. Flyway) lets break it down and separate out schema primitives (like types and tables) from functions (whose migrations can be run repeatedly without the need to create new versions every time you update the function).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants