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.

Details

Related Gerrit Patches:
cdb : masterAdd Cdb\Writer\Hash implementation
mediawiki/core : masterUse StaticArrayWriter in LCStoreStaticArray
mediawiki/core : masterSupport nested arrays in StaticArrayWriter
mediawiki/core : masterRemove wfMakeStaticArrayFile()
mediawiki/extensions/SecureLinkFixer : masterUse StaticArrayWriter class
mediawiki/core : masterMove wfMakeStaticArrayFile() into a class
mediawiki/extensions/SyntaxHighlight_GeSHi : masterUse StaticArrayWriter class
mediawiki/extensions/SyntaxHighlight_GeSHi : masterUse wfMakeStaticArrayFile()
mediawiki/extensions/TrustedXFF : REL1_30Run str_replace on a string
mediawiki/extensions/TrustedXFF : REL1_31Run str_replace on a string
mediawiki/extensions/TrustedXFF : masterRun str_replace on a string

Event Timeline

Legoktm created this task.Jul 28 2018, 10:58 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 28 2018, 10:58 PM
Reedy added a subscriber: Reedy.Jul 29 2018, 12:42 AM

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

Reedy updated the task description. (Show Details)Jul 29 2018, 12:51 AM
Reedy removed a project: Patch-For-Review.

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.

Krinkle moved this task from Untriaged to To Do on the Librarization board.Aug 12 2018, 12:43 PM

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

Legoktm claimed this task.Aug 19 2018, 9:56 AM

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

Jdforrester-WMF added a subscriber: Jdforrester-WMF.

No open patchsets FWICS.

The last implementation that needed porting is TrustedXFF.

daniel added a subscriber: daniel.Dec 17 2018, 12:42 PM

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.

Legoktm closed this task as Resolved.Dec 20 2018, 9:39 PM

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.