webapp/postfixadmin
1
0
Fork 0
mirror of https://github.com/postfixadmin/postfixadmin.git synced 2025-07-31 10:04:20 +03:00
Code Activity

Refactor some methods

Browse Source 
PFAHandler::store() -> PFAHandler::save();
 PFAHandler::storemore() -> PFAHandler::postSave();
 PFAHandler::beforestore() -> PFAHandler::preSave();
This commit is contained in:
David Goodwin
2020-09-25 21:29:45 +01:00
parent 10c92da8c8
commit 3c7da4f3b8
15 changed files with 268 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
model/AdminHandler.php
View File

@ -123,7 +123,7 @@ class AdminHandler extends PFAHandler {
* called by $this->store() after storing $this->values in the database
* can be used to update additional tables, call scripts etc.
*/
protected function storemore() {
protected function postSave() : bool {
# store list of allowed domains in the domain_admins table
if (isset($this->values['domains'])) {
if (is_array($this->values['domains'])) {

2
model/AdminpasswordHandler.php
View File

@ -35,7 +35,7 @@ class AdminpasswordHandler extends PFAHandler {
);
}
public function init($id) {
public function init($id) :bool {
# hardcode to logged in admin
if ($this->admin_username == '') {
die("No admin logged in");

6
model/AliasHandler.php
View File

@ -136,7 +136,7 @@ class AliasHandler extends PFAHandler {
* AliasHandler needs some special handling in init() and therefore overloads the function.
* It also calls parent::init()
*/
public function init($id) {
public function init(string $id) : bool {
$bits = explode('@', $id);
if (sizeof($bits) == 2) {
$local_part = $bits[0];
@ -303,7 +303,7 @@ class AliasHandler extends PFAHandler {
$this->values['goto'] = join(',', $values['goto']);
}
protected function storemore() {
protected function postSave() : bool {
# TODO: if alias belongs to a mailbox, update mailbox active status
return true;
}
@ -357,7 +357,7 @@ class AliasHandler extends PFAHandler {
return array($condition, $searchmode);
}
public function getList($condition, $searchmode = array(), $limit=-1, $offset=-1) {
public function getList($condition, $searchmode = array(), $limit=-1, $offset=-1) : bool {
list($condition, $searchmode) = $this->condition_ignore_mailboxes($condition, $searchmode);
$this->set_is_mailbox_extrafrom($condition, $searchmode);
$result = parent::getList($condition, $searchmode, $limit, $offset);

2
model/AliasdomainHandler.php
View File

@ -51,7 +51,7 @@ class AliasdomainHandler extends PFAHandler {
}
}
public function init($id) {
public function init($id) : bool {
$success = parent::init($id);
if ($success) {
if (count($this->struct['alias_domain']['options']) == 0 && $this->new) {

4
model/DomainHandler.php
View File

@ -148,7 +148,7 @@ class DomainHandler extends PFAHandler {
}
protected function beforestore() {
protected function preSave() : bool {
# TODO: is this function superfluous? _can_edit should already cover this
if ($this->is_superadmin) {
return true;
@ -161,7 +161,7 @@ class DomainHandler extends PFAHandler {
* called by $this->store() after storing $this->values in the database
* can be used to update additional tables, call scripts etc.
*/
protected function storemore() {
protected function postSave() : bool {
if ($this->new && $this->values['default_aliases']) {
foreach (Config::read_array('default_aliases') as $address=>$goto) {
$address = $address . "@" . $this->id;

10
model/MailboxHandler.php
View File

@ -60,7 +60,7 @@ class MailboxHandler extends PFAHandler {
}
}
public function init($id) {
public function init($id) : bool {
if (!parent::init($id)) {
return false;
}
@ -217,7 +217,7 @@ class MailboxHandler extends PFAHandler {
}
protected function beforestore() {
protected function preSave() : bool {
if (isset($this->values['quota']) && $this->values['quota'] != -1 && is_numeric($this->values['quota'])) {
$multiplier = Config::read_string('quota_multiplier');
if ($multiplier == 0 || !is_numeric($multiplier)) { // or empty string, or null, or false...
@ -256,7 +256,7 @@ class MailboxHandler extends PFAHandler {
return false;
}
if (!$ah->store()) {
if (!$ah->save()) {
$this->errormsg[] = $ah->errormsg[0];
return false;
}
@ -292,7 +292,7 @@ class MailboxHandler extends PFAHandler {
return $ok && parent::set($values);
}
protected function storemore() {
protected function postSave() : bool {
if ($this->new) {
if (!$this->mailbox_post_script()) {
# return false; # TODO: should this be fatal?
@ -649,7 +649,7 @@ class MailboxHandler extends PFAHandler {
/**
* Called by storemore() after a mailbox has been created.
* Called by postSave() after a mailbox has been created.
* Immediately returns, unless configuration indicates
* that one or more sub-folders should be created.
*

24
model/PFAHandler.php
View File

@ -337,7 +337,7 @@ abstract class PFAHandler {
* initialize with $id and check if it is valid
* @param string $id
*/
public function init($id) {
public function init(string $id) : bool {
// postfix treats address lookups (aliases, mailboxes) as if they were lowercase.
// MySQL is normally case insenstive, PostgreSQL is case sensitive.
@ -526,7 +526,7 @@ abstract class PFAHandler {
}
/**
* store $this->values in the database
* save $this->values to the database
*
* converts values based on $this->struct[*][type] (boolean, password encryption)
*
@ -534,13 +534,13 @@ abstract class PFAHandler {
* @return bool - true if all values were stored in the database, otherwise false
* error messages (if any) are stored in $this->errormsg
*/
public function store() {
public function save() : bool {
if ($this->values_valid == false) {
$this->errormsg[] = "one or more values are invalid!";
return false;
}
if (!$this->beforestore()) {
if (!$this->preSave()) {
return false;
}
@ -584,9 +584,9 @@ abstract class PFAHandler {
return false;
}
$result = $this->storemore();
$result = $this->postSave();
# db_log() even if storemore() failed
# db_log() even if postSave() failed
db_log($this->domain, $this->msg['logname'], $this->id);
if ($result) {
@ -600,17 +600,17 @@ abstract class PFAHandler {
/**
* called by $this->store() before storing the values in the database
* @return bool - if false, store() will abort
* @return bool - if false, save() will abort
*/
protected function beforestore() {
protected function preSave() : bool {
return true; # do nothing, successfully ;-)
}
/**
* called by $this->store() after storing $this->values in the database
* called by $this->save() after storing $this->values in the database
* can be used to update additional tables, call scripts etc.
*/
protected function storemore() {
protected function postSave() : bool {
return true; # do nothing, successfully ;-)
}
@ -735,7 +735,7 @@ abstract class PFAHandler {
* @param int $offset - number of first row to return
* @return array - rows (as associative array, with the ID as key)
*/
protected function read_from_db($condition, $searchmode = array(), $limit=-1, $offset=-1) {
protected function read_from_db($condition, $searchmode = array(), $limit=-1, $offset=-1) : array {
$queryparts = $this->build_select_query($condition, $searchmode);
$query = $queryparts['select_cols'] . $queryparts['from_where_order'];
@ -800,7 +800,7 @@ abstract class PFAHandler {
* @return bool - always true, no need to check ;-) (if $result is not an array, getList die()s)
* The data is stored in $this->result (as array of rows, each row is an associative array of column => value)
*/
public function getList($condition, $searchmode = array(), $limit=-1, $offset=-1) {
public function getList($condition, $searchmode = array(), $limit=-1, $offset=-1) : bool {
if (is_array($condition)) {
$real_condition = array();
foreach ($condition as $key => $value) {

2
model/VacationHandler.php
View File

@ -279,7 +279,7 @@ class VacationHandler extends PFAHandler {
# TODO: supress logging in AliasHandler if called from VacationHandler (VacationHandler should log itsself)
if (!$handler->store()) {
if (!$handler->save()) {
print_r($handler->errormsg); # TODO: error handling
return false;
}

2
public/setup.php
View File

@ -458,7 +458,7 @@ function create_admin($values) {
return array(1, "", $handler->errormsg);
}
if (!$handler->store()) {
if (!$handler->save()) {
return array(1, "", $handler->errormsg);
}

2
public/users/edit-alias.php
View File

@ -121,7 +121,7 @@ if ($_SERVER['REQUEST_METHOD'] == "POST") {
flash_error($errormsg[0]);
}
$updated = $ah->store();
$updated = $ah->save();
if ($updated) {
header("Location: main.php");

2
public/users/password-change.php
View File

@ -70,7 +70,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
$values = $handler->result;
$values['password'] = $fPassword;
$values['password2'] = $fPassword2;
if ($handler->set($values) && $handler->store()) {
if ($handler->set($values) && $handler->save()) {
flash_info(Config::lang_f('pPassword_result_success', $tUsername));
header('Location: main.php');
exit(0);

2
public/xmlrpc.php
View File

@ -176,7 +176,7 @@ class AliasProxy {
//error_log('ah->set failed' . print_r($values, true));
return false;
}
$store = $ah->store();
$store = $ah->save();
return $store;
}

293
tests/AliasHandlerTest.php
View File

@ -1,15 +1,21 @@
<?php
class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
public function testBasic() {
$x = new AliasHandler();
$list = $x->getList("");
$this->assertTrue($list);
$results = $x->result();
$this->assertEmpty($results);
class AliasHandlerTest extends \PHPUnit\Framework\TestCase
{
protected function setUp(): void
{
// Fake being an admin.
$_SESSION = [
'sessid' => [
'roles' => ['global-admin']
]
];
parent::setUp();
}
public function tearDown(): void {
protected function tearDown(): void
{
$_SESSION = [];
db_query('DELETE FROM alias');
db_query('DELETE FROM domain_admins');
@ -18,7 +24,18 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
parent::tearDown();
}
public function testCannotAddAliasUntilDomainIsThere() {
public function testBasic()
{
$x = new AliasHandler();
$list = $x->getList("");
$this->assertTrue($list);
$results = $x->result();
$this->assertEmpty($results);
}
public function testCannotAddAliasUntilDomainIsThere()
{
// Fake us being an admin.
@ -52,7 +69,8 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
/**
* @see https://github.com/postfixadmin/postfixadmin/pull/375 and https://github.com/postfixadmin/postfixadmin/issues/358
*/
public function testCannotAddAliasThatPointsToItself() {
public function testCannotAddAliasThatPointsToItself()
{
// Fake being an admin.
$_SESSION = [
'sessid' => [
@ -82,7 +100,7 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
$this->assertTrue($ret);
$ret = $dh->store();
$ret = $dh->save();
$this->assertTrue($ret);
@ -125,7 +143,7 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
$this->assertEquals(4, count($list)); // default aliases.
$x->set($values);
$x->store();
$x->save();
$this->assertNotEmpty($x->errormsg);
$this->assertEquals(
@ -135,61 +153,18 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
], $x->errormsg);
}
public function testAddingDataEtc() {
public function testAddingDataEtc()
{
// Fake being an admin.
$_SESSION = [
'sessid' => [
'roles' => ['global-admin']
]
];
// Add example.com
$dh = new DomainHandler(1, 'admin', true);
$dh->init('example.com');
$this->addDomain('example.com', 'admin');
$ret = $dh->set(
[
'domain' => 'example.com',
'description' => 'test domain',
'aliases' => 11,
'mailboxes' => 12,
'active' => 1,
'backupmx' => 0,
'default_aliases' => 1
]
);
$this->assertEmpty($dh->errormsg);
$this->assertEmpty($dh->infomsg);
$this->assertTrue($ret);
$ret = $dh->store();
$this->assertTrue($ret);
// Need to add 'admin' as a domain_admin
db_insert('domain_admins', ['username' => 'admin', 'domain' => 'example.com', 'created' => '2020-01-01', 'active' => 1], ['created'], true);
$dh = new DomainHandler(0, 'admin', true);
$dh->getList('');
$result = $dh->result();
$this->assertEmpty($dh->infomsg);
$this->assertEmpty($dh->errormsg);
$this->assertNotEmpty($result);
$this->assertEquals('example.com', $result['example.com']['domain']);
$this->assertEquals('test domain', $result['example.com']['description']);
$this->assertEquals(11, $result['example.com']['aliases']);
$this->assertEquals(12, $result['example.com']['mailboxes']); // default aliases.
$this->assertEquals(4, $result['example.com']['alias_count']); // default aliases.
$this->assertEquals(0, $result['example.com']['mailbox_count']);
$this->assertEquals(1, $result['example.com']['active']);
$x = new AliasHandler(1, 'admin', true);
@ -208,7 +183,7 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
$this->assertEquals(4, count($list)); // default aliases.
$x->set($values);
$x->store();
$x->save();
$x->getList('');
@ -230,4 +205,202 @@ class AliasHandlerTest extends \PHPUnit\Framework\TestCase {
$this->assertTrue($found, "check output : " . json_encode($list));
}
private function addDomain(string $domain, string $username): void
{
// Fake being an admin.
$_SESSION = [
'sessid' => [
'roles' => ['global-admin']
]
];
// Add example.com
$dh = new DomainHandler(1, $username, true);
$dh->init('example.com');
$ret = $dh->set(
[
'domain' => $domain,
'description' => 'test domain',
'aliases' => 11,
'mailboxes' => 12,
'active' => 1,
'backupmx' => 0,
'default_aliases' => 1
]
);
$this->assertEmpty($dh->errormsg);
$this->assertEmpty($dh->infomsg);
$this->assertTrue($ret);
$ret = $dh->save();
$this->assertTrue($ret);
// Need to add 'admin' as a domain_admin
db_insert('domain_admins', ['username' => $username, 'domain' => $domain, 'created' => '2020-01-01', 'active' => 1], ['created'], true);
$dh = new DomainHandler(0, $username, true);
$dh->getList('');
$result = $dh->result();
$this->assertEmpty($dh->infomsg);
$this->assertEmpty($dh->errormsg);
$this->assertNotEmpty($result);
$expected = [
'domain' => 'example.com',
'description' => 'test domain',
'aliases' => 11,
'alias_count' => 4,
'mailboxes' => 12,
'mailbox_count' => 0,
'backupmx' => 0,
'active' => 1,
];
foreach ($expected as $k => $v) {
$this->assertEquals($v, $result[$domain][$k]);
}
}
public function testYouCannotAddMoreAliasesThanTheDomainLimit()
{
$this->addDomain('example.com', 'admin');
// default limit is 11 aliases.... so it should exit once we get past that.
$dh = new DomainHandler(0, 'admin', true);
$this->assertTrue($dh->getList(''));
$result = $dh->result();
// alias count limit is 11.
$this->assertEquals(11, $result['example.com']['aliases']);
// 4 default aliases were added.
$this->assertEquals(4, $result['example.com']['alias_count']);
foreach (range(1, 7) as $char) {
$newAlias = $char . '-test@example.com';
$x = new AliasHandler(1, 'admin', true);
$values = [
'localpart' => explode('@', $newAlias)[0],
'domain' => 'example.com',
'active' => 1,
'address' => $newAlias,
'goto' => ['dest@example.com']
];
$r = $x->init($newAlias);
$this->assertTrue($r);
$x->set($values);
$this->assertTrue($x->save());
$this->assertTrue($x->getList(''));
$list = $x->result();
$this->assertArrayHasKey($newAlias, $list);
$this->assertEquals(1, $list[$newAlias]['active']);
}
// try and add one more - it should fail.
$x = new AliasHandler(1, 'admin', true);
$values = [
'localpart' => 'z-david.test',
'domain' => 'example.com',
'active' => 1,
'address' => 'z-david.test@example.com',
'goto' => ['dest@example.com']
];
$r = $x->init('z-david.test@example.com');
// doesn't already exist.
$this->assertFalse($r);
// try saving ....
$x->set($values);
$this->assertFalse($x->save());
$this->assertEquals([
'address' => "You have reached your limit to create aliases!",
0 => "one or more values are invalid!"
], $x->errormsg);
}
public function testLoadsOfAliasesGetHandledByPager()
{
$this->addDomain('example.com', 'admin');
// default limit is 11 aliases.... so it should exit once we get past that.
$dh = new DomainHandler(0, 'admin', true);
$dh->init('example.com');
$this->assertTrue($dh->set(
[
//'domain' => 'example.com',
'aliases' => 99,
'mailboxes' => 88,
'backupmx' => 0,
'active' => 1,
]
));
$this->assertTrue($dh->save());
$dh->getList('');
$domain = $dh->result()['example.com'];
$this->assertEquals(99, $domain['aliases']);
$this->assertEquals(88, $domain['mailboxes']);
foreach (range(1, 80) as $char) {
$newAlias = $char . '-test@example.com';
$x = new AliasHandler(1, 'admin', true);
$values = [
'localpart' => explode('@', $newAlias)[0],
'domain' => 'example.com',
'active' => 1,
'address' => $newAlias,
'goto' => ['dest@example.com']
];
$r = $x->init($newAlias);
$this->assertTrue($r);
$x->set($values);
$this->assertTrue($x->save());
$this->assertTrue($x->getList(''));
$list = $x->result();
$this->assertArrayHasKey($newAlias, $list);
$this->assertEquals(1, $list[$newAlias]['active']);
}
// try and add one more - it should fail.
$x = new AliasHandler(0, 'admin', true);
$x->getList('', [], 5, 20);
$results = $x->result();
$this->assertEquals(5, count($results));
$this->assertTrue(isset($results['31-test@example.com']));
}
}

4
tests/DomainHandlerTest.php
View File

@ -48,7 +48,7 @@ class DomainHandlerTest extends \PHPUnit\Framework\TestCase
$this->assertEmpty($dh->errormsg);
$this->assertEmpty($dh->infomsg);
$this->assertTrue($ret);
$ret = $dh->store();
$ret = $dh->save();
$this->assertTrue($ret);
// Need to add 'admin' as a domain_admin
@ -93,7 +93,7 @@ class DomainHandlerTest extends \PHPUnit\Framework\TestCase
);
$this->assertTrue($ret);
$this->assertTrue($dh->store());
$this->assertTrue($dh->save());
$this->assertEmpty($dh->errormsg);
$dh->getList('');

6
tests/MailboxHandlerTest.php
View File

@ -60,7 +60,7 @@ class MailboxHandlerTest extends \PHPUnit\Framework\TestCase {
$this->assertTrue($ret);
$ret = $dh->store();
$ret = $dh->save();
$this->assertTrue($ret);
@ -109,7 +109,7 @@ class MailboxHandlerTest extends \PHPUnit\Framework\TestCase {
$this->assertEquals(0, count($list));
$x->set($values);
$x->store();
$x->save();
$x->getList('');
@ -149,7 +149,7 @@ class MailboxHandlerTest extends \PHPUnit\Framework\TestCase {
$this->assertEmpty($h->errormsg);
$this->assertEmpty($h->infomsg);
$this->assertTrue($r);
$this->assertTrue($h->store());
$this->assertTrue($h->save());
$h->getList('');
$list = $h->result();