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

Remove ADODB #6264

Closed
asmecher opened this issue Oct 2, 2020 · 8 comments
Closed

Remove ADODB #6264

asmecher opened this issue Oct 2, 2020 · 8 comments
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.

Comments

@asmecher
Copy link
Member

asmecher commented Oct 2, 2020

Remove ADODB.

@asmecher
Copy link
Member Author

asmecher commented Oct 2, 2020

PRs:

TODO:

Highlights/implications:

  • Result counts are no longer available automatically when you run a query (e.g. DAOResultFactory->getCount / DAOResultFactory->wasEmpty). It's always been inefficient to both iterate and count, and using Generators (as we will do more in the future) means it's a bad pattern. It's sometimes necessary, though, e.g. for paging.
    • For existing use cases, you can provide the SQL to the DAOResultFactory (e.g. EmailLogDAO::_getByEventType) to allow it to count with a separate query (ADODB used to do this behind the scenes anyway).
    • For future use cases, if counting query results is really needed, try Laravel's Lazy Collections.
    • You will see a DAOResultFactory instances cannot be counted unless supplied in constructor exception if you need to adjust this.
  • Retrieving DB results has changed now that ADODB's functions are removed.
    • The DAOResultFactory is still available and works roughly the same, though it should be considered deprecated.
    • Instead, for multiple results, use either
      • ...the patterns in place in the Service classes (which use iterators), or
      • ...you can experiment with Generators (e.g. AnnouncementDAO::getByTypeId), which can be looped through using foreach but don't instantiate the objects it's fetching until they're needed, or
      • ...experiment with a new pattern with Laravel's Lazy Collections
    • For single results, the getters become really short and sweet (e.g. SubscriptionTypeDAO::getById for an object, and ContextDAO::existsByPath for single fields).
  • The DAO query functions (DAO::retrieve, DAO::retrieveLimit, DAO::retrieveRange, DAO::update) you're used to are still mostly present, but have some slight behavioural changes:
    • If you are only binding a single parameter to a query, you now need to put it in an array, where ADODB would accept either a literal or an array. (See e.g. ContextDAO::getByPath.) You will see Argument 1 passed to Illuminate\Database\Connection::prepareBindings() must be of the type array if you need to adjust this.
    • $dao->update now returns a count of affected rows, whereas before it returned a success/failure boolean that was frequently misused and undocumented. I have not cleaned up all these cases. See e.g. TemporaryFileDAO::deleteTemporaryFileById. Error conditions should use Exceptions rather than null/false returns that frequently go uncaught.
    • DAO::retrieveCached has been removed pending a suitable replacement (probably a different approach than a query-level cache).
    • However, these functions should be considered deprecated in favour of Laravel's query builder tools. You can either use the patterns set in the query builder classes (these will likely be changed soon) or use the Capsule object to construct queries directly in the DAO. See e.g. AnnouncementDAO::getById.
  • The ADODB XML schema management toolset has been removed. This includes the removal of tools/dbXMLtoSQL.php.

asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 2, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 2, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 2, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Oct 2, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Oct 2, 2020
@NateWr NateWr added the Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. label Oct 5, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 6, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Oct 6, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Oct 6, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Oct 6, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 13, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 13, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 13, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 13, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 13, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 13, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 13, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 14, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/ops that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/pkp-lib that referenced this issue Nov 19, 2020
asmecher added a commit to pkp/reviewReport that referenced this issue Nov 19, 2020
asmecher added a commit to pkp/usageStats that referenced this issue Nov 19, 2020
asmecher added a commit to pkp/browse that referenced this issue Nov 19, 2020
asmecher added a commit to pkp/staticPages that referenced this issue Nov 19, 2020
asmecher added a commit that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/ops that referenced this issue Nov 19, 2020
asmecher added a commit to pkp/ops that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/ojs that referenced this issue Nov 19, 2020
asmecher added a commit to pkp/ojs that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/omp that referenced this issue Nov 19, 2020
asmecher added a commit to asmecher/omp that referenced this issue Nov 19, 2020
asmecher added a commit to pkp/omp that referenced this issue Nov 19, 2020
@asmecher
Copy link
Member Author

All PRs merged, not including the testing-purposes-only commits.

@asmecher
Copy link
Member Author

Closing this; there will doubtlessly be related errors but they can either reopen this issue with details or file new entries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Projects
None yet
Development

No branches or pull requests

2 participants