Branch 0.3 #2
Merged
Branch 0.3 #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sadly, we're stuck with PHPUNIT 7.5.20 because of WP’s test suite 😢
- In `DBUtilities::sanitize_table_name()`, allow the table name `0`. - In `DBUtilities::quote_string()`, don't escape simple quotes: in `DBUtilities::prepare_values_list()`, quote escaping is done by `esc_sql()`, so it would result in double escape. Added warnings in the DocBlocks. Also, made few adjustments to make the code testable.
Moved the log method from `Fixtures\src\DBUtilities::local_log()` to `Unit\src\DBUtilities\TestCase->log()`, which makes more sense.
- The table name must be wrapped in single quotes, not ticks. - WP's test suite doesn't allow to create "normal" DB tables, only temporary ones, which cannot be detected like normal ones. Another way has been added for this case.
… it will ba handled elsewhere Also, added a bunch of `$wpdb->hide_errors();` in all relevant methods.
Also renamed the fixture class into `DBUtilitiesUnit`, as we will also need one for integration tests.
This is to better cope with hidden errors.
- Use `DBUtilities::class` as default value instead of a hardcoded string. - Remove the heading slash from the non-default value of `$table_worker`, for concistancy with `::class`. - In `clone_to()` and `copy_to()`, test the sanitized table name against `null`: this allows for `'0'` table name.
This tests the case when a string is passed as "worker" with a heading backslash.
It regroups all the code used to create and manipulate temporary tables during the integration tests.
It regroups all the code used to log messages during the integration tests.
So it can be used in unit tests.
Added `empty_mocks()` and `set_mocks()`
This provides a way to set the "table worker" via the constructor. It was possible to set it via class inheritance until now, but this provides more flexibility and is consistent with the class `Table`.
- Moved some code from the constructor to `init()`. - Reversed logic in `table_is_up_to_date()` when downgrade is supported. - Removed unused `$wpdb` in `maybe_upgrade_table()`.
- New method `invokeMethod()`. - Replaced calls to `new ReflectionClass()` by `new ReflectionMethod()` and `new ReflectionProperty()` directly.
- Mainly, reorder arguments to something that makes more sense to me: `$class`, `$property`|`$method`, `$value`. - Harmonize DocBlocks a bit. - Fix `set_reflective_property()` to work with static properties (like `setPropertyValue()`).
This allows to pass arguments to the invoked method. This must be an array of arguments.
- Removed `@uses esc_sql()` from the class DocBlock. - In `prepare_select_for_query()`, wrap column names in backticks after trimming any space and quote characters. Also remove empty column names. - In `get_placeholders()`, don't return columns that don't have a placeholder. - In `cast()`, cast numeric values as strings when the placeholder is not `%d` nor `%f`, nor a serializable column. - In `serialize_columns()`, removed an unused variable.
`get_table_definition()` is the olnly method tested: - It's the only public method. - All the other protected method already have unit tests. - All the other proptected methods are used in the class `CRUD`. The exception is `cast_col()`, which is used nowhere.
This value will remove field names from the results, which will prevent `cast_row()` to work. Also improved DocBlock for `Basic->replace()`.
This will disappear soon anyway.
The static class `DBUtilities` is now the instanciable class `DBWorker\Worker` that implements `DBWorker\WorkerInterface`. Seeing how `DBUtilities` was used at this point, using a static class was not shorter. The advantage now is that the instance can be typehinted against the interface. Although, it is not typehinted directly in `AbstractTableDefinition` today, we need php 7.1 for nullable typehints (this is for a future release), but `instanceof` is used for the time being. The full instanciation is now `new Table( new MyTableDefinition( new MyWorker() ) )`. `AbstractTableDefinition` falls back to `new DBWorker\Worker()` if no custom worker is specified as argument.
Also, added markdownlint config rules in a `.markdownlint.json` file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tests
Unit and integration tests have been added, they cover 100% of the current code.
AbstractCRUD
prepare_select_for_query():esc_sql()anymore.get_placeholders():cast():%dnor%fand the column is not serializable, return numeric values as string.Basic
get():ARRAY_Nvalue for the output type, as it conflicts withcast_row().AbstractTableDefinition
Workercan be injected into the constructor.DBUtilities / DBWorker\Worker
DBUtilitiesis now the instanciable classDBWorker\Workerthat implementsDBWorker\WorkerInterface.Seeing how
DBUtilitieswas used at this point, using a static class was not shorter. The advantage now is that the instance can be typehinted against the interface. Although, it is not typehinted directly inAbstractTableDefinitiontoday, we need php 7.1 for nullable typehints (this is for a future release), butinstanceofis used for the time being.The full instanciation is now
new Table( new MyTableDefinition( new MyWorker() ) ).AbstractTableDefinitionfalls back tonew DBWorker\Worker()if no custom worker is specified as argument.table_exists():get_last_error(), which returns the last$wpdberror.sanitize_table_name():0as a valid table name.quote_string():esc_sql()inprepare_values_list().can_log():screenfeed_autowpdb_can_log.Table
get_last_error(), which returns the last DB error.clone_to():0as a valid table name.TableUpgrader
staticinstead ofself, so the class can be extended and tested.init():init()method.false.table_is_up_to_date():