[commits] [Wiki] changed: Doc/Dev/ConversionH6
Ralf Lang (B1 Systems GmbH)
lang at b1-systems.de
Sun Oct 11 12:26:47 UTC 2020
rlang Sun, 11 Oct 2020 12:26:47 +0000
Modified page: https://wiki.horde.org/Doc/Dev/ConversionH6
New Revision: 3
Change log: Some thoughts on what H6 apps look like and what (little)
needs to be done vs what (much more) SHOULD be done
@@ -3,4 +3,253 @@
These are by far not complete.
+
+++ General
+
+* Unnamespaced code mostly follows Horde 5 Coding standards and very
little conversion requirements
+* Most of the upgrade requirements are only targeted at namespaced code
+* With some necessary exceptions, Horde 6 should be able to cooperate
with Horde 5-ish apps and libraries with very little modification for
composer compat
+* Deprecate PSR-0 autoloading but don't break it intentionally.
+* Consider: Horde 5 Framework lived out 2012-2021, I would like Horde
6 to be intentionally shortlived, deprecating more stuff over time.
+* Allow a very smooth transition path, avoid big bang changes.
+
++++ Exceptions to the rule
+* Most of the upgrade requirements are only targeted at namespaced code
+
++++ Mandatory
+
+* Horde:: is deprecated
+* [rla thinks] we should adopt PSR-2, PSR-12 wholesale with an
optional opt-out for protected/private underscoring
+* Composer based setup must work with all libraries we intend to keep
+* [rla thinks] Deprecate Horde_Forms usage. Keep it where it is until
we can refactor, but make it clear this is a legacy concept
+* Update Horde_Core to support both namespaced apps non-namespaced
apps and mixed.
+* Update Horde_Test to support both namespaced and unnamespaced unit tests
+* Unit Tests must support the current stable phpunit
+* Check all pear package dependencies if they are also available via
packagist or a proper composer channel rather than pear
+
+* Update skeleton to use
+ * Horde Controllers,
+ * Namespacing,
+ * Type Hints where appropriate and
+ * Model Interface and Rdo implementation rather than driver architecture
+
++++ Optional / Best Practice
+
+* Try to drop usage of pear libraries if we only use little portions
of their api
+ * Consider if there is a more modern alternative available on packagist
+ * Consider if it is easy to
+* Unbundle bundled code if you can, use composer dependencies
+* Maintain a branch or fork of Skeleton to showcase Horde Ajax
Framework and an SPA frontend
+* All URL generation should be moved to Horde_Routes_UrlWriter driven
by config/routes.php
+* A more copy/paste ready boilerplate for ClassLevelDocBlock,
FileLevelDocBlock, MethodLevelDocBlock
+
++++ Exceptions To The Rule
+
+* ?
+
+++ Namespaces and dir layout
++++ Mandatory
+
+* The home namespace for a library is $Vendor\$Name\, e.g. Horde\View
+* The home namespace for a subtype library is $Vendor\$Parent\$Name
e.g. Horde\Xml\Element rather than Horde\Xml_Element
+
+* The Home Namespace for an app in $Horde\$Name, e.g. Horde\Turba
+ * TODO: Should we allow third party apps to use their proper vendor
if the composer package is type horde-app? We could have an optional
registry key (B1 clause)
+
+* The Horde Base app is Horde\Base
+ * DISCUSS: Any arguments for calling it Horde\Horde? Should we
rename/alias the base app horde/base altogether?
+
+* If a library has an interface or base class Horde_$Name, promote it
to either:
+ - Horde\$Name\$Name
+ - Horde\$Name\Interface
+ - Horde\$Name\Base
+ - Horde\$Name\Constants
+ - In case of base classes, re-think if we are not better off with
an interface and some trait
+
+
+* Horde:: is Horde\Core\Horde:: though I think we should re-think
using it at all
+* Un-namespaced controllers go to App\, namespaced controllers go to
src/Controller
+* Un-namespaced library code goes to lib/
+* Namespaced library code goes to src/.
+
+
++++ Optional
+
+* Unnamespaced code should be preceded with backslash \ to ensure we
do not run into issues with namespaced derived classes or use cases
+
++++ Exceptions to the rule
+
+* Code which was already namespaced before Horde 6 may be left in
lib/ if other unnamespaced code depends on it
+
+
+
+++ PHPDoc / Type Hints / Type Declarations
+
+Remember Liskov:
+* Interface/Base class parameters should be as specific as possible.
Derived/Implementing classes can only accept the same or less specific
definitions
+* Interface/Base class return type parameters should be as unspecific
as makes sense. Derived/Implementing classes can announce to return
the same or more specific definitions.
+
+
++++ Mandatory
+
+* PHPDoc all parameters and return values, regardless if you have
typehinted them.
+* Prefer A|B|null over "mixed"
+
++++ Optional
+
+* Parameter Type hints for string, int, boolean SHOULD be added to
the namespaced interfaces where possible
+* Parameter Type hints for classes SHOULD be added to be added to the
namespaced interfaces where possible.
+* Use Nullable (?) for optional parameters
+* Prefer iterable over array if you can to allow later upgrades from
arrays to container objects, generators, iterators etc
+* Where Type Hints cannot express exactly what a parameter accepts,
typehint for "object" or leave typehint altogether. In any case, make
the PHPDoc more specific and wordy
+
++++ Exceptions to the rule
+* ?
+
+++ GLOBALS usage
+
++++ Whitelist
+
+* For $injector:
+ * Horde\Core\Registry
+ *
+
+* For $_GET, $_POST, $_REQUEST
+ * Horde\Controller\
+ * Horde\Routes\
+ * Horde\Core\
+ * Horde\Rpc\
+ * Horde\Base\
+ * Horde\Cli\
+
+* for $config:
+ * Horde\Base\
+ * Horde\Core\Registry\
+ * Horde\Core\Config\
+
+* for $page_output:
+ * ?
+
+* for $__autoload
+
++++ Mandatory
+
+* Namespaced Classes which are not in the whitelist must get the
listed globals from constructor or method parameter
+
+
++++ Optional
+
+* Factories SHOULD use constructor/setter parameters rather than
globals if possible.
+
++++ Exceptions to the rule
+* ?
+
+
+++ DI / Injector and Constructors
+
+As Horde has evolved over the years, we have different types of
classes, sometimes even mixed.
+
+* Classes which implement create-on-the-fly objects with no/few
dependencies or are mostly static, e.g. Horde_Url, Horde_Date,
Horde_String
+* Classes which have one or few instances to cover most or all use
cases, e.g. the DB Driver, the registry. They will typically provided
by the injector directly or via a factory without explicit parameters
+* Messy stuff which cannot decide if it is a factory or a base class
or wants composition, e.g. Horde_Form, Horde_Rpc
+* Classes which are intended as Per-Entity objects
+
++++ Mandatory
+
+* Constructors SHOULD depend on interfaces or base classes, not on
the (single) implementation.
+* HordeBase or App configuration SHOULD be depended on as *TODO*
object, not as array and explicitly not via global state
+* Apps must not care for configuration of other apps, only theirs or
HordeBase
+
+* Namespaced app-internal objects which implement an interface should
be registered with the injector using the fully qualified name to
allow for autowiring of other code using it. e.g. if you have an
interface or abstract class Horde\Kronolith\Calendar\Resource, bind it
as 'Horde\Kronolith\Calendar\Resource' - not
'\Horde\Kronolith\Calendar\Resource\'
+* As this makes for very long keys which are hard to read and write,
as a convention, also register the same object as 'Calendar/Resource'.
This is the string you would use when calling the injector yourself
+* If an object can be autowired by the injector (all constructor
dependencies are injectable and already registered with the injector),
only register the shorthand but not the fully qualified name.
+
++++ Optional
+
+* Constructors SHOULD better depend on interfaces rather than base
classes but SHOULD NOT depend on the (single) implementation.
+* Try to refactor constructors to only require interfaces which can
be provided by the injector (directly or via a factory)
+* If a class used to have a very flexible constructor allowing
multiple calling convention, try to distill one calling convention and
create either static creator methods or an external factory/builder
class
+* Internal creation of objects from other packages should be left to
trivial cases (e.g. Horde_Date). Otherwise it should happen by
injecting some instance provider.
+* Use sub-injectors to ensure we do not spill application specific
bindings into other apps.
+
++++ Exceptions to the rule
+
+* Dependencies may be created internally with some hardcoded setup in
case of convenience/fallback scenarios. We should not overuse this as
it encourages relying on it.
+
+++ reqire/require_once, Horde\Autoloader and Composer Autoloader
+
++++ Required
+
+* include, require and require_once should only ever happen to setup
autoloading or as part of the autoloader or related to non-php code
(templates, data)
+* make all explicit require/require_once check if the required class
is already available for loading
+
+* At least for now, we should not drop Horde\Autoloader
+* Horde\Autoloader must always act AFTER the composer autoloader, not first
+
++++ Optional
+
+* Detect composer autoloader and provide a backend which translates
Horde\Autoloader runtime API to composer API
+
++++ UNCLEAR
+
+* Do we still want to support git-tools setups?
+
+
++ package specific notes
+++ Horde\Injector
++++ Optional
+* Implement PSR-11 but do not deprecate getInstance(), setInstance()
[PR exists]
+
+++ Horde\Date
++++ Optional
+ * Horde\Date use cases should generally be checked for timezone
mismatches e.g. when converting UTC strings from database
+ * There is some headache potential with Horde\Db implicitly using
Horde\Date without timezone information when SELECTing datetime values.
+ * This is especially annoying with Horde\Rdo
+ * Horde\Date constructor should always require an explicit timezone
and current time
+ * Provide convenience static methods for "fromThisFormat()",
"fromThatFormat()" to keep that complexity out of the value object
+
+++ Horde\Core
+Mandatory:
+
+* Provide an alternative for Horde_PageOutput which only returns
strings rather than doing output (needed, otherwise controllers are
littered with ob_start())
+* Provide a type to wrap horde config array for DI
+* A lot of class names must be fixed for namespacing
+
+Optional:
+* Registry (probably rather 6.1) Provide a more robust inter-app API
but don't break the current inter-app API for now
+ * Allow passing objects as long as they implement sleep/wakeup so
RPC usecases don't break
+ * Allow implementing specific APIs and methods in
Horde\$App\Api\$Api but still support $App_Api class for non-namespaced
+ * Move vfs/webdav implementations to Horde\$App\Api\Vfs
+
+++ Horde\Rpc
+Mandatory:
+* Fix the signature missmatch errors
+Optional:
+ * Default to JsonRpc rather than XmlRpc
+ * Refactor to controller framework
+
+
++ Library Upgrade strategy (pre release)
+
+Apart from mandatory changes, we should not currently port all horde
to the new standard quickly.
++ Mandatory
+* Existing close-to-core libraries should provide some backward
compat either by keeping the code in lib/ mostly unchanged or by
having lib/ be some wrapper for src/
+* Once H6 has been released, H6.x libraries need to keep whatever
legacy support they had until H7
+
+++ Optional
+
+* New libraries should only be implemented according to current best
practices
+* Existing fringe libraries may be completely converted to
namespaces, dropping unnamespaced versions altogether (pre H6 release)
+
+++ Exception to the rule
+
+* Breaking apps which have not officially been released as Horde 5 is
OK, we may fix them later. Still, it's nice to be nice if you can.
+
++ App Upgrade Strategy (pre release)
+
+++ Required
+* Apps may change their internal composition in minor releases (middle digit)
+* Apps may mix and match src/ and lib/ but may only ever have ONE
implementation of a class either src/ or lib/. (internal consistency)
+
+++ Optional
+* Apps should move from unnamespaced library usage to namespaced
library usage whenever feasible even if there is a compat layer.
More information about the commits
mailing list