Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stop using deprecated code in CMS 4 for core #10542

Closed
5 tasks
emteknetnz opened this issue Oct 12, 2022 · 3 comments
Closed
5 tasks

Stop using deprecated code in CMS 4 for core #10542

emteknetnz opened this issue Oct 12, 2022 · 3 comments
Assignees
@sabina-talipova
Labels
affects/v4 type/enhancement

Comments

@emteknetnz
Copy link
Member

emteknetnz commented Oct 12, 2022 •
edited
Loading

Splitting from #10500

Acceptance criteria

  • Update all code paths in core (silverstripe/installer) so that deprecated code is no longer being called
  • There's a documented way of skipping test targeting deprecated methods.
  • Manually mark test targeting deprecated method so the developer running the test can choose to skip them. These tests will be later removed in Remove deprecated code in CMS5 for core #10501
    • Only skip these tests when deprecation notices are enabled and the deprecation version is >= 5.0.0
  • Update calls to config()->update() to config()->set() if they are using non-arrays

Notes

  • Ensure that we have a reliable way to surface deprecation warnings from up-stream dependencies such as symfony and guzzle e.g. investigate using this https://symfony.com/doc/current/components/phpunit_bridge.html#usage
  • In some instances there will be no immediate migration path for code in 4 because the new API only exists in 5 e.g. swiftmailer -> symfony/mailer

Implementation notes - stand-alone CI

  • Highly recommend creating a stand alone CI on a non-silverstripe account with deprecation turned on to validate work
  • The CI will needs to use multiple creative-commoners forks (several core modules) at once
  • Previous attempts to use multiple forks in CI have failed due to rate limit issues. Adding a github-token via the setup-php action have NOT worked.
  • The work around this, recommend the special CI try using https://github.com/emteknetnz/vendor-code-patcher to dynamically generate patches within the CI.
  • Failing that, fall back is to manually add the patches on laptop and push to /silverstripe-installer, though this is laborious.
  • Also want to use Logger everything that is deprecated to a CI log file, reason being that behat can still pass even with warnings covering the screen, so we want to capture these deprecations warnings
  • The stand-alone CI is NOT going to be merged back into the silverstripe

Background for CI deprecation job

I tried turning on deprecation warning on a CMS 4 fork on emteknetnz/silverstripe-framework by simply changing the Deprecation::notification_version in _config.php from 4.0.0 to 5.0.0

While most deprecation warnings were harmless, a bunch of them causes the build to go red e.g. Config_ForClass:update()

Because of it causing the build to go red, it's not viable to have a permanent job with deprecation warnings turned on. This is doubly so because gha-ci is an upstream provider for non cms-squad modules that use the CI.

I also don't think have a permanent deprecation job on is even really helpful. There is no further use for it once the work is this card and the corresponding card "Stop using deprecated code in CMS 4 for sink" is completed.

silverstripe/installer run including PRs below

PRs

Bonus PR to update silverstripe.log
@emteknetnz emteknetnz mentioned this issue Oct 12, 2022
Closed
3 tasks
@emteknetnz emteknetnz self-assigned this Oct 12, 2022
This was referenced Oct 12, 2022
Open
Closed
This was referenced Nov 2, 2022
Merged
Closed
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@emteknetnz
Copy link
Member Author

emteknetnz commented Nov 3, 2022 •
edited
Loading

NON-FRAMEWORK

Admin - deprecated

  • LeftAndMain.help_link
  • LeftAndMain::menu_title_for_class()
  • ModelAdmin::getSearchContext()
  • ModelAdmin::SearchSummary()
  • ModelAdmin::SearchForm()

Asset-admin - deprecated

  • UsedOnTableExtension::updateUsage()
  • Embeddable
  • EmbedResource

Assets - deprecations added

  • MigrateFileTask

Assets - migrations

  • File::ini2bytes($iniValue) -> Convert::memstring2bytes($iniValue)

Assets - deprecated

  • File::RelativeLink() (note: SiteTree::RelativeLink() is NOT deprecated)
  • File:ini2bytes()
  • Filesystem::fixfiles()
  • FixFolderPermissionsHelper
  • VersionedFilesMigrator
  • FileMigrationHelper
  • FileMigrationHelper::getFilenameArray
  • FolderMigrationHelper
  • LegacyThumbnailMigrationHelper
  • NormaliseAccessMigrationHelper
  • SecureAssetsMigrationHelper
  • TagsToShortcodeHelper
  • TagsToShortcodeTask
  • VersionedFilesMigrationTask
  • LegacyFileIDHelper
  • FlysystemAssetStore.legacy_filenames
  • FlysystemAssetStore::getFilesystemFor()
  • FlysystemAssetStore::deleteFromFilesystem()
  • FlysystemAssetStore::findVariants()
  • FlysystemAssetStore::moveBetweenFilesystems()
  • FlysystemAssetStore::getStreamSHA1()
  • FlysystemAssetStore::useLegacyFilenames()
  • FlysystemAssetStore::cleanFilename()
  • FlysystemAssetStore::parseFileID()
  • FlysystemAssetStore::getOriginalFilename()
  • FlysystemAssetStore::getVariant()
  • FlysystemAssetStore::removeVariant()
  • FileLinkTracking::ImageTracking()
  • SilverStripe\Assets\Tests\Storage\AssetStoreTest\TestAssetStore

Behat-extension - deprecated

  • BasicContext::iAttachTheFileTo() - @Given /^(?:|I )attach the file "(?P<path>[^"]*)" to "(?P<field>(?:[^"]|\\")*)" with HTML5$/

Campaign-admin - deprecated

  • AddToCampaignHelper::handle()

CMS - deprecations added

  • CMSMain::LinkPageHistory()

CMS - deprecated

  • CMSMain::publishall()
  • CMSPageHistoryController
  • SiteTree::creatableChildren()
  • SiteTreeFileExtension
  • SiteTreeFileFormFactoryExtension
  • SiteTreeFileFormFactoryExtension::updateFormFields()
  • SiteTreeFolderExtension
  • SiteTreeFolderExtension::getUnusedFilesListFilter()
  • VirtualPage::updateImageTracking()

Config - deprecated

  • MemoryConfigColection::update()
  • MemoryConfigColection::serialize()
  • MemoryConfigColection::unserialize()
  • MiddlewareCommon::serialize()
  • MiddlewareCommon::unserialize()

ErrorPage - deprecated

  • (none)

Event-dispatcher - deprecated

  • (none)

Frameworktest - deprecated

  • (none)

GraphQL - deprecated

  • (none)

Login froms - deprecated

  • (none)

Mimevalidator - deprecated

  • (none)

Registry - deprecated

  • (none)

Reports - deprecated

  • (none)

Serve - deprecated

  • (none)

Session manager - deprecated

  • (none)

Siteconfig - deprecated

  • (none)

Testsession - deprecated

  • (none)

Versioned - deprecations added

  • Versioned::Versions()

Versioned - undeprecated

  • Versioned::allVersions()
  • class VersionedTestSessionExtension extends VersionedStateExtension -- raise card to investigate redeprecating

Versioned - deprecated

  • Versioned::getLastEditedForVersion()
  • Versioned::doPublish()
  • Versioned::onAfterRevertToLive()
  • Versioned::publish()
  • Versioned::migrateVersion()
  • Versioned::VersionsList()
  • Versioned::allVersions()
  • Versioned::doRollbackTo()
  • Versioned::onAfterRollback()
  • class VersionedTestSessionExtension extends VersionedStateExtension

Versioned admin - deprecated

  • HistoryControllerFactory
  • CMSMainExtension

FRAMEWORK

"Use xyz instead" - Undeprecated

  • Session::recursivelyApply() - called by Session::start()
  • TreeDropdownField::getLabelField() - method supposed to migrate to uses $this->titleField instead of $this->labelField
  • TreeDropdownField::setLabelField() - method supposed to migrate to uses $this->titleField instead of $this->labelField

"Use xyz instead" - Migrations

  • Convert::array2json($val) -> json_encode($val)
  • Convert::json2array($val) -> json_decode($val, true)
  • Convert::raw2json($val) -> json_encode($val)
  • DataObject::doValidate() -> DataObject::validate
  • Module::getResourcePath($path) -> Module::getResource($path)->getPath()
  • SapphireTest::assertDOSEquals($matches, $dataObjectSet) -> assertListEquals($matches, $dataObjectSet)

"Use xyz instead" - Notes

  • I cannot remove the call in HTTPCacheControlMiddleware::process() to HTTP:augmentState() - though, which itself calls a bunch of deprecated stuff in HTTP - however this will be removed in CMS 5

Deprecated - "Use xyz instead"

  • BaseKernel::sessionEnvironment()
  • ClassInfo::baseDataClass()
  • ClassInfo::table_for_object_field()
  • ClassLoader::classExists()
  • CmsUiContext::iShouldSeeANotice() - @then /^I should see a "([^"]*)" notice$/
  • Config_ForClass::update()
  • Configurable::set_stat()
  • Configurable::stat()
  • Convert::array2json()
  • Convert::json2array()
  • Convert::json2obj()
  • Convert::raw2json()
  • Convert::xml2array()
  • DatabaseAdmin::updateLegacyClassNames()
  • DataObject::doValidate()
  • DataObject::duplicateManyManyRelations()
  • DB::getConn()
  • DBClassName::clear_classname_cache()
  • DebugView::writeError()
  • DebugView::writeFooter()
  • DebugView::writeInfo()
  • DebugView::writeHeader()
  • DebugView::writeSourceFragment()
  • DebugView::writeTrace();
  • DebugView::writeVariable();
  • Deprecation::notification_version()
  • Deprecation::set_enabled()
  • Deprecation.enabled
  • Director::isManifestFlushed()
  • EmbedResource
  • EmbedShortcodeProvider::embedForTemplate()
  • FlushInvalidatedResource::serialize()
  • FlushInvalidatedResource::unserialize()
  • FunctionalTest::get_use_draft_site()
  • FunctionalTest::useDraftSite()
  • FunctionalTest.use_draft_site
  • GridFieldFilterHeader::getLegacyFilterHeader()
  • GridFieldVersionedState
  • HTTP::augmentState()
  • HTTP::gmt_date()
  • HTTP::register_etag()
  • HTTP::register_modification_date()
  • HTTP::register_modification_timestamp()
  • HTTP::set_cache_age()
  • HTTP.cache_age
  • HTTP.cache_ajax_requests
  • HTTP.disable_http_cache
  • HTTP.etag
  • HTTP.modification_date
  • Injector::hasService()
  • LoginForm.authenticator_class
  • ManifestFileFinder.RESOURCES_DIR
  • Member::checkPassword()
  • Member::currentUser()
  • Member::currentUserID()
  • Member::default_admin()
  • Member::logged_in_session_exists()
  • Member::logIn()
  • Member::logOut()
  • Member::set_title_columns()
  • Module::getRelativeResourcePath()
  • Module::getResourcePath()
  • Module::getResourceURL()
  • Module::hasResource()
  • Module::serialize()
  • Module::unserialize()
  • Module.TRIM_CHARS
  • MonologErrorHandler::getLogger()
  • MonologErrorHandler::setLogger()
  • PasswordValidator::characterStrength()
  • PasswordValidator::checkHistoricalPasswords()
  • PasswordValidator::minLength()
  • PDOConnector::connect()
  • RandomGenerator::generateEntropy()
  • RequestFilter
  • RequestProcessor
  • SapphireTest::assertDOSAllMatch()
  • SapphireTest::assertDOSContains()
  • SapphireTest::assertDOSEquals()
  • SapphireTest::assertNotDOSContains()
  • SapphireTest::getFixtureFactory()
  • SapphireTest::loadFixture()
  • SapphireTest::setFixtureFactory()
  • SapphireTest.fixtureFactory
  • Security::check_default_admin()
  • Security::clear_default_admin()
  • Security::default_admin_password()
  • Security::default_admin_username()
  • Security::findAnAdministrator()
  • Security::getLoginForms()
  • Security::has_default_admin()
  • Security::setDefaultAdmin()
  • Session::recursivelyApply()
  • SSViewer_BasicIteratorSuport::First()
  • SSViewer_BasicIteratorSuport::Last()
  • SSViewer::set_theme()
  • TextField::InternallyLabelledField()
  • TinyMCEConfig::getTinyMCEPath()
  • TreeDropdownField::getLabelField()
  • TreeDropdownField::setLabelField()
  • ValidationResult::serialize()
  • ValidationResult::unserialize()
  • ViewableData::ThemeDir()

Other Notes:

  • Director::isManifestFlushed() migrated to Injector::inst()->get(Kernel::class)->isFlushed()
  • RequestProcessor is a deprecated class, but requestprocessors.yml adds a singleton of it (cannot remove from yml until 5)

Things in "Will be removed without equivalent functionality" that will be removed in CMS5, though we cannot remove in CMS4

  • The "CIConfig" functionality used to support phpunit5 + phpunit9 e.g. TestKernel::setIgnoredCIConfigs() is removed in 5, however we possibly cannot stop using this in 4 because some project may still have phpunit5 tests
  • All deprecated code under "Email" is fully replaced in CMS 5 where SwiftMailer was replaced with symfony/mailer
  • GridFieldFilterHeader.* - though I don't think this is actually enabled in any tests
  • SimpleResourceURLGenerator::resolveUnsecuredResource() - removed in CMS 5 where public dir is required (apparently? link?)

Undeprecated things in "Will be removed without equivalent functionality" because there is no upgrade path

  • BaseKernel::getEnvironment() is still used by Director::get_environment_type()
  • DBSchemaManager::showTableNameWarning() is still used by DataObjectSchema::buildTableName()

Deprecated - "Will be removed without equivalent functionality"

  • AbstractConfirmationToken
  • BaseKernel::getIgnoredCIConfigs()
  • ConfirmationTokenChain
  • DataExtension::unload_extra_statics()
  • DBSchemaManager::showTableNameWarning()
  • Deprecation::dump_settings()
  • Deprecation::get_calling_module_from_trace()
  • Deprecation::get_enabled()
  • Deprecation::restore_settings()
  • Deprecation.module_version_overrides
  • Deprecation.notice_level
  • Deprecation.verison
  • Email::debug()
  • Email::findPlainPart()
  • Email::generatePlainPartFromBody()
  • Email::getFailedRecipients()
  • Email::getSwiftMessage()
  • Email::hasPlainPart()
  • Email::render()
  • Email::setFailedRecipients()
  • Email::setSwiftMessage()
  • ErrorControlChain
  • ErrorControlChainMiddleware
  • ErrorDirector
  • Form::formHtmlContent()
  • GridFieldFilterHeader.force_legacy
  • GridFieldFilterHeader.updateSearchContextCallback
  • GridFieldFilterHeader.updateSearchFormCallback
  • GridFieldFilterHeader.useLegacyFilterHeader
  • HTTPRequest::detect_method()
  • InstallerTest
  • Member::create_new_password()
  • Module::getCIConfig()
  • ParameterConfirmationToken
  • Permission::get_declared_permissions_list()
  • Permission::get_label_for_permission()
  • Permission::traverse_declared_permissions()
  • Requirements_Backend::getMinifier()
  • SapphireInfo
  • Security.word_list
  • SimpleResourceURLGenerator::resolveUnsecuredResource()
  • TestKernel::getIgnoredCIConfigs()
  • TestKernel::setIgnoredCIConfigs()
  • TinyMCEGZIPGenerator
  • URLConfirmationToken

Deprecated - other - notes

  • Controller.basicAuthEnabled cannot be removed because Controller::init() uses it - however it along with the non-middleware basic-auth should probably all be removed from CMS5
  • Removing LoginAttempt->Email (replace with EmailHashed) is probably a small card itself
  • Removing Requirements_Minifier would mean we should also deprecate Requirements_Backend::getCombinedFileURL() and some other things in Requirements_Backend - should investigate to see if we want to do this

Deprecated - other - undeprecated

  • Requirements::add_i18n_javascript() - 3rd param $langOnly - used by various things e.g. CampaignAdmin::init()
  • CookieJar::getSameSite() - no migration path

Deprecated - other

  • Controller.basicAuthEnabled
  • Controller.disableBasicAuth()
  • CookieJar::getSameSite()
  • Extensible::constructExtensions()
  • CsvBulkLoader::splitFile()
  • CsvBulkLoader::getNewSplitFileName()
  • CsvBulkLoader::processChunk()
  • FieldList::collateDataFields()
  • TinyMCEConfig::getAdminModule()
  • FlushInvalidatedResource::getResource()
  • DataObject.destroyed
  • SearchContext.connective
  • LoginAttempt->Email
  • Permission.declared_permissions
  • Permission.declared_permissions_list
  • Requirements_Minifier
  • undeprecate this Requirements::add_i18n_javascript() - 3rd parm $langOnly
  • SSViewer.theme

This was referenced Nov 3, 2022
Merged
Open
Merged
Open
Open
Merged
@emteknetnz emteknetnz removed their assignment Nov 9, 2022
@GuySartorelli
Copy link
Member

There should probably be a docs PR as well - the 4.12 changelog should mention things that you've newly deprecated.

@sabina-talipova
Copy link
Contributor

@emteknetnz could you check this failed CI, please.
https://github.com/silverstripe/silverstripe-cms/actions/runs/3467505777/jobs/5809980321

@emteknetnz emteknetnz mentioned this issue Feb 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sabina-talipova sabina-talipova

Labels
affects/v4 type/enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@emteknetnz @GuySartorelli @sabina-talipova