Skip to content

Commit 0ca2f4c

Browse files
jenkins-botGerrit Code Review
authored andcommitted
Merge "Revert^2 "Implement new usage types for statement with qualifiers and references""
2 parents 6809e9f + d367fa4 commit 0ca2f4c

20 files changed

+632
-42
lines changed

client/includes/Changes/AffectedPagesFinder.php

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,20 @@ public function getChangedAspects( EntityChange $change ) {
141141
$aspects = array_merge( $aspects, $aliasAspects );
142142
}
143143

144-
if ( $diffAspects->getStatementChanges() !== [] ) {
145-
$statementAspects = $this->getChangedStatementAspects(
146-
$diffAspects->getStatementChanges()
144+
// If a statement mainsnak changes, we need to tell any pages which have STATEMENT_USAGE or STATEMENT_WITH_QUAL_OR_REF_USAGE
145+
// This is regardless of whether it also includes a reference/qualifer change, in addition to the mainSnak change
146+
if ( $diffAspects->getStatementChangesExcludingQualOrRefOnly() !== [] ) {
147+
$statementAspects = $this->getChangedStatementAspects( //tells STATEMENT_USAGE and STATEMENT_WITH_QUAL_OR_REF_USAGE
148+
$diffAspects->getStatementChangesExcludingQualOrRefOnly()
149+
);
150+
$aspects = array_merge( $aspects, $statementAspects );
151+
}
152+
153+
// If it is a qualifer or ref only change (no mainsnak change), we don't need to tell those with STATEMENT_USAGE, but those with
154+
// STATEMENT_WITH_QUAL_OR_REF_USAGE
155+
if ( $diffAspects->getStatementChangesQualOrRefOnly() !== [] ) {
156+
$statementAspects = $this->getChangedStatementAspectsQualOrRefOnlyChange( //only tells STATEMENT_WITH_QUAL_OR_REF_USAGE
157+
$diffAspects->getStatementChangesQualOrRefOnly()
147158
);
148159
$aspects = array_merge( $aspects, $statementAspects );
149160
}
@@ -164,15 +175,39 @@ public function getChangedAspects( EntityChange $change ) {
164175
* @param string[] $diff
165176
*
166177
* @return string[]
178+
*
179+
* If a statement changes, we need to tell any pages which have STATEMENT_USAGE or STATEMENT_WITH_QUAL_OR_REF_USAGE
167180
*/
168181
private function getChangedStatementAspects( array $diff ) {
169182
$aspects = [];
170183

171184
foreach ( $diff as $propertyId ) {
172185
$aspects[] = EntityUsage::makeAspectKey( EntityUsage::STATEMENT_USAGE, $propertyId );
186+
$aspects[] = EntityUsage::makeAspectKey( EntityUsage::STATEMENT_WITH_QUAL_OR_REF_USAGE, $propertyId );
173187
}
174188

175189
$aspects[] = EntityUsage::makeAspectKey( EntityUsage::STATEMENT_USAGE );
190+
$aspects[] = EntityUsage::makeAspectKey( EntityUsage::STATEMENT_WITH_QUAL_OR_REF_USAGE );
191+
192+
return $aspects;
193+
}
194+
195+
/**
196+
* @param string[] $diff
197+
*
198+
* @return string[]
199+
*
200+
* If any statement's qual/ref changes, we need to tell any pages which have STATEMENT_WITH_QUAL_OR_REF_USAGE,
201+
* but do not need to tell those with STATEMENT_USAGE
202+
*/
203+
private function getChangedStatementAspectsQualOrRefOnlyChange( array $diff ) {
204+
$aspects = [];
205+
206+
foreach ( $diff as $propertyId ) {
207+
$aspects[] = EntityUsage::makeAspectKey( EntityUsage::STATEMENT_WITH_QUAL_OR_REF_USAGE, $propertyId );
208+
}
209+
210+
$aspects[] = EntityUsage::makeAspectKey( EntityUsage::STATEMENT_WITH_QUAL_OR_REF_USAGE );
176211

177212
return $aspects;
178213
}

client/includes/DataAccess/Scribunto/WikibaseEntityLibrary.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public function getUsageAccumulator(): UsageAccumulator {
161161
}
162162

163163
/**
164-
* Add a statement usage (called once specific statements are accessed).
164+
* Add a statement usage without qualifiers and/or references (called once specific statements are accessed).
165165
*
166166
* @param string $entityId The Entity from which the statements were accessed.
167167
* @param string $propertyId Property id of the statements accessed.
@@ -170,6 +170,16 @@ public function addStatementUsage( string $entityId, string $propertyId ): void
170170
$this->getImplementation()->addStatementUsage( $entityId, $propertyId );
171171
}
172172

173+
/**
174+
* Add a statement usage with qualifiers and/or references as well as mainsnak (called once specific statements are accessed).
175+
*
176+
* @param string $entityId The Entity from which the statements were accessed.
177+
* @param string $propertyId Property id of the statements accessed.
178+
*/
179+
public function addStatementWithQualOrRefUsage( string $entityId, string $propertyId ): void {
180+
$this->getImplementation()->addStatementWithQualOrRefUsage( $entityId, $propertyId );
181+
}
182+
173183
/**
174184
* Add a label usage (called once specific labels are accessed).
175185
*
@@ -232,6 +242,7 @@ public function register() {
232242
'formatStatements' => [ $this, 'formatStatements' ],
233243
'formatPropertyValues' => [ $this, 'formatPropertyValues' ],
234244
'addStatementUsage' => [ $this, 'addStatementUsage' ],
245+
'addStatementWithQualOrRefUsage' => [ $this, 'addStatementWithQualOrRefUsage' ],
235246
'addLabelUsage' => [ $this, 'addLabelUsage' ],
236247
'addDescriptionUsage' => [ $this, 'addDescriptionUsage' ],
237248
'addTitleOrSiteLinksUsage' => [ $this, 'addTitleOrSiteLinksUsage' ],

client/includes/DataAccess/Scribunto/WikibaseLibrary.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use Wikibase\DataModel\Entity\EntityId;
2424
use Wikibase\DataModel\Entity\EntityIdParser;
2525
use Wikibase\DataModel\Entity\EntityIdParsingException;
26+
use Wikibase\DataModel\Entity\NumericPropertyId;
2627
use Wikibase\DataModel\Entity\PropertyId;
2728
use Wikibase\DataModel\Services\Lookup\EntityAccessLimitException;
2829
use Wikibase\DataModel\Services\Lookup\EntityRetrievingClosestReferencedEntityIdLookup;
@@ -352,6 +353,7 @@ public function register() {
352353
'entityExists' => [ $this, 'entityExists' ],
353354
'getBadges' => [ $this, 'getBadges' ],
354355
'getEntityStatements' => [ $this, 'getEntityStatements' ],
356+
'addStatementWithQualOrRefUsage' => [ $this, 'addStatementWithQualOrRefUsage' ],
355357
'getEntityUrl' => [ $this, 'getEntityUrl' ],
356358
'renderSnak' => [ $this, 'renderSnak' ],
357359
'formatValue' => [ $this, 'formatValue' ],
@@ -495,6 +497,20 @@ public function getEntityStatements( string $prefixedEntityId, string $propertyI
495497
return [ $statements ];
496498
}
497499

500+
/**
501+
* Add a statement usage (called once specific statements are accessed) when the qualifiers and/or references are also accessed.
502+
*
503+
* @param string $entityId The Entity from which the statements were accessed.
504+
* @param string $propertyId Property id of the statements accessed.
505+
*/
506+
public function addStatementWithQualOrRefUsage( string $entityId, string $propertyId ): void {
507+
$entityId = trim( $entityId );
508+
$entityId = $this->entityIdParser->parse( $entityId );
509+
$propertyId = new NumericPropertyId( $propertyId );
510+
511+
$this->usageAccumulator->addStatementWithQualOrRefUsage( $entityId, $propertyId );
512+
}
513+
498514
/**
499515
* Wrapper for getEntityId in WikibaseLanguageIndependentLuaBindings
500516
*/

client/includes/DataAccess/Scribunto/WikibaseLuaEntityBindings.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,27 @@ public function formatStatements(
9696
* @param string $propertyId Property id of the statements accessed.
9797
*/
9898
public function addStatementUsage( string $entityId, string $propertyId ): void {
99+
$entityId = trim( $entityId );
99100
$entityId = $this->entityIdParser->parse( $entityId );
100101
$propertyId = new NumericPropertyId( $propertyId );
101102

102103
$this->usageAccumulator->addStatementUsage( $entityId, $propertyId );
103104
}
104105

106+
/**
107+
* Add a statement usage (called once specific statements are accessed) when the qualifiers and/or references are also accessed.
108+
*
109+
* @param string $entityId The Entity from which the statements were accessed.
110+
* @param string $propertyId Property id of the statements accessed.
111+
*/
112+
public function addStatementWithQualOrRefUsage( string $entityId, string $propertyId ): void {
113+
$entityId = trim( $entityId );
114+
$entityId = $this->entityIdParser->parse( $entityId );
115+
$propertyId = new NumericPropertyId( $propertyId );
116+
117+
$this->usageAccumulator->addStatementWithQualOrRefUsage( $entityId, $propertyId );
118+
}
119+
105120
/**
106121
* Add a label usage (called once specific labels are accessed).
107122
*

client/includes/DataAccess/Scribunto/mw.wikibase.entity.lua

Lines changed: 98 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,28 @@ local function isValidPropertyId( propertyId )
4949
return type( propertyId ) == 'string' and propertyId:match( '^P[1-9]%d*$' )
5050
end
5151

52-
-- Log access to claims of entity
52+
-- This should only ever change during the clone operation of getEntityStatements
53+
local qualOrRefUsageTrackingEnabled = true
54+
55+
-- Log access to claims of entity with qualifiers and/or references
56+
--
57+
-- @param {string} entityId
58+
-- @param {string} propertyId
59+
local function addStatementWithQualOrRefUsage( entityId, propertyId )
60+
if isValidPropertyId( propertyId ) and qualOrRefUsageTrackingEnabled == true then
61+
-- Only attempt to track the usage if we have a valid property id.
62+
php.addStatementWithQualOrRefUsage( entityId, propertyId )
63+
end
64+
end
65+
66+
-- Log access to claims of entity mainsnak only
5367
--
5468
-- @param {string} entityId
5569
-- @param {string} propertyId
5670
local function addStatementUsage( entityId, propertyId )
5771
if isValidPropertyId( propertyId ) then
5872
-- Only attempt to track the usage if we have a valid property id.
59-
php.addStatementUsage( entityId, propertyId ) --add an argument here??
73+
php.addStatementUsage( entityId, propertyId )
6074
end
6175
end
6276

@@ -104,11 +118,84 @@ local function maskEntityTable( entity, tableName, usageFunc )
104118
setmetatable( entity[tableName], pseudoTableMetatable )
105119
end
106120

121+
-- Function to mask a statement's subtables in order to log access
122+
-- Code for logging based on: http://www.lua.org/pil/13.4.4.html
123+
--
124+
-- @param {table} statement
125+
-- @param {string} tableName
126+
-- @param {function} usageFunc
127+
-- @param {string} entityId
128+
-- @param {string} propertyId
129+
local function maskStatementTable( statement, tableName, usageFunc, entityId, propertyId )
130+
if statement[tableName] == nil then
131+
return
132+
end
133+
134+
local actualStatementTable = statement[tableName]
135+
statement[tableName] = {}
136+
137+
local function logNext( _, key )
138+
local k, v = next( actualStatementTable, key )
139+
if k ~= nil then
140+
usageFunc( entityId, propertyId )
141+
end
142+
return k, v
143+
end
144+
145+
local function ipairsfunc( _ , i )
146+
i = i + 1
147+
if actualStatementTable[i] ~= nil then
148+
usageFunc( entityId, propertyId )
149+
return i, actualStatementTable[i]
150+
end
151+
return -- no nil to match default ipairs()
152+
end
153+
154+
local pseudoTableMetatable = {
155+
__index = function( _, key )
156+
-- note: we do not specify string here like in maskEntityTable, because
157+
-- sometimes the key is e.g. P1 e.g. for qualifier
158+
-- sometimes the key is a number e.g. for references
159+
usageFunc( entityId, propertyId )
160+
return actualStatementTable[key]
161+
end,
162+
163+
__newindex = function (table,key,value)
164+
if (table[key] == actualStatementTable[key]) then
165+
return
166+
--note: this seems to happen in getBestStatements but the values match so no update needed
167+
else
168+
error( 'Statement table cannot be modified', 2 )
169+
end
170+
end,
171+
172+
__pairs = function( _ )
173+
return logNext, {}, nil
174+
end,
175+
176+
__ipairs = function( _ )
177+
return ipairsfunc, {}, 0
178+
end,
179+
}
180+
181+
setmetatable( statement[tableName], pseudoTableMetatable )
182+
end
183+
107184
-- Function to mask an entity's subtables in order to log access and prevent modifications
108185
--
109186
-- @param {table} entity
110187
local function maskEntityTables( entity )
111-
maskEntityTable( entity, 'claims', addStatementUsage )
188+
-- within claim we need to mask subtables for quals and refs too so they can be tracked
189+
if entity.claims then
190+
for propertyId, statements in pairs(entity.claims) do -- for each property e.g. P1
191+
for _, statement in ipairs(statements) do
192+
maskStatementTable(statement, "qualifiers", addStatementWithQualOrRefUsage, entity.id, propertyId)
193+
maskStatementTable(statement, "references", addStatementWithQualOrRefUsage, entity.id, propertyId)
194+
end
195+
end
196+
end
197+
198+
maskEntityTable( entity, 'claims', addStatementUsage )
112199
maskEntityTable( entity, 'labels', php.addLabelUsage )
113200
maskEntityTable( entity, 'sitelinks', php.addTitleOrSiteLinksUsage )
114201
maskEntityTable( entity, 'descriptions', php.addDescriptionUsage )
@@ -266,8 +353,15 @@ local function getEntityStatements( entity, propertyLabelOrId, funcName )
266353
end
267354

268355
if propertyId and entity.claims[propertyId] then
356+
357+
qualOrRefUsageTrackingEnabled = false
269358
-- Create a deep copy of the table to prevent unexpected value changes (T270851).
270-
return mw.clone(entity.claims[propertyId])
359+
-- The clone operation is considered a read of qualifiers and references so we need to
360+
-- switch off tracking just during the clone operation
361+
local clone = mw.clone(entity.claims[propertyId])
362+
qualOrRefUsageTrackingEnabled = true
363+
364+
return clone
271365
end
272366

273367
return {}

0 commit comments

Comments
 (0)