(Bug 59204) Properties do not unset on page deletion #71
Conversation
Clean-up of related subjects to a property has to be done before store data are modified which means that either run UpdateDispatcherJob before the deleteSubject() in "online" mode or put both UpdateDispatcherJob and deleteSubject() in a DeferredSubjectRemovalJob to ensure synchronize execution.
|
Tests are failing because of SQLite, unittest_unittest_smw_fpt_mdat SMW\Test\MwDatabaseSQLStoreIntegrationTest::testAfterPageCreation_StoreHasDataToRefreshWithoutJobs with data set #0 (0, 'withInterWiki', 'foo') |
$db->tableName( $proptable->getName() ) returns unittest_unittest on SQLite which causes test to fail. https://s3.amazonaws.com/archive.travis-ci.org/jobs/16315316/log.txt
|
Running a var_dump check reveals that for "$from unittest_smw_fpt_mdat getName smw_fpt_mdat" $db->select( $from, '*', $where, 'SMW::getProperties', array( 'LIMIT' => 1 ) ); returns with Query: SELECT * FROM unittest_unittest_smw_fpt_mdat WHERE s_id='4' LIMIT 1 on SQLite which of course fails with "Error: 1 no such table: unittest_unittest_smw_fpt_mdat" because of unittest_unittest_smw_fpt_mdat https://s3.amazonaws.com/archive.travis-ci.org/jobs/16318048/log.txt |
This method really should be its own class. Also not sure why this just happened for sqlite. Perhaps the affected branch of the if is just generally broken and another bug that is sqlite specific caused it to be executed for sqlite?
Fix double prefixing issue in SMWSQLStore3Readers::getProperties
|
Looks like I fixed the issue you ran into. Not sure about the original bug though. Feel free to merge if you think all is fine. |
Clean-up of related subjects to a property has to be done before store data are modified which means that either run UpdateDispatcherJob before the deleteSubject() in "online" mode or put both UpdateDispatcherJob and deleteSubject() in a DeferredSubjectRemovalJob to ensure synchronize execution.
$db->tableName( $proptable->getName() ) returns unittest_unittest on SQLite which causes test to fail. https://s3.amazonaws.com/archive.travis-ci.org/jobs/16315316/log.txt
This method really should be its own class. Also not sure why this just happened for sqlite. Perhaps the affected branch of the if is just generally broken and another bug that is sqlite specific caused it to be executed for sqlite?
|
Thanks for the sqlite fix. I'll want to have a feedback first from the bug reporter before merging this and I want to extend the test coverage after I got a feedback. |
ghost
assigned
|
Well I knew from the start that this has do go into a Job but I didn't want to waste my time in the first place if it wouldn't solve the issue reported, now that it seems that the issue was actually resolved I just made sure that the legacy behavior continues to work as before and for those who want a proper update of related entities can adopt the settings. When the clean-up is initiated, it should preferably done as DeferredJob otherwise the chance that one runs into a runtime error is probable because finding the appropriate data before store->deleteSubject() can be expensive (we don't have any revision information about stored entity data therefore this has to be done synchronously). |
|
OK, in this case I will tag this with 1.9.0.1 |
|
Would have been nice to have #76 and this merged before 1.9 rel announce, in which case we could have linked it to 1.9.0.1. I'm however not comfortable with tagging 1.9.0.1 right after this gets merged without it first being tried by several users. Not a real issue though. |
|
Would be great if you could merge #76 into this PR btw, as currently it is not updating the rel notes, which is fixed by 76. |
CRAP: 14 CC: 100% The removal of a subject from the Store is being moved into a `DeleteSubjectJob` and if necessary can be scheduled as `DeferredJob` (enables to run deletion as background job). Prior SMW 1.9, the subject deletion did not initiate any clean-up of possible associate subjects, the `smwgDeleteSubjectWithAssociatesRefresh` setting will allow to refresh entities that are connected to a deleted subject. A subject may be connected to a large group of associates, it is therefore advisable that `smwgDeleteSubjectAsDeferredJob` is set TRUE in order to avoid any performance issues (or runtime errors) during the deletion process. In case the deletion is executed as `DeferredJob` it is suggested that the JobQueue is being run repeatedly within a narrow time frame to avoid an increased backlog. ### Legacy (default) setting $GLOBALS['smwgOnDeleteAction'] = array( 'smwgDeleteSubjectAsDeferredJob' => false, 'smwgDeleteSubjectWithAssociatesRefresh' => false );
…aWiki.git into bug
|
After this is merged, I'll have/add another MwJobWithSQLStoreIntegrationTest which runs a test against an actual JobQueue for DeleteSubjectJob. |
|
Fantastic. Done with review. Looks good, merging. |
(Bug 59204) Properties do not unset on page deletion
|
Just a heads up, when this is merged scrutinizer-ci will drop to 7.6 which is not caused by this change rather being related to "The rating algorithm used for inspecting the base and head index was different. This might result in rating changes which are unrelated to your code changes!". |
Clean-up of related subjects to a property has to be done before store data
are modified which means that either run UpdateDispatcherJob before the
deleteSubject() in "online" mode or put both UpdateDispatcherJob and deleteSubject()
in a DeferredSubjectRemovalJob to ensure synchronize execution.
https://bugzilla.wikimedia.org/show_bug.cgi?id=59204