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

RFC: Upgrading Zend Framework #5359

Closed
kinglozzer opened this issue Apr 20, 2016 · 22 comments
Closed

RFC: Upgrading Zend Framework #5359

kinglozzer opened this issue Apr 20, 2016 · 22 comments

Comments

@kinglozzer
Copy link
Member

RFC: Upgrading Zend Framework

Author: @kinglozzer
Status: Draft
Version: 0.1

Disclaimer: there are outstanding issues yet to be resolved in this RFC that I am actively seeking opinions on (see the “Problems” section).

Introduction

SilverStripe Framework is currently running a release of Zend Framework 1 dated approximately 2011. We’ve added a few hacks & hotfixes since, but the code is largely unchanged since it was originally added. I think we’re long overdue an update.

We currently depend on the following ZF1 components:

  • Translate - i18n depends on this
  • Cache - SS_Cache depends on this, as does i18n & ZF1’s Translate component (Translate has opt-in caching of translations, which we do use)
  • Currency - DBMoney depends on this for formatting
  • Date - a few form fields, DBDate, DBDateTime, DBTime all depend on this. There are a few other uses (e.g. Member uses it to show date format examples), but those would be relatively simple to update
  • Locale - i18n, a few form fields, ZF1’s Currency & Date components depend on this
  • Loader - only used by the components above

Motivation

I can see the following benefits to upgrading to ZF2:

  • Less code in our repository. As ZF1 was included in framework before Composer was commonplace, it currently sits in thirdparty/.
  • Fewer bugs. Besides the handfull of patches we’ve applied ourselves, the code has barely been touched for around 5 years. No doubt many bugs have been discovered and fixed since then that we’ve missed.
  • Easier upgrade path in future. Using Composer instead of installing in thirdparty/ will mean that we get new releases automatically.
  • New features. For example, there are now many more cache adapters available than there were in ZF1.
  • Better community support.
  • Better performance (maybe? I’ve not done any benchmarks on what I’ve tested so far).

Proposal

  1. Upgrade Translate (now called i18n) & Cache. i18n depends on Zend’s Cache component for caching translations - functionality we’d definitely want to keep - so we can’t just drop Cache. Refactor i18n and SS_Cache APIs (I’ve done a small amount of work on this).
  2. Remove Currency component - the component doesn’t exist in ZF2. There is a CurrencyFormat view helper included in the i18n component that we might make use of instead, but it does require that the PHP intl extension is installed (see “Problems” below).
  3. Remove Date component - again, this component no longer exists. There is a DateFormat view helper which also requires intl (see “Problems” below).
  4. Remove Locale component. Core uses of this can be worked around in places, but this will need consideration for other classes such as DateField and NumericField (see “Problems” below).

Problems

intl

Significant portions of the code we’re using in ZF1 have no equivalent in ZF2, or now require the intl PHP extension. As a result of this, we will either have to refactor these in such a way that the functionality is optional (e.g. form fields are only locale aware if intl is installed) or look for alternative libraries that offer similar/equivalent functionality. Many Composer i18n/i10n packages now require ext-intl, so perhaps this is indicative of modern PHP development.

This is probably the main barrier for this RFC. Choosing alternative libraries will be difficult, as it’s hard to guarantee feature parity with what we currently have.

Backward compatibility

The API for the components has changed significantly since ZF1, which will add to upgrade pains. At a fundamental level method names and argument orders have changed (for example $cache->get($key) and $cache->save($data, $key) have become $cache->getItem($key) and $cache->setItem($key)), but there are also deeper design changes.

Because of this, I’m proposing delaying this work until 5.0 - just because we can break as much stuff as we like in each major release doesn’t mean we should, and there is no urgent need for an upgrade.

Alternatives

The simplest alternative that I can see is to remove thirdparty/Zend/ and instead install the latest copy of ZF1 with Composer. This solves the issue of our copy of ZF being out of date (ZF1 is still maintained, just not as actively as ZF2) and should help keep us up-to-date with bug fixes. The drawbacks are that we don’t get any new features, and that support for ZF1 will likely end much sooner than ZF2 so we may find ourselves in the same boat further down the river.

This alternative proposal may be appropriate for targetting SilverStripe 4.0, with a view to solving the remaining issues of this RFC and upgrading to ZF2 for SilverStripe 5.0.

@tractorcow
Copy link
Contributor

My vote is for leaving the status quo for 3.x, but with a focus on upgrading 4.0 to pull in the latest version of zf1 for 4.x via composer, maybe alpha2. I don't see a problem with 5.0 as a milestone for upgrading to zf2; By the time maybe there will be something better as an alternative?

@sminnee
Copy link
Member

sminnee commented Apr 29, 2016

I think it's better to frame the biggest component of this RFC as "the future of internationalisation" rather than "the future of Zend Framework in SilverStripe".

The other piece "the future of caching", I don't particularly like the ZF caching library and it's possible that another library would serve us better. I don't know if we want to look at a PSR-6 library such as https://github.com/tedious/Stash or https://github.com/symfony/cache (full list here https://packagist.org/providers/psr/cache-implementation)

@kinglozzer
Copy link
Member Author

My vote is for leaving the status quo for 3.x, but with a focus on upgrading 4.0 to pull in the latest version of zf1 for 4.x via composer, maybe alpha2

Yeah, now I’ve had some more time to think about this I think that’s the best approach. Sticking with ZF1 means much less risk of regression now that master is quite “mature” and closing in on 4.0.

By the time maybe there will be something better as an alternative?

I think it's better to frame the biggest component of this RFC as "the future of internationalisation" rather than "the future of Zend Framework in SilverStripe".

Good point. I’d been thinking solely of keeping things as similar as possible to the status quo (with the intention of targeting 4.0), but if there would be major changes to get ZF2 out the door then we may as well do a wider review of how we do things instead.

I don't particularly like the ZF caching library and it's possible that another library would serve us better

I completely agree. I’d stuck with keeping ZF cache as a dependency because we depend on that for the i18n component (and it’s far too slow without caching), but that could be up for debate if we were to look at other internationalisation libraries. If we stick with Zend we could even keep ZF cache for i18n only, and make SS_Cache wrap a different caching lib - assuming we’d want to keep SS_Cache that is.

@kinglozzer
Copy link
Member Author

@chillu Can you offer any insight into ad6b7a4? It’s the only thing I’m stuck on with updating to the latest ZF1 and moving to Composer. mi and mi_NZ still aren’t recognised in zf1/zend-locale, and I can’t find anything resembling the custom XML file added in that commit in the latest CLDR release.

Based on the discussion in #1457, this might be a showstopper for moving ZF1 out of thirdparty... :(

@tractorcow
Copy link
Contributor

Options are:

  • Fork Zend_Locale and re-vendor.
  • PR to https://github.com/zf1/zend-locale and hope they accept (which feels unlikely)
  • Hack in a "custom" Zend_Locale class, and ensure that it is autoloaded instead of the version provided by that composer module. This is risky in that you will end up with multiple files with the same class included.

I notice that they did include maori in https://github.com/zf1/zend-locale/blob/master/library/Zend/Locale/Data/Translation.php for some reason. :)

@sminnee
Copy link
Member

sminnee commented Jun 2, 2016

The 2nd option sounds like the only tolerable one... Otherwise, do we upgrade to ZF2?

Do we need to live with php-intl becoming a requirement for SS4?

@kinglozzer
Copy link
Member Author

kinglozzer commented Jun 2, 2016

Right, I’ve worked out what’s gone on with the missing mi/mi_NZ data. In CLDR 2.0 (release notes), locales with “insufficient” data were moved out of the releases, and are only available in the SVN repo (latest release) - which is presumably why they’re now missing from the ZF1 releases.

Mystery solved, but unfortunately doesn’t solve our problem 😅. It might possibly make the ZF1 maintainers more amenable to a PR though... I’ll open an issue, you never know.

Do we need to live with php-intl becoming a requirement for SS4?

If we decide to upgrade to ZF2, or switch to another package, I think it will need to be a requirement. I can’t find any other i10n/i18n packages that don’t require that extension.

Edit: Created an issue here zendframework/zf1#699

@sminnee
Copy link
Member

sminnee commented Jun 2, 2016

I think that'd be better than retaining support for old code.

It may be that, in cases where intl isn't installed, we can run with more limited localisation features. For comparison, Moodle seems to recommend rather than require it.

@chillu
Copy link
Member

chillu commented Jun 24, 2016

After a bit of digging, ZF1 is indeed available through composer. I couldn't find an official reference from the ZF website though, might be somebody's pet project. Either way, I don't see a whole lot of benefit in spending energy to get ZF1 out of thirdparty/, without a larger refactor of the functionality. The git repo size won't shrink (megabytes of CLDR XML...). Repo network transfer and archive download size will be the same. A composer create-project will take even longer than it already does due to the additional dependencies.

ZF3 is in development, so it's awkward timing to upgrade to ZF2 (http://framework.zend.com/blog/announcing-the-zend-framework-3-roadmap.html). They seem to be getting close to a release there.

I think we can refactor i18n to work with php-intl in a fairly backwards compatible fashion. We can change the i18n class from static accessors into an injectable instance is a feasible 5.x change, and would allow us to make php-intl an optional dependency. The ICU lib which php-intl depends on works based on the same CLDR locales as Zend - since all those XML files in the Zend project are just a data dump straight from the Unicode CLDR project. php-intl and ZF also both use ISO date formats. A good example where betting on standards benefits us on the long run :)

php-intl seems to provide all the features we need (date/time formats, currency formats, number formats). We'd still need to use a third party lib for translations (e.g. ZF2/3 i18n).

Note that we might still run into the "missing mi_NZ" locale issue; Debian Stable php-intl package comes with libicu 52. Which in turn packages in CLDR 24. Which doesn't have mi_NZ either (neither does the latest CLDR release). Since we can't hack in support in the same way as with ZF1 there, we would need to implement a locale cascade to fall back to en_NZ. It's very much an edge case though - since the mi_NZ issue is limited to formats, rather than translations (where we provide a free form locale).

@kinglozzer
Copy link
Member Author

Note that we might still run into the "missing mi_NZ" locale issue; Debian Stable php-intl package comes with libicu 52. Which in turn packages in CLDR 24. Which doesn't have mi_NZ either (neither does the latest CLDR release). Since we can't hack in support in the same way as with ZF1 there, we would need to implement a locale cascade to fall back to en_NZ. It's very much an edge case though - since the mi_NZ issue is limited to formats, rather than translations (where we provide a free form locale).

Just checking, have you seen my comment above? It is still present, it’s just not bundled in the main release. I wonder if (hope!) libicu will pull in data from the SVN repo...

@chillu
Copy link
Member

chillu commented Jun 24, 2016

Oh, maybe I've looked in the wrong place then? common/main doesn't have it, but seed/main does. No idea what the difference is in CLDR terms.

@tractorcow
Copy link
Contributor

Significant portions of the code we’re using in ZF1 have no equivalent in ZF2, or now require the intl PHP extension.

I'd be happy to require php-intl for advanced features, but I much prefer to pull in an existing library than having our own wrapper around php-intl.

If we require both zf2-i18n and php-intl (or accept limited functionality if it's not available) then what additional features will we need to build?

If the missing mi_NZ locale becomes a platform maintenance task, then I'm happy to accept this given we have an upgrade path for users that require mi_NZ (e.g. installation docs for debian, etc).

@tractorcow
Copy link
Contributor

tractorcow commented Jul 14, 2016

I guess upgrading to zf2 (and doing it properly) would make a zf3 upgrade in the future easier, so let's not discount that as a stepping stone.

@kinglozzer
Copy link
Member Author

Worth noting here that ZF1 reaches its EOL on 28th September. It’s not super critical for us as we don’t rely on some of the more sensitive things like the database components, but it’s another sign that we need to bite the bullet with this task soon :)

ZF3 is out now, but requires PHP 5.6.

@dhensby
Copy link
Contributor

dhensby commented Sep 26, 2016

When is ZF2 EOL? I guess it's unlikely we'll push to PHP 5.6 for SS4

@kinglozzer
Copy link
Member Author

March 31st, 2018 if we stick an LTS version (2.4): https://framework.zend.com/long-term-support

@dhensby
Copy link
Contributor

dhensby commented Sep 26, 2016

hmm, we're hoping to support SS4 into 2nd quater of 2020

@sminnee
Copy link
Member

sminnee commented Sep 26, 2016

When is ZF2 EOL? I guess it's unlikely we'll push to PHP 5.6 for SS4

If there was a significant reason to do it, we could put it to the community.

@chillu
Copy link
Member

chillu commented Sep 27, 2016

FYI, I've separated out the i18n discussion to a new RFC (to be written): #6090

@dhensby
Copy link
Contributor

dhensby commented Sep 27, 2016

If there was a significant reason to do it, we could put it to the community.

Agreed

@tractorcow
Copy link
Contributor

I've drafted up a new RFC for replacement of zend at #6194

@chillu
Copy link
Member

chillu commented Oct 31, 2016

Closing in favour of more focused tickets:

@chillu chillu closed this as completed Oct 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants