From dc36725869dc7e261ff0bb7181e17ca7d96138d8 Mon Sep 17 00:00:00 2001 From: Paul Meyrick Date: Fri, 18 Mar 2011 15:01:09 +1300 Subject: [PATCH] MINOR Using BlankPage template in SecurityTest, BasicAuthTest to remove ContentController dependency MINOR Checking for SiteTree class existence in Security, Translatable MINOR Checking for ContentController existence in FulltextSearchable MINOR Removed unnecessary ContentController tests from ObjectTest MINOR Replaced CMS specific examples in PermissionCheckboxSetFieldTest, DataObjectTest MINOR Changed SecurityTest to make assertions against Security/login rather than relying on redirection from admin/cms --- core/model/Translatable.php | 44 ++++----- dev/Debug.php | 13 +-- search/FulltextSearchable.php | 5 +- security/Security.php | 8 +- templates/BlankPage.ss | 15 ++++ tests/DataObjectTest.php | 22 +++-- tests/DataObjectTest.yml | 90 +++++++++---------- tests/ObjectTest.php | 9 -- tests/RequestHandlingTest.php | 10 ++- tests/forms/FormTest.php | 9 ++ tests/security/BasicAuthTest.php | 18 ++-- .../PermissionCheckboxSetFieldTest.php | 12 +-- .../PermissionCheckboxSetFieldTest.yml | 17 ++-- tests/security/SecurityTest.php | 4 +- 14 files changed, 147 insertions(+), 129 deletions(-) create mode 100644 templates/BlankPage.ss diff --git a/core/model/Translatable.php b/core/model/Translatable.php index 244157189..619fd2667 100755 --- a/core/model/Translatable.php +++ b/core/model/Translatable.php @@ -453,7 +453,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { * @deprecated 2.4 Use Object::add_extension('SiteTree', 'Translatable') */ static function enable() { - Object::add_extension('SiteTree', 'Translatable'); + if(class_exists('SiteTree')) Object::add_extension('SiteTree', 'Translatable'); } /** @@ -462,7 +462,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { * @deprecated 2.4 Use Object::remove_extension('SiteTree', 'Translatable') */ static function disable() { - Object::remove_extension('SiteTree', 'Translatable'); + if(class_exists('SiteTree')) Object::remove_extension('SiteTree', 'Translatable'); } /** @@ -472,7 +472,11 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { * @return boolean True if enabled */ static function is_enabled() { - return Object::has_extension('SiteTree', 'Translatable'); + if(class_exists('SiteTree')){ + return Object::has_extension('SiteTree', 'Translatable'); + }else{ + return false; + } } @@ -620,7 +624,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { ))->column(); if(!$idsWithoutLocale) return; - if($this->owner->class == 'SiteTree') { + if(class_exists('SiteTree') && $this->owner->class == 'SiteTree') { foreach(array('Stage', 'Live') as $stage) { foreach($idsWithoutLocale as $id) { $obj = Versioned::get_one_by_stage( @@ -779,7 +783,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { // Caution: This logic is very sensitve to infinite loops when translation status isn't determined properly // If a parent for the newly written translation was existing before this // onBeforeWrite() call, it will already have been linked correctly through createTranslation() - if($this->owner->hasField('ParentID') && $this->owner instanceof SiteTree) { + if(class_exists('SiteTree') && $this->owner->hasField('ParentID') && $this->owner instanceof SiteTree) { if( !$this->owner->ID && $this->owner->ParentID @@ -898,16 +902,19 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { * seeing readonly fields as well. */ function updateCMSFields(FieldSet &$fields) { + if(!class_exists('SiteTree')) return; // Don't apply these modifications for normal DataObjects - they rely on CMSMain logic if(!($this->owner instanceof SiteTree)) return; // used in CMSMain->init() to set language state when reading/writing record $fields->push(new HiddenField("Locale", "Locale", $this->owner->Locale) ); - // Don't allow translation of virtual pages because of data inconsistencies (see #5000) - $excludedPageTypes = array('VirtualPage'); - foreach($excludedPageTypes as $excludedPageType) { - if(is_a($this->owner, $excludedPageType)) return; + // Don't allow translation of virtual pages because of data inconsistencies (see #5000) + if(class_exists('VirtualPage')){ + $excludedPageTypes = array('VirtualPage'); + foreach($excludedPageTypes as $excludedPageType) { + if(is_a($this->owner, $excludedPageType)) return; + } } $excludeFields = array( @@ -1271,7 +1278,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { } function providePermissions() { - if(!Object::has_extension('SiteTree', 'Translatable')) return false; + if(!Object::has_extension('SiteTree', 'Translatable') || !class_exists('SiteTree')) return false; $locales = self::get_allowed_locales(); @@ -1324,23 +1331,6 @@ class Translatable extends DataObjectDecorator implements PermissionProvider { return $returnMap; } - /** - * Get the RelativeLink value for a home page in another locale. This is found by searching for the default home - * page in the default language, then returning the link to the translated version (if one exists). - * - * @return string - */ - public static function get_homepage_link_by_locale($locale) { - $originalLocale = self::get_current_locale(); - - self::set_current_locale(self::default_locale()); - $original = SiteTree::get_by_link(RootURLController::get_default_homepage_link()); - self::set_current_locale($originalLocale); - - if($original) { - if($translation = $original->getTranslation($locale)) return trim($translation->RelativeLink(true), '/'); - } - } /** * @deprecated 2.4 Use {@link Translatable::get_homepage_link_by_locale()} diff --git a/dev/Debug.php b/dev/Debug.php index 66db18766..2755c6904 100644 --- a/dev/Debug.php +++ b/dev/Debug.php @@ -335,12 +335,13 @@ class Debug { if(Director::is_ajax()) { echo $friendlyErrorMessage; } else { - $errorFilePath = ErrorPage::get_filepath_for_errorcode($statusCode, Translatable::get_current_locale()); - if(file_exists($errorFilePath)) { - $content = file_get_contents(ASSETS_PATH . "/error-$statusCode.html"); - // $BaseURL is left dynamic in error-###.html, so that multi-domain sites don't get broken - echo str_replace('$BaseURL', Director::absoluteBaseURL(), $content); - + if(class_exists('ErrorPage')){ + $errorFilePath = ErrorPage::get_filepath_for_errorcode($statusCode, Translatable::get_current_locale()); + if(file_exists($errorFilePath)) { + $content = file_get_contents(ASSETS_PATH . "/error-$statusCode.html"); + // $BaseURL is left dynamic in error-###.html, so that multi-domain sites don't get broken + echo str_replace('$BaseURL', Director::absoluteBaseURL(), $content); + } } else { $renderer = new DebugView(); $renderer->writeHeader(); diff --git a/search/FulltextSearchable.php b/search/FulltextSearchable.php index 8b46fa7d1..117748710 100644 --- a/search/FulltextSearchable.php +++ b/search/FulltextSearchable.php @@ -52,8 +52,9 @@ class FulltextSearchable extends DataObjectDecorator { } } self::$searchable_classes = $searchableClasses; - - Object::add_extension("ContentController", "ContentControllerSearchExtension"); + if(class_exists("ContentController")){ + Object::add_extension("ContentController", "ContentControllerSearchExtension"); + } } /** diff --git a/security/Security.php b/security/Security.php index 497dd2e04..722603dba 100644 --- a/security/Security.php +++ b/security/Security.php @@ -417,7 +417,7 @@ class Security extends Controller { Session::clear('Security.Message'); // custom processing - return $customisedController->renderWith(array('Security_login', 'Security', $this->stat('template_main'), 'ContentController')); + return $customisedController->renderWith(array('Security_login', 'Security', $this->stat('template_main'), 'BlankPage')); } function basicauthlogin() { @@ -455,7 +455,7 @@ class Security extends Controller { )); //Controller::$currentController = $controller; - return $customisedController->renderWith(array('Security_lostpassword', 'Security', $this->stat('template_main'), 'ContentController')); + return $customisedController->renderWith(array('Security_lostpassword', 'Security', $this->stat('template_main'), 'BlankPage')); } @@ -514,7 +514,7 @@ class Security extends Controller { )); //Controller::$currentController = $controller; - return $customisedController->renderWith(array('Security_passwordsent', 'Security', $this->stat('template_main'), 'ContentController')); + return $customisedController->renderWith(array('Security_passwordsent', 'Security', $this->stat('template_main'), 'BlankPage')); } @@ -594,7 +594,7 @@ class Security extends Controller { } } - return $customisedController->renderWith(array('Security_changepassword', 'Security', $this->stat('template_main'), 'ContentController')); + return $customisedController->renderWith(array('Security_changepassword', 'Security', $this->stat('template_main'), 'BlankPage')); } /** diff --git a/templates/BlankPage.ss b/templates/BlankPage.ss new file mode 100644 index 000000000..86b1d5376 --- /dev/null +++ b/templates/BlankPage.ss @@ -0,0 +1,15 @@ + + + + + +$Title +<% base_tag %> + + + $Content +
+ $Form +
+ + \ No newline at end of file diff --git a/tests/DataObjectTest.php b/tests/DataObjectTest.php index 465ad2416..f430a6408 100755 --- a/tests/DataObjectTest.php +++ b/tests/DataObjectTest.php @@ -697,22 +697,20 @@ class DataObjectTest extends SapphireTest { } function testNewClassInstance() { - $page = $this->objFromFixture('Page', 'page1'); - $changedPage = $page->newClassInstance('RedirectorPage'); - $changedFields = $changedPage->getChangedFields(); + $dataObject = $this->objFromFixture('DataObjectTest_TeamComment', 'comment1'); + $changedDO = $dataObject->newClassInstance('File'); + $changedFields = $changedDO->getChangedFields(); // Don't write the record, it will reset changed fields - $this->assertType('RedirectorPage', $changedPage); - $this->assertEquals($changedPage->ClassName, 'RedirectorPage'); - $this->assertEquals($changedPage->RedirectionType, 'Internal'); - //$this->assertEquals($changedPage->RecordClassName, 'RedirectorPage'); + $this->assertType('File', $changedDO); + $this->assertEquals($changedDO->ClassName, 'File'); $this->assertContains('ClassName', array_keys($changedFields)); - $this->assertEquals($changedFields['ClassName']['before'], 'Page'); - $this->assertEquals($changedFields['ClassName']['after'], 'RedirectorPage'); + $this->assertEquals($changedFields['ClassName']['before'], 'DataObjectTest_TeamComment'); + $this->assertEquals($changedFields['ClassName']['after'], 'File'); - $changedPage->write(); - $this->assertType('RedirectorPage', $changedPage); - $this->assertEquals($changedPage->ClassName, 'RedirectorPage'); + $changedDO->write(); + $this->assertType('File', $changedDO); + $this->assertEquals($changedDO->ClassName, 'File'); } function testManyManyExtraFields() { diff --git a/tests/DataObjectTest.yml b/tests/DataObjectTest.yml index ad7f68990..fa6f16ab4 100644 --- a/tests/DataObjectTest.yml +++ b/tests/DataObjectTest.yml @@ -1,54 +1,54 @@ Page: - home: - Title: Home - page1: - Title: First Page - Content:

Some test content

- page2: - Title: Second Page + home: + Title: Home + page1: + Title: First Page + Content:

Some test content

+ page2: + Title: Second Page DataObjectTest_Team: - team1: - Title: Team 1 - team2: - Title: Team 2 + team1: + Title: Team 1 + team2: + Title: Team 2 DataObjectTest_Player: - captain1: - FirstName: Captain 1 - FavouriteTeam: =>DataObjectTest_Team.team1 - Teams: =>DataObjectTest_Team.team1 - captain2: - FirstName: Captain 2 - Teams: =>DataObjectTest_Team.team2 - player1: - FirstName: Player 1 - player2: - FirstName: Player 2 - Teams: =>DataObjectTest_Team.team1,=>DataObjectTest_Team.team2 + captain1: + FirstName: Captain + FavouriteTeam: =>DataObjectTest_Team.team1 + Teams: =>DataObjectTest_Team.team1 + captain2: + FirstName: Captain 2 + Teams: =>DataObjectTest_Team.team2 + player1: + FirstName: Player 1 + player2: + FirstName: Player 2 + Teams: =>DataObjectTest_Team.team1,=>DataObjectTest_Team.team2 DataObjectTest_SubTeam: - subteam1: - Title: Subteam 1 - SubclassDatabaseField: Subclassed 1 - DecoratedDatabaseField: Decorated 1 - subteam2_with_player_relation: - Title: Subteam 2 - SubclassDatabaseField: Subclassed 2 - DecoratedHasOneRelationship: =>DataObjectTest_Player.player1 - subteam3_with_empty_fields: - Title: Subteam 3 + subteam1: + Title: Subteam 1 + SubclassDatabaseField: Subclassed 1 + DecoratedDatabaseField: Decorated 1 + subteam2_with_player_relation: + Title: Subteam 2 + SubclassDatabaseField: Subclassed 2 + DecoratedHasOneRelationship: =>DataObjectTest_Player.player1 + subteam3_with_empty_fields: + Title: Subteam 3 DataObjectTest_TeamComment: - comment1: - Name: Joe - Comment: This is a team comment by Joe - Team: =>DataObjectTest_Team.team1 - comment2: - Name: Bob - Comment: This is a team comment by Bob - Team: =>DataObjectTest_Team.team1 - comment3: - Name: Phil - Comment: Phil is a unique guy, and comments on team2 - Team: =>DataObjectTest_Team.team2 \ No newline at end of file + comment1: + Name: Joe + Comment: This is a team comment by Joe + Team: =>DataObjectTest_Team.team1 + comment2: + Name: Bob + Comment: This is a team comment by Bob + Team: =>DataObjectTest_Team.team1 + comment3: + Name: Phil + Comment: Phil is a unique guy, and comments on team2 + Team: =>DataObjectTest_Team.team2 diff --git a/tests/ObjectTest.php b/tests/ObjectTest.php index 6c2c1e271..208878120 100755 --- a/tests/ObjectTest.php +++ b/tests/ObjectTest.php @@ -20,7 +20,6 @@ class ObjectTest extends SapphireTest { /* SiteTree should have all of the methods that Versioned has, because Versioned is listed in SiteTree's * extensions */ $st = new SiteTree(); - $cc = new ContentController($st); $this->assertTrue($st->hasMethod('publish'), "Test SiteTree has publish"); $this->assertTrue($st->hasMethod('migrateVersion'), "Test SiteTree has migrateVersion"); @@ -29,14 +28,6 @@ class ObjectTest extends SapphireTest { $this->assertTrue($st->hasMethod('PuBliSh'), "Test SiteTree has PuBliSh"); $this->assertTrue($st->hasMethod('MiGratEVersIOn'), "Test SiteTree has MiGratEVersIOn"); - /* In a similar manner, all of SiteTree's methods should be available on ContentController, because $failover is set */ - $this->assertTrue($cc->hasMethod('canView'), "Test ContentController has canView"); - $this->assertTrue($cc->hasMethod('linkorcurrent'), "Test ContentController has linkorcurrent"); - - /* This 'method copying' is transitive, so all of Versioned's methods should be available on ContentControler. - * Once again, this is case-insensitive */ - $this->assertTrue($cc->hasMethod('MiGratEVersIOn'), "Test ContentController has MiGratEVersIOn"); - /* The above examples make use of SiteTree, Versioned and ContentController. Let's test defineMethods() more * directly, with some sample objects */ $objs = array(); diff --git a/tests/RequestHandlingTest.php b/tests/RequestHandlingTest.php index 4bdf7a48d..c749be546 100755 --- a/tests/RequestHandlingTest.php +++ b/tests/RequestHandlingTest.php @@ -317,7 +317,10 @@ class RequestHandlingTest_Controller extends Controller implements TestOnly { public function throwhttperror() { $this->httpError(404, 'This page does not exist.'); } - + + public function getViewer(){ + return new SSViewer('ContentController'); + } } class RequestHandlingTest_FormActionController extends Controller { @@ -369,6 +372,11 @@ class RequestHandlingTest_FormActionController extends Controller { function formactionInAllowedActions($data, $form = null) { return 'formactionInAllowedActions'; } + + public function getViewer(){ + return new SSViewer('ContentController'); + } + } /** diff --git a/tests/forms/FormTest.php b/tests/forms/FormTest.php index c6517558b..c53fdef1d 100755 --- a/tests/forms/FormTest.php +++ b/tests/forms/FormTest.php @@ -422,6 +422,11 @@ class FormTest_Controller extends Controller implements TestOnly { $form->sessionMessage('Test save was successful', 'good'); return $this->redirectBack(); } + + function getViewer(){ + return new SSViewer('ContentController'); + } + } class FormTest_ControllerWithSecurityToken extends Controller implements TestOnly { @@ -454,6 +459,10 @@ class FormTest_ControllerWithSecurityToken extends Controller implements TestOnl $form->sessionMessage('Test save was successful', 'good'); return $this->redirectBack(); } + + function getViewer(){ + return new SSViewer('ContentController'); + } } Director::addRules(50, array( diff --git a/tests/security/BasicAuthTest.php b/tests/security/BasicAuthTest.php index c021c0470..4c339e607 100644 --- a/tests/security/BasicAuthTest.php +++ b/tests/security/BasicAuthTest.php @@ -107,34 +107,38 @@ class BasicAuthTest extends FunctionalTest { } -class BasicAuthTest_ControllerSecuredWithPermission extends ContentController implements TestOnly { +class BasicAuthTest_ControllerSecuredWithPermission extends Controller implements TestOnly { static $post_init_called = false; static $index_called = false; + + protected $template = '../sapphire/templates/BlankPage.ss'; function init() { self::$post_init_called = false; self::$index_called = false; BasicAuth::protect_entire_site(true, 'MYCODE'); - parent::init(); - + self::$post_init_called = true; } function index() { self::$index_called = true; } - + + + } -class BasicAuthTest_ControllerSecuredWithoutPermission extends ContentController implements TestOnly { - +class BasicAuthTest_ControllerSecuredWithoutPermission extends Controller implements TestOnly { + + protected $template = '../sapphire/templates/BlankPage.ss'; + function init() { BasicAuth::protect_entire_site(true, null); - parent::init(); } diff --git a/tests/security/PermissionCheckboxSetFieldTest.php b/tests/security/PermissionCheckboxSetFieldTest.php index bb0638441..b184edb99 100644 --- a/tests/security/PermissionCheckboxSetFieldTest.php +++ b/tests/security/PermissionCheckboxSetFieldTest.php @@ -14,14 +14,14 @@ class PermissionCheckboxSetFieldTest extends SapphireTest { 'GroupID' ); $f->setHiddenPermissions( - array('CMS_ACCESS_ReportAdmin') + array('NON-ADMIN') ); $this->assertEquals( $f->getHiddenPermissions(), - array('CMS_ACCESS_ReportAdmin') + array('NON-ADMIN') ); - $this->assertContains('CMS_ACCESS_CMSMain', $f->Field()); - $this->assertNotContains('CMS_ACCESS_ReportAdmin', $f->Field()); + $this->assertContains('ADMIN', $f->Field()); + $this->assertNotContains('NON-ADMIN', $f->Field()); } function testSaveInto() { @@ -53,7 +53,7 @@ class PermissionCheckboxSetFieldTest extends SapphireTest { // add some permissions $field->setValue(array( 'ADMIN'=>true, - 'CMS_ACCESS_AssetAdmin'=>true + 'NON-ADMIN'=>true )); $field->saveInto($group); @@ -61,7 +61,7 @@ class PermissionCheckboxSetFieldTest extends SapphireTest { $untouchable->flushCache(); $this->assertEquals($group->Permissions()->Count(), 2, 'The tested group has two permissions permission'); $this->assertEquals($group->Permissions("\"Code\"='ADMIN'")->Count(), 1, 'The tested group has ADMIN permission'); - $this->assertEquals($group->Permissions("\"Code\"='CMS_ACCESS_AssetAdmin'")->Count(), 1, 'The tested group has CMS_ACCESS_AssetAdmin permission'); + $this->assertEquals($group->Permissions("\"Code\"='NON-ADMIN'")->Count(), 1, 'The tested group has CMS_ACCESS_AssetAdmin permission'); $this->assertEquals($untouchable->Permissions()->Count(), 1, 'The other group has one permission'); $this->assertEquals($untouchable->Permissions("\"Code\"='ADMIN'")->Count(), 1, 'The other group has ADMIN permission'); diff --git a/tests/security/PermissionCheckboxSetFieldTest.yml b/tests/security/PermissionCheckboxSetFieldTest.yml index f7df7d52e..92f53159f 100644 --- a/tests/security/PermissionCheckboxSetFieldTest.yml +++ b/tests/security/PermissionCheckboxSetFieldTest.yml @@ -1,10 +1,11 @@ Group: - group: - Code: group - untouchable: - Code: untouchable - + group: + Code: group + untouchable: + Code: untouchable Permission: - perm1: - Code: ADMIN - Group: =>Group.untouchable + perm1: + Code: ADMIN + Group: =>Group.untouchable + perm2: + Code: NON-ADMIN \ No newline at end of file diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index 126d98b44..44cad6c7d 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -59,7 +59,7 @@ class SecurityTest extends FunctionalTest { $this->session()->inst_set('loggedInAs', $member->ID); /* View the Security/login page */ - $this->get('Security/login'); + $response = $this->get('Security/login'); $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.action'); @@ -93,7 +93,7 @@ class SecurityTest extends FunctionalTest { $this->autoFollowRedirection = true; /* Attempt to get into the admin section */ - $response = $this->get('admin/cms/'); + $response = $this->get('Security/login/'); $items = $this->cssParser()->getBySelector('#MemberLoginForm_LoginForm input.text');