| thiemowmde added a comment. |
@Smalyshev wrote:I don't see where term types are defined […]
That's exactly the issue I referred to. These "term types" being strings is mostly an implementation detail of how the wb_terms table is designed. The currently specified term types can be found in TermIndexEntry. Please use these constants to make it easier to find places working with these term types.
- TermIndexEntry::TYPE_LABEL = "label"
- TermIndexEntry::TYPE_ALIAS = "alias"
- TermIndexEntry::TYPE_DESCRIPTION = "description"
but could there be more?
We played around with ideas to have other types for #lexicographical_data, but we are most probably not going to do this. So these 3 types are all that exist.
Is Elastic implementation supposed to support them […]
Please either return no result if a caller asks for an unknown term type, or throw an exception.
Also, there's an overlap of coverage - you can use getLabel or you can use getPrefetchedTerm(... 'label' ...).
This is just one of the many problems of this combination of older and newer interfaces. Please focus on the more narrow methods (everything without $termType) if possible.
code dealing with prefetching, buffering, resolving different repos […] should all this code be reimplemented for ElasticSearch implementation of prefetchTerms or can it be somehow reused? All that code relies on TermBuffer/TermIndex now.
We might need to split the existing implementations. But I believe it is easier to start over and not touch the existing implementations. For example, you can start by implementing nothing but a TermLookup. If this works, you can add a small TermBuffer implementation on top that allows for prefetching. Since TermBuffer alone is not very useful, you might need to use the PrefetchingTermLookup interface as a decorator instead:
class PrefetchingElasticTermLookup implements PrefetchingTermLookup {
private $lookup; public function __construct( ElasticTermLookup $lookup ) { $this->lookup = $lookup; } public function prefetchTerms( array $entityIds, array $termTypes = null, array $languageCodes = null ) { /* TBD */ } public function getPrefetchedTerm( EntityId $entityId, $termType, $languageCode ) { switch ( $termType ) { case TermIndexEntry::TYPE_LABEL: return $this->getLabel( $entityId, $languageCode ); case TermIndexEntry::TYPE_DESCRIPTION: return $this->getDescription( $entityId, $languageCode ); default: throw new \InvalidArgumentException( "Not defined for \$termType \"$termType\"" ); } } public function getLabel( EntityId $entityId, $languageCode ) { $this->lookup->getLabel( $entityId, $languageCode ); } public function getLabels( EntityId $entityId, array $languageCodes ) { $this->lookup->getLabels( $entityId, $languageCodes ); } public function getDescription( EntityId $entityId, $languageCode ) { $this->lookup->getDescription( $entityId, $languageCode ); } public function getDescriptions( EntityId $entityId, array $languageCodes ) { $this->lookup->getDescriptions( $entityId, $languageCodes ); } }
TermLookup methods are supposed to be dealing with entities from federated repos too, and also working on the WikibaseClient side […]
This might indeed be needed. However, I suggest to not care for these for the moment. We can add such functionality later via dispatchers, decorators, and such.
Cc: WMDE-leszek, Smalyshev, hoo, Liuxinyu970226, Aklapper, aude, JeroenDeDauw, Tobi_WMDE_SW, thiemowmde, adrianheine, Lydia_Pintscher, Ricordisamoa, daniel, Lahi, Gq86, Darkminds3113, GoranSMilovanovic, QZanden, LawExplorer, Vali.matei, Volker_E, Wikidata-bugs, GWicke, Mbch331, Jay8g
_______________________________________________ Wikidata-bugs mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
