From 2240ea1f316156f3cb4475ea23a16246c999b6f0 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Sun, 12 Aug 2012 12:00:37 +0200 Subject: first start at refactoring the subscription system BROKEN This introduces a class for nicer wrapping and easier testing. Some functions were changed to provide nicer APIs (no throwing around of unescaped regexps) and to simplify things (hopefully). The refactoring isn't completed yet, so this will break the subscription system. The goal is to move as much subscription related stuff to this class as possible. Currently there is some code in lib/exe/indexer.php and maybe elsewhere (common.php?). Additionally everything should be covered by tests. A few tests are included here already. --- _test/tests/inc/subscription.test.php | 121 ++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 _test/tests/inc/subscription.test.php (limited to '_test/tests/inc/subscription.test.php') diff --git a/_test/tests/inc/subscription.test.php b/_test/tests/inc/subscription.test.php new file mode 100644 index 000000000..77bf3e830 --- /dev/null +++ b/_test/tests/inc/subscription.test.php @@ -0,0 +1,121 @@ +buildregex($test[0], $test[1], $test[2]); + $this->assertFalse(empty($re), "test line $row"); + $result = preg_grep($re, $data); + $this->assertEquals($test[3], count($result), "test line $row. $re got\n".print_r($result, true)); + + $row++; + } + } + + function test_addremove(){ + $sub = new MockupSubscription(); + + // no subscriptions + $this->assertArrayNotHasKey( + 'wiki:dokuwiki', + $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + ); + + // add page subscription + $sub->add('wiki:dokuwiki', 'testuser', 'every'); + + // one subscription + $this->assertArrayHasKey( + 'wiki:dokuwiki', + $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + ); + + // remove page subscription + $sub->remove('wiki:dokuwiki', 'testuser'); + + // no subscription + $this->assertArrayNotHasKey( + 'wiki:dokuwiki', + $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + ); + + // add namespace subscription + $sub->add('wiki:', 'testuser', 'every'); + + // one subscription + $this->assertArrayHasKey( + 'wiki:', + $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + ); + + // remove (non existing) page subscription + $sub->remove('wiki:dokuwiki', 'testuser'); + + // still one subscription + $this->assertArrayHasKey( + 'wiki:', + $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + ); + + // change namespace subscription + $sub->add('wiki:', 'testuser', 'digest', '1234567'); + + // still one subscription + $this->assertArrayHasKey( + 'wiki:', + $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + ); + + // check contents + $this->assertEquals( + array('wiki:' => array('testuser' => array('digest', '1234567'))), + $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + ); + } + + + +} + +/** + * makes protected methods visible for testing + */ +class MockupSubscription extends Subscription { + public function buildregex($user = null, $style = null, $data = null) { + return parent::buildregex($user, $style, $data); + } +} + +//Setup VIM: ex: et ts=4 : -- cgit v1.2.3 From adec979fd5453cf213b776d7dceaaaac4eb05713 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Sun, 12 Aug 2012 15:07:03 +0200 Subject: more subscription refactoring BROKEN now the actual sending of bulk messages (digest, list) is reimplemented and partially tested. Still not complete --- _test/tests/inc/subscription.test.php | 97 +++++++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 15 deletions(-) (limited to '_test/tests/inc/subscription.test.php') diff --git a/_test/tests/inc/subscription.test.php b/_test/tests/inc/subscription.test.php index 77bf3e830..b3d49a533 100644 --- a/_test/tests/inc/subscription.test.php +++ b/_test/tests/inc/subscription.test.php @@ -2,7 +2,7 @@ class subscription_test extends DokuWikiTest { - function test_regexp(){ + function test_regexp() { // data to test against $data = array( "casper every\n", @@ -19,13 +19,13 @@ class subscription_test extends DokuWikiTest { array('casper', null, null, 1), array('nope', null, null, 0), array('lights', null, null, 0), - array(array('Cold Fusion','casper','nope'), null, null, 2), + array(array('Cold Fusion', 'casper', 'nope'), null, null, 2), array(null, 'list', null, 1), array(null, 'every', null, 2), array(null, 'digest', null, 3), array(null, array('list', 'every'), null, 3), array('casper', 'digest', null, 0), - array('casper', array('digest','every'), null, 1), + array('casper', array('digest', 'every'), null, 1), array('zioth', 'list', '1344691369', 1), array('zioth', null, '1344691369', 1), array('zioth', 'digest', '1344691369', 0), @@ -34,7 +34,7 @@ class subscription_test extends DokuWikiTest { $sub = new MockupSubscription(); $row = 0; - foreach($tests as $test){ + foreach($tests as $test) { $re = $sub->buildregex($test[0], $test[1], $test[2]); $this->assertFalse(empty($re), "test line $row"); $result = preg_grep($re, $data); @@ -44,13 +44,13 @@ class subscription_test extends DokuWikiTest { } } - function test_addremove(){ + function test_addremove() { $sub = new MockupSubscription(); // no subscriptions $this->assertArrayNotHasKey( 'wiki:dokuwiki', - $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) ); // add page subscription @@ -59,7 +59,7 @@ class subscription_test extends DokuWikiTest { // one subscription $this->assertArrayHasKey( 'wiki:dokuwiki', - $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) ); // remove page subscription @@ -68,7 +68,7 @@ class subscription_test extends DokuWikiTest { // no subscription $this->assertArrayNotHasKey( 'wiki:dokuwiki', - $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) ); // add namespace subscription @@ -77,7 +77,7 @@ class subscription_test extends DokuWikiTest { // one subscription $this->assertArrayHasKey( 'wiki:', - $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) ); // remove (non existing) page subscription @@ -86,7 +86,7 @@ class subscription_test extends DokuWikiTest { // still one subscription $this->assertArrayHasKey( 'wiki:', - $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) ); // change namespace subscription @@ -95,17 +95,66 @@ class subscription_test extends DokuWikiTest { // still one subscription $this->assertArrayHasKey( 'wiki:', - $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) ); // check contents $this->assertEquals( array('wiki:' => array('testuser' => array('digest', '1234567'))), - $sub->subscribers('wiki:dokuwiki', null, array('every','list','digest')) + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) + ); + + // change subscription data + $sub->add('wiki:', 'testuser', 'digest', '7654321'); + + // still one subscription + $this->assertArrayHasKey( + 'wiki:', + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) + ); + + // check contents + $this->assertEquals( + array('wiki:' => array('testuser' => array('digest', '7654321'))), + $sub->subscribers('wiki:dokuwiki', null, array('every', 'list', 'digest')) ); } + function test_bulkdigest(){ + $sub = new MockupSubscription(); + + // let's start with nothing + $this->assertEquals(0, $sub->send_bulk('sub1:test')); + + // create a subscription + $sub->add('sub1:', 'testuser', 'digest', '978328800'); // last mod 2001-01-01 + + // now create change + $_SERVER['REMOTE_USER'] = 'someguy'; + saveWikiText('sub1:test', 'foo bar', 'a subscription change', false); + // should trigger a mail + $this->assertEquals(1, $sub->send_bulk('sub1:test')); + $this->assertEquals(array('arthur@example.com'), $sub->mails); + + $sub->reset(); + + // now create more changes + $_SERVER['REMOTE_USER'] = 'someguy'; + saveWikiText('sub1:sub2:test', 'foo bar', 'a subscription change', false); + saveWikiText('sub1:another_test', 'foo bar', 'a subscription change', false); + + // should not trigger a mail, because the subscription time has not been reached, yet + $this->assertEquals(0, $sub->send_bulk('sub1:test')); + $this->assertEquals(array(), $sub->mails); + + // reset the subscription time + $sub->add('sub1:', 'testuser', 'digest', '978328800'); // last mod 2001-01-01 + + // we now should get mails for three changes + $this->assertEquals(3, $sub->send_bulk('sub1:test')); + $this->assertEquals(array('arthur@example.com','arthur@example.com','arthur@example.com'), $sub->mails); + } } @@ -113,9 +162,27 @@ class subscription_test extends DokuWikiTest { * makes protected methods visible for testing */ class MockupSubscription extends Subscription { - public function buildregex($user = null, $style = null, $data = null) { - return parent::buildregex($user, $style, $data); - } + public $mails; // we keep sent mails here + + public function __construct(){ + $this->reset(); + } + + /** + * resets the mail array + */ + public function reset(){ + $this->mails = array(); + } + + public function buildregex($user = null, $style = null, $data = null) { + return parent::buildregex($user, $style, $data); + } + + protected function send($subscriber_mail, $replaces, $subject, $id, $template){ + $this->mails[] = $subscriber_mail; + return true; + } } //Setup VIM: ex: et ts=4 : -- cgit v1.2.3 From 8c7bcacde38ed7306d14a7e5769d6d1f2f8b9a21 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Sun, 12 Aug 2012 16:50:37 +0200 Subject: added list test --- _test/tests/inc/subscription.test.php | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) (limited to '_test/tests/inc/subscription.test.php') diff --git a/_test/tests/inc/subscription.test.php b/_test/tests/inc/subscription.test.php index b3d49a533..be2e68b74 100644 --- a/_test/tests/inc/subscription.test.php +++ b/_test/tests/inc/subscription.test.php @@ -156,6 +156,43 @@ class subscription_test extends DokuWikiTest { $this->assertEquals(array('arthur@example.com','arthur@example.com','arthur@example.com'), $sub->mails); } + function test_bulklist(){ + $sub = new MockupSubscription(); + + // let's start with nothing + $this->assertEquals(0, $sub->send_bulk('sub1:test')); + + // create a subscription + $sub->add('sub1:', 'testuser', 'list', '978328800'); // last mod 2001-01-01 + + // now create change + $_SERVER['REMOTE_USER'] = 'someguy'; + saveWikiText('sub1:test', 'foo bar', 'a subscription change', false); + + // should trigger a mail + $this->assertEquals(1, $sub->send_bulk('sub1:test')); + $this->assertEquals(array('arthur@example.com'), $sub->mails); + + $sub->reset(); + + // now create more changes + $_SERVER['REMOTE_USER'] = 'someguy'; + saveWikiText('sub1:sub2:test', 'foo bar', 'a subscription change', false); + saveWikiText('sub1:another_test', 'foo bar', 'a subscription change', false); + + // should not trigger a mail, because the subscription time has not been reached, yet + $this->assertEquals(0, $sub->send_bulk('sub1:test')); + $this->assertEquals(array(), $sub->mails); + + // reset the subscription time + $sub->add('sub1:', 'testuser', 'list', '978328800'); // last mod 2001-01-01 + + // we now should get a single mail for all three changes + $this->assertEquals(1, $sub->send_bulk('sub1:test')); + $this->assertEquals(array('arthur@example.com'), $sub->mails); + } + + } /** -- cgit v1.2.3 From 84c1127cc070777c8cbcf488f5422bc4b71470a8 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Sun, 12 Aug 2012 17:30:01 +0200 Subject: correctly check if subscriptions are enabled --- _test/tests/inc/subscription.test.php | 4 ++++ 1 file changed, 4 insertions(+) (limited to '_test/tests/inc/subscription.test.php') diff --git a/_test/tests/inc/subscription.test.php b/_test/tests/inc/subscription.test.php index be2e68b74..80548160a 100644 --- a/_test/tests/inc/subscription.test.php +++ b/_test/tests/inc/subscription.test.php @@ -212,6 +212,10 @@ class MockupSubscription extends Subscription { $this->mails = array(); } + public function isenabled(){ + return true; + } + public function buildregex($user = null, $style = null, $data = null) { return parent::buildregex($user, $style, $data); } -- cgit v1.2.3 From 2ed38036a53a489d2fcadc46ce601f8c876fca31 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Fri, 21 Sep 2012 11:53:17 +0200 Subject: consolidate more notification code in subscription class This is untested and probably broken currently --- _test/tests/inc/subscription.test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to '_test/tests/inc/subscription.test.php') diff --git a/_test/tests/inc/subscription.test.php b/_test/tests/inc/subscription.test.php index 80548160a..c6f33dec7 100644 --- a/_test/tests/inc/subscription.test.php +++ b/_test/tests/inc/subscription.test.php @@ -220,7 +220,7 @@ class MockupSubscription extends Subscription { return parent::buildregex($user, $style, $data); } - protected function send($subscriber_mail, $replaces, $subject, $id, $template){ + protected function send($subscriber_mail, $subject, $id, $template, $trep, $hrep){ $this->mails[] = $subscriber_mail; return true; } -- cgit v1.2.3 From 6686e3d1ac97a3c4d256f4e9df7f772102baed96 Mon Sep 17 00:00:00 2001 From: Andreas Gohr Date: Fri, 18 Jan 2013 10:40:49 +0100 Subject: fixed tests --- _test/tests/inc/subscription.test.php | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) (limited to '_test/tests/inc/subscription.test.php') diff --git a/_test/tests/inc/subscription.test.php b/_test/tests/inc/subscription.test.php index c6f33dec7..333400576 100644 --- a/_test/tests/inc/subscription.test.php +++ b/_test/tests/inc/subscription.test.php @@ -120,7 +120,7 @@ class subscription_test extends DokuWikiTest { ); } - function test_bulkdigest(){ + function test_bulkdigest() { $sub = new MockupSubscription(); // let's start with nothing @@ -153,10 +153,10 @@ class subscription_test extends DokuWikiTest { // we now should get mails for three changes $this->assertEquals(3, $sub->send_bulk('sub1:test')); - $this->assertEquals(array('arthur@example.com','arthur@example.com','arthur@example.com'), $sub->mails); + $this->assertEquals(array('arthur@example.com', 'arthur@example.com', 'arthur@example.com'), $sub->mails); } - function test_bulklist(){ + function test_bulklist() { $sub = new MockupSubscription(); // let's start with nothing @@ -192,7 +192,24 @@ class subscription_test extends DokuWikiTest { $this->assertEquals(array('arthur@example.com'), $sub->mails); } + /** + * Tests, if overwriting subscriptions works even when subscriptions for the same + * user exist for two nested namespaces, this is a test for the bug described in FS#2580 + */ + function test_overwrite() { + $sub = new MockupSubscription(); + $sub->add(':', 'admin', 'digest', '123456789'); + $sub->add(':wiki:', 'admin', 'digest', '123456789'); + $sub->add(':', 'admin', 'digest', '1234'); + $sub->add(':wiki:', 'admin', 'digest', '1234'); + + $subscriptions = $sub->subscribers(':wiki:', 'admin'); + + $this->assertCount(1, $subscriptions[':'], 'More than one subscription saved for the root namespace even though the old one should have been overwritten.'); + $this->assertCount(1, $subscriptions[':wiki:'], 'More than one subscription saved for the wiki namespace even though the old one should have been overwritten.'); + $this->assertCount(2, $subscriptions, 'Didn\'t find the expected two subscriptions'); + } } /** @@ -201,18 +218,18 @@ class subscription_test extends DokuWikiTest { class MockupSubscription extends Subscription { public $mails; // we keep sent mails here - public function __construct(){ + public function __construct() { $this->reset(); } /** * resets the mail array */ - public function reset(){ + public function reset() { $this->mails = array(); } - public function isenabled(){ + public function isenabled() { return true; } @@ -220,7 +237,7 @@ class MockupSubscription extends Subscription { return parent::buildregex($user, $style, $data); } - protected function send($subscriber_mail, $subject, $id, $template, $trep, $hrep){ + protected function send($subscriber_mail, $subject, $id, $template, $trep, $hrep = null) { $this->mails[] = $subscriber_mail; return true; } -- cgit v1.2.3