Without these qualifiers, the tables returned by the metadata object
will include tables from _all_ databases in the database instance, which
is definitely not something we want. Not only does this pose a potential
performance impact, it can also return very false positives, which then
result in an (inaptly named) SQLSyntaxErrorException when the subsequent
query to the potentially non-existent table fails.
To reproduce the issue:
- Create two new, empty databases in a fresh MySQL/MariaDB instance.
- Point the extension at one of them.
- Spin up the server, let the extension initialize the database.
- Stop the server.
- Point the extension at the other database.
- Spin the server back up and watch it crash.
By qualifying the `getTables()` call with the catalog and schema of the
underlying connection object, we limit the search to _just_ that catalog
and schema, which is what we wanted all along. According to the method
documentation, the args are _patterns_, where `%` matches substrings and
`_` matches "any character". So by being hyper-specific, we shouldn't
get any more false positives. In testing on MySQL and MariaDB, only the
`catalog` value is non-null (matches the database name), while `schema`
remains null.
Spectators produce leave events, but they won't have a join event that
matches the leave event they produce. This is because spectators either
joined _as_ spectators (spectating doesn't produce join events), or they
_transitioned_ into the spectator state after _dying_ in the session (in
which case they already produced a death event that "cements" the player
session).
This fixes an edge case problem that occurs when a spectator produces a
leave event while exactly one player is currently in the lobby. In this
case, the leave event triggers a session deletion, since the leave event
logic "thinks" the leave event is triggered by the player in the lobby.
It would be possible to prevent that problem with additional checks in
the leave event logic, but the underlying problem really is that leave
events produced by spectators shouldn't be recorded or reacted to in any
way, shape, or form, as they are not relevant to the stats (leave times
are only really relevant if the leave event happens _during_ a session,
which this change does not affect).
While we do want to keep track of join and ready times, we really don't
care about those stats for players who join the lobby _and leave_ prior
to the arena actually starting.
Coincidentally, _not_ storing stats for players that don't actually pick
a class means we don't end up trying to insert a `null` value into the
class name column, which should solve the error thrown when trying to do
just that as a result of the auto-start timer kicking players or players
themselves leaving prior to starting.
Fixes#16
Apparently there's a security issue <= 8.0.27. It doesn't really matter
all that much here since we're in the `provided` scope, but better safe
than sorry.
This is just a single sunshine scenario test because that's all the
command supports right now. Issues #2 and #3 highlight some of the
problems with this command, and when those issues are tackled, tests
should be introduced along with the fixes.
Not a lot of variation in these commands, but the output is a little
difficult to write robust tests for because it's all stringly typed.
These tests will break pretty hard (or require lots of adapting) if we
introduce i18n.
The tests don't cover tab completion. That will have to come later.
These are pretty much whitebox tests, so they don't really provide any
improvements to the architecture, but they do add a level of confidence
in terms of refactoring the somewhat unorthodox import/export structure.
This commit makes use of the `register(Command)` method in MobArena's
command handler to pre-instantiate the command classes and inject the
main plugin instance in their constructors. This allows us to get rid of
the horrible Singleton anti-pattern.
Closes#6
This commit adds to the main plugin class a tiny event listener that
listens for MobArena's post-reload event and responds by calling the
internal reload method. This gives us a super cheap way to support
reloads while giving server owners one fewer commands to worry about.
Closes#11
This allows us to make use of the new upstream functionality before a
new named release is ready. This particular hash contains both the new
reload events as well as the new command registration overload.
Introduces the bStats metrics library to the plugin. This first commit
contains just a single custom chart, the `store_type`, which will help
figure out which stores to focus on for performance optimization and
such.
Changes the command output in the query commands to the same approach
used in the import/export and delete commands for better consistency
(and arguably prettier output).
It could probably still do with some fine tuning or maybe custmization
options, but that's out of scope for this particular issue.
Closes#1