Page MenuHomePhabricator

Standardize on a single PHP static array writer implementation
Closed, ResolvedPublic

Description

There's an implementation in SyntaxHighlight, TrustedXFF, and core's LCStoreStaticArray. They all differ slightly, and given that we're moving away from CDB to static PHP arrays, we should have a single implementation in core that can be reused.

Event Timeline

Is this something that might be worth implementing straight as a vendored package?

Change 448992 had a related patch set uploaded (by Krinkle; owner: Reedy):
[mediawiki/extensions/TrustedXFF@master] Run str_replace on a string

https://gerrit.wikimedia.org/r/448992

Change 448993 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/TrustedXFF@REL1_31] Run str_replace on a string

https://gerrit.wikimedia.org/r/448993

Change 448994 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/TrustedXFF@REL1_30] Run str_replace on a string

https://gerrit.wikimedia.org/r/448994

Change 448992 merged by jenkins-bot:
[mediawiki/extensions/TrustedXFF@master] Run str_replace on a string

https://gerrit.wikimedia.org/r/448992

Change 448993 merged by jenkins-bot:
[mediawiki/extensions/TrustedXFF@REL1_31] Run str_replace on a string

https://gerrit.wikimedia.org/r/448993

Change 448994 merged by jenkins-bot:
[mediawiki/extensions/TrustedXFF@REL1_30] Run str_replace on a string

https://gerrit.wikimedia.org/r/448994

There's an implementation in SyntaxHighlight, TrustedXFF, and core's LCStaticArray.

Also mediawiki-extensions-WikimediaMaintenance:/dumpInterwiki.php, and as of recent wfMakeStaticArrayFile() in MediaWiki core as used by maintenance/generateNormalizerDataMl.php.

Oh, I forgot about wfMakeStaticArrayFile. I think that meets my needs. Theoretically we could put it in the cdb library to match \Cdb\Reader\Hash, but that seems weird since it has nothing to do with cdb.

Change 449000 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/SyntaxHighlight_GeSHi@master] Use wfMakeStaticArrayFile()

https://gerrit.wikimedia.org/r/449000

Change 449001 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[cdb@master] Add Cdb\Writer\Hash implementation

https://gerrit.wikimedia.org/r/449001

Legoktm renamed this task from Put a PHP static array writer implementation into MediaWiki core to Standardize on a single PHP static array writer implementation.Jul 29 2018, 5:02 AM

Change 449000 merged by jenkins-bot:
[mediawiki/extensions/SyntaxHighlight_GeSHi@master] Use wfMakeStaticArrayFile()

https://gerrit.wikimedia.org/r/449000

Tagging as a 1.32 release blocker to clarify on whether we want to librarize this (in cdb, or maybe something else) or whether we're ok with wfMakeStaticArrayFile() becoming a stable interface.

@Legoktm Aye, I apologise for including a new global function in 2018. It looked so nice alongside the wfGetPrecompiledData() function I was replacing at https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/434040/6/includes/GlobalFunctions.php.

Let's at least make it an autoloaded class rather than a global function before this becomes a stable release.

Change 453567 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Move wfMakeStaticArrayFile() into a class

https://gerrit.wikimedia.org/r/453567

Change 453568 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] [WIP] Remove wfMakeStaticArrayFile()

https://gerrit.wikimedia.org/r/453568

Change 453569 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/SyntaxHighlight_GeSHi@master] Use StaticArrayWriter class

https://gerrit.wikimedia.org/r/453569

Change 453570 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/SecureLinkFixer@master] Use StaticArrayWriter class

https://gerrit.wikimedia.org/r/453570

Change 453567 merged by jenkins-bot:
[mediawiki/core@master] Move wfMakeStaticArrayFile() into a class

https://gerrit.wikimedia.org/r/453567

Change 453570 merged by jenkins-bot:
[mediawiki/extensions/SecureLinkFixer@master] Use StaticArrayWriter class

https://gerrit.wikimedia.org/r/453570

Change 453569 merged by jenkins-bot:
[mediawiki/extensions/SyntaxHighlight_GeSHi@master] Use StaticArrayWriter class

https://gerrit.wikimedia.org/r/453569

Change 453568 merged by jenkins-bot:
[mediawiki/core@master] Remove wfMakeStaticArrayFile()

https://gerrit.wikimedia.org/r/453568

Change 453674 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Support nested arrays in StaticArrayWriter

https://gerrit.wikimedia.org/r/453674

Change 453675 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Use StaticArrayWriter in LCStoreStaticArray

https://gerrit.wikimedia.org/r/453675

Change 453674 merged by jenkins-bot:
[mediawiki/core@master] Support nested arrays in StaticArrayWriter

https://gerrit.wikimedia.org/r/453674

Change 453675 merged by jenkins-bot:
[mediawiki/core@master] Use StaticArrayWriter in LCStoreStaticArray

https://gerrit.wikimedia.org/r/453675

Krinkle triaged this task as Medium priority.Sep 2 2018, 12:58 AM

Change 449001 abandoned by Legoktm:
Add Cdb\Writer\Hash implementation

Reason:
Not going in this direction

https://gerrit.wikimedia.org/r/449001

The last implementation that needed porting is TrustedXFF.

This is marked as a release blocker. What's still needed here?

Looks like TrustedXFF still has its own version of StaticArrayWriter. I didn't realise the class names clashed, that's odd.

TrustedXFF isn't a bundled extension, so it shouldn't be a release blocker. I'll file a separate bug for that since I think it needs more work than just this part.