+ Notes on converting apps from Horde 5 to Horde 6 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 tosupport both namespacedapps non-namespaced apps and mixed. * Update Horde_Test to support bothnamespaced and unnamespacedunit 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/ * Namespacedlibrary 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 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. * 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 class combined with ::class, e.g. <code type="php"> // Use a factory to get an implementation // In case the factory cannot be autowired itself, you need to register how to get it first. $injector->bindFactory(Horde\Kronolith\Calendar\Resource::class, Horde\Kronolith\Calendar\Factory::class, 'create') $injector->bindImplementation(Horde\Kronolith\Renderer::class, Horde\Kronolith\Renderer\Default::class); * Note there are no strings, this is the class itself - ::class returns the fully qualified strings * If you have imported the class with use, you can just use the imported name: <code type="php"> use Horde\Kronolith\Calendar\Resource; use Horde\Kronolith\Calendar\Factory as CalendarFactory; use Horde\Kronolith\Renderer; use Horde\Kronolith\Renderer\Default as RendererDefault; // Use a factory to get an implementation // In case the factory cannot be autowired itself, you need to register how to get it first. $injector->bindFactory(Resource::class, CalendarFactory::class, 'create') // In case you want a specific implementation and it can be autowired, just bind it. $injector->bindImplementation(Renderer::class, RendererDefault::class); * Registering an implementation by an arbitrary string is still legal, we even might need it for bc compat with some designs. <code type="php"> $injector->bindImplementation('Forms', '\Horde_Forms_Base'); </code> +++ 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.