Page MenuHomePhabricator
Create Task
Maniphest T108286

[Task] Move Wikibase Lib code that belongs in DataModel Services to this new component
Closed, ResolvedPublic
Actions

Description

This is to collect and track services to move from Wikibase Lib to Wikibase DataModel Services as per https://phabricator.wikimedia.org/T93741

A similar task, for the services in Wikibase DataModel has already been completed: https://phabricator.wikimedia.org/T104187

Please review items in the "needing feedback" list and promote them to the "list with +1". This will allow us to start with moving those classes for which there are no serious concerns, while we work out what to do with the other ones separately.

List with +1:

  • Wikibase\Lib\Store\EntityRedirect (could go in DM)
  • Wikibase\Lib\EntityIdFormatter
  • Wikibase\Lib\EscapingEntityIdFormatter
  • Wikibase\Lib\PlainEntityIdFormatter
  • Wikibase\Lib\TypedValueFormatter
  • Wikibase\Lib\EntityIdLabelFormatter (depends on LabelDescriptionLookup)
  • Wikibase\Lib\Parsers\SuffixEntityIdParser
  • Wikibase\Lib\ClaimGuidValidator
  • Wikibase\Lib\Store\EntityLookup (there already is an ItemLookup and PropertyLookup in DMS)
  • Wikibase\Lib\Store\TermLookup
  • Wikibase\Lib\Store\LabelDescriptionLookup
  • Wikibase\Lib\Store\EntityRedirectLookup
  • Wikibase\Lib\Store\EntityRetrievingTermLookup (depends on TermLookup and EntityLookup)
  • Wikibase\Lib\Store\LanguageLabelDescriptionLookup (depends on LabelDescriptionLookup and TermLookup)
  • Wikibase\Lib\EntityRetrievingDataTypeLookup (depends on EntityLookup)
  • Wikibase\Lib\Store\RedirectResolvingEntityLookup (depends on EntityLookup and EntityRedirectResolvingDecorator)
  • Wikibase\PropertyLabelResolver (this is in lib/includes/store, the NS is just different)
  • Wikibase\Lib\Store\EntityPrefetcher and NullEntityPrefetcher
  • Wikibase\Store\TermBuffer (this is in lib/includes/store, the NS is just different)
  • Wikibase\ValuesFinder
  • Wikibase\WikibaseDiffOpFactory
  • Wikibase\Lib\Store\EntityRedirectResolvingDecorator (depends on UnresolvedRedirectException which derives from StorageException and MWException. That would need changing)

Related Objects
Search...

Event Timeline

JeroenDeDauw created this task.Aug 7 2015, 6:37 AM
JeroenDeDauw raised the priority of this task from to Needs Triage.
JeroenDeDauw updated the task description. (Show Details)
JeroenDeDauw added projects: Wikibase-DataModel-Services, Wikidata.
JeroenDeDauw subscribed.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 7 2015, 6:37 AM
JeroenDeDauw added subscribers: aude, • Jonas, daniel and 5 others.Aug 7 2015, 6:38 AM
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 6:46 AM
JeroenDeDauw set Security to None.
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 6:49 AM
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 6:52 AM
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 6:58 AM
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 7:04 AM
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 7:09 AM
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 8:32 AM
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 8:39 AM
JeroenDeDauw updated the task description. (Show Details)Aug 7 2015, 8:41 AM
JeroenDeDauw added a parent task: T108296: [Epic] Remove WikibaseLib.Aug 7 2015, 9:00 AM
Addshore updated the task description. (Show Details)Aug 7 2015, 10:58 AM

Well, I just went thought them all and agree with them all, so I have moved them all to the +1 section in the description!
I may throw some patches up for these today!

Addshore added a comment.Aug 7 2015, 2:22 PM

Many of these are now merged into the Services repo.

The following PRs are still open:

The following services still need to be moved / have PRs made for them out of the list above:

  • Wikibase\Lib\Store\EntityRetrievingTermLookup (depends on TermLookup and EntityLookup)
  • Wikibase\Lib\Store\LanguageLabelDescriptionLookup (depends on LabelDescriptionLookup and TermLookup)
  • Wikibase\Lib\EntityRetrievingDataTypeLookup (depends on EntityLookup)
  • Wikibase\Lib\Store\RedirectResolvingEntityLookup (depends on EntityLookup and EntityRedirectResolvingDecorator)
  • Wikibase\Lib\Store\EntityRedirectResolvingDecorator (depends on UnresolvedRedirectException which derives from StorageException and MWException. That would need changing)
Addshore added a project: Patch-For-Review.Aug 7 2015, 2:22 PM
thiemowmde added a comment.Aug 7 2015, 2:40 PM
  • Agree on all EntityId formatter and parsers.
  • Agree on all lookups, prefetchers and resolvers.
  • Disagree on EntityRedirect. DataModel please. This is not even a "service", only a very simple value object.
  • Disagree on TypedValueFormatter. This belongs to DataValues.
  • Mixed feelings about ValuesFinder. Could also go in DataValues.
JeroenDeDauw added a comment.EditedAug 8 2015, 5:19 AM

Thanks for the feedback Thiemo. Unfortunately I've already merged TypedValueFormatter and ValuesFinder now, as there where no objections on the PRs and I had not seen this. No real damage done as no release has happened yet. Let's discuss them and revert if needed.

Mixed feelings about ValuesFinder. Could also go in DataValues.

It can't - it uses PropertyDataTypeLookup which is in DM Services and PropertyId which is in DM

Disagree on TypedValueFormatter. This belongs to DataValues.

When I looked at it at first, I had a similar reaction. After all, this class does not strictly speaking depend on DM, so it seems to not match the contract of DM Services. .... #fail. Ok, I agree now this does not belong in this component. I mistakingly identified $dataTypeId as DM concept. And since it is not, this class indeed does not match the components contract of residence :)

Removal submitted in https://github.com/wmde/WikibaseDataModelServices/pull/25

JeroenDeDauw added a comment.Aug 8 2015, 5:29 AM

I suggest we try to round this up quickly, as otherwise modifications we need to mirror might happen to the old copies. This task does not need t be 100% complete before we do the first release with moved stuff.

Ricordisamoa subscribed.Aug 8 2015, 11:20 PM
JeroenDeDauw added a comment.Aug 9 2015, 3:09 PM
This comment was removed by JeroenDeDauw.
JeroenDeDauw added a comment.Aug 9 2015, 6:33 PM

Moved over EntityRetrievingTermLookup, LanguageLabelDescriptionLookup and EntityRetrievingDataTypeLookup.

https://github.com/wmde/WikibaseDataModelServices/pull/27
https://github.com/wmde/WikibaseDataModelServices/pull/30
https://github.com/wmde/WikibaseDataModelServices/pull/32

Also started with EntityRedirectResolvingDecorator, though that needs more work. Feel free to pick up where I left it so far: https://github.com/wmde/WikibaseDataModelServices/pull/33

JeroenDeDauw added a comment.Aug 12 2015, 1:41 PM

Now also moved RedirectResolvingEntityLookup: https://github.com/wmde/WikibaseDataModelServices/pull/40

Which are all the classes in the list if I'm not mistaken... still need to remove the old copies though.

Addshore added a comment.Aug 14 2015, 3:01 PM

I will make a new release of the components (DMS) once I have made a PR for sorting the EntityLookup stuff & exceptions etc

daniel added a comment.Sep 10 2015, 1:33 PM

What's the status of this? Still in limbo? The code is going to diverge...

daniel triaged this task as High priority.Sep 10 2015, 1:35 PM

Bumping to high: The component split was started, and now we have several classes in the old and new component. If we want to go forward with this, we need to get a move on. Otherwise, the code is just going to diverge, and we have to start over.

daniel moved this task from incoming to consider for next sprint on the Wikidata board.Sep 10 2015, 1:35 PM
thiemowmde added a comment.Sep 10 2015, 2:13 PM

I checked all classes in the current Wikibase-DataModel-Services 2.0.1 and found these duplicates:

  • EntityRedirectResolvingDecorator
  • UnresolvedRedirectException
  • RedirectResolvingEntityLookup
  • RestrictedEntityLookup
  • PropertyLabelResolver
  • TermBuffer

All these should be removed from lib fast. Do not forget to check if they are identical!

thiemowmde renamed this task from Move Wikibase Lib code that belongs in DataModel Services to this new component to [Task] Move Wikibase Lib code that belongs in DataModel Services to this new component.Sep 10 2015, 2:14 PM
thiemowmde removed a project: Patch-For-Review.
thiemowmde added a subtask: T108963: [Task] Use services from WikibaseDataModelServices added in 1.1.0 release.Sep 10 2015, 2:44 PM
JeroenDeDauw added a comment.Sep 14 2015, 11:17 AM

Thanks for providing the list Thiemo, this is very helpful. I was under the apparently wrong impression we already removed all the old copies. Will have a look at removing the remaining ones.

JeroenDeDauw added a comment.EditedSep 14 2015, 12:32 PM

There appears to be a design and/or problem with EntityRedirectResolvingDecorator, RedirectResolvingEntityLookup and UnresolvedRedirectException that is preventing ditching the old versions. Needs more investigation. Too tired to untangle that now.

JeroenDeDauw added a comment.Sep 14 2015, 12:42 PM
Diffusion mentioned this in rMEXTf404cfc3d173: Updated mediawiki/extensions Project: mediawiki/extensions/Wikibase….Sep 15 2015, 8:45 AM
JeroenDeDauw mentioned this in rEWBA6f3d1c110e09: Remove old copy of PropertyLabelResolver interface.Sep 15 2015, 8:46 AM
JeroenDeDauw mentioned this in rMEXT5c7dfd08a1b7: Updated mediawiki/extensions Project: mediawiki/extensions/Wikibase….Sep 15 2015, 9:35 AM
JeroenDeDauw mentioned this in rEWBA70f8f219aef2: Remove old copy of TermBuffer interface.
JeroenDeDauw mentioned this in rMEXTf3e98c0c1eeb: Updated mediawiki/extensions Project: mediawiki/extensions/Wikibase….Sep 16 2015, 8:24 PM
JeroenDeDauw mentioned this in rEWBA321e006f958d: Remove old copy of RestrictedEntityLookup.
JeroenDeDauw added a comment.Sep 17 2015, 12:41 PM

The final removal (EntityRedirectResolvingDecorator and RedirectResolvingEntityLookup) is done with https://gerrit.wikimedia.org/r/#/c/239085/

Addshore added a comment.Dec 4 2015, 1:18 PM

So all of the things are now removed (as the above was merged) and this can now be closed? @JeroenDeDauw?

JeroenDeDauw closed this task as Resolved.Dec 8 2015, 10:36 AM
JeroenDeDauw claimed this task.

Yeah, we can always open a new ticket if we find something more

JeroenDeDauw closed subtask T108963: [Task] Use services from WikibaseDataModelServices added in 1.1.0 release as Resolved.Dec 8 2015, 10:41 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits