(civicrm-setup/1) CRM_Core_I18n - Loosen coupling to DB layer
authorTim Otten <totten@civicrm.org>
Tue, 20 Feb 2018 22:27:31 +0000 (14:27 -0800)
committerTim Otten <totten@civicrm.org>
Tue, 20 Feb 2018 22:58:01 +0000 (14:58 -0800)
commit2c4bba53e056473398a740a8c2c24e3eb6e52754
tree8662490373309e4a46c1fcb932e816a878256c3e
parent159a0a906c2cb86c199ee0f5e57458da95fbe9c8
(civicrm-setup/1) CRM_Core_I18n - Loosen coupling to DB layer

Overview
--------

The `ts()`/`{ts}` feature from `CRM_Core_I18n` has an option to escape output
(e.g.  `{ts escape="sql"}Activities{/ts}`).  However, SQL encoding is subjective
(depending on the connection details). For `{ts}` to support this feature, it
must have a dependency on the DB subsystem (e.g. it eventually calls `CRM_Core_DAO`).

For https://github.com/civicrm/civicrm-setup/issues/1, we have an issue
where expressions like `{ts escape="sql"}Activities{/ts}` fail during
installation because `CRM_Core_DAO` is not fully available.

This change loosens the coupling, so that we can use `{ts escape="sql"}Activities{/ts}`
without needing `CRM_Core_DAO` per se.

Before
------

`ts`/`CRM_Core_I18n` is tightly coupled to `CRM_Core_DAO`. There is no way
to use `{ts escape=sql}Activities{/ts}` without spinning-up `CRM_Core_DAO`.

After
-----

`ts`/`CRM_Core_I18n` *defaults* to calling `CRM_Core_DAO`.  However, this can
be overriden by manipulating a property.

Comments
--------

* I feel a little dirty keeping any coupling between i18n and the DB.
  However, changing this would mean removing support for the
  `{ts escape=sql}` option, and that would be a clear compatibility-break.
* Arguably, there may be a microsecond-level penalty in using
  `call_user_func($SQL_ESCAPER)` rather than a specific class/function.
  However, it's only incurred if you actually call `{ts escape=sql}`
  while setting `$SQL_ESCAPER`, and that's pretty rare circumstance.
  The typical runtime use-cases for `ts()` are unaffected.
CRM/Core/I18n.php