It should be good to have some clear standard for this question, raised first on this issue #1785086: Introduce a generic API for interface translation strings, but it would affect any other storage controller like the ones created in entity_get_controller() (entity.inc)

As a side issue, I'd like to clarify too what is the right way to define the class to be used for the storage controller when there is'nt hook_info() to declare it, like in that locale_storage() function in the linked issue.

CommentFileSizeAuthor
#25 how_to_register_storage-1806756-25.patch3.23 KBhitesh-jain
#22 1806756-22.patch3.29 KByogeshmpawar
#17 1806756-17.patch4.41 KBchishah92
#13 1806756-13.patch4.34 KBrpayanm
#11 interdiff.txt7.81 KBalansaviolobo
#11 how_to_register_storage-1806756-11.patch4.39 KBalansaviolobo
#3 locale_storage_dic-01.patch6.03 KBjose reyero

Comments

jose reyero’s picture

jose reyero commented
Title: Do storage controllers need a 'target' database parameter? (Looking for a common standard) » How to register storage controllers / 'target' database parameter (Looking for a common standard)
Category: feature » support

It seems the answer is in Drupal\Core\CoreBundle.php

/**
 * Bundle class for mandatory core services.
 *
 * This is where Drupal core registers all of its services to the Dependency
 * Injection Container. Modules wishing to register services to the container
 * should extend Symfony's Bundle class directly, not this class.
 */

Thus we'll need to create a LocaleBundle class, and register the storage like this:

class LocaleBundle extends Bundle
{
  public function build(ContainerBuilder $container) {
    $container->register('locale.storage', 'Drupal\locale\StringDatabaseStorage')
    ->addArgument(new Reference('database'))
    ->addArgument(array('target' => 'default'));
  }
}

This seems to work and be a clean usage of DIC. More feedback welcomed though.

Note: if you couldn't find any documentation either: Classes in mymodule/lib/Drupal/mymodule/MymoduleBundle are automatically loaded by Drupal core from DrupalKernel::registerBundles(). Note the capital 'M' in 'Mymodule'

jose reyero’s picture

jose reyero commented

That approach above seems to work, but not for install/update, for which the storage seems not to be created, though locale_storage() is invoked causing a DIC exception. This may be related to #1807272: Mixed locale issues with upgrade scripts, unit tests, 'Table simpletest..locales_source doesn't exist', etc...

jose reyero’s picture

jose reyero commented
Category: support » feature
Status: Active » Needs review
Issue tags: +D8MI
StatusFileSize
new locale_storage_dic-01.patch6.03 KB

This is the working patch atm for locale storage controller. This is quite some special case, as noted in the locale_storage() function's comments so we need to handle the case of no locale storage present in DIC too.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Issue tags: -D8MI

I don't think the second argument is necessary. If I wanted locale.storage to use a non-default database, I would do the following:

class LocaleBundle extends Bundle {

  public function build(ContainerBuilder $container) {
    $container->register('database.some_target', 'Drupal\Core\Database\Connection')
      ->setFactoryClass('Drupal\Core\Database\Database')
      ->setFactoryMethod('getConnection')
      ->addArgument('some_target');
    $container->getDefinition('locale.storage')
      ->replaceArgument(0, new Reference('database.some_target'));
  }
}

That way local.storage doesn't need to know about database targets.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Issue tags: +D8MI

Restoring tag. The previous was a x-post.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Oh, I just saw, that DatabaseStringStorage already takes an $option parameter, so #4 is bogus. (Or if at all, a follow-up.)

Status: Needs review » Needs work

The last submitted patch, locale_storage_dic-01.patch, failed testing.

jose reyero’s picture

jose reyero commented

Well, the patch fails but it actually fixes some issues. We get now real exceptions instead of "another exception thrown while handling one exception", which was happending before (because of #1808864: Exception handling is not 'locale safe' (thus not database safe))

This error is triggered by javascript file translation from the installer and would be in principle easy to fix, though this may not be the best solution:

--- a/core/modules/locale/locale.module
+++ b/core/modules/locale/locale.module
@@ -414,6 +414,12 @@ function locale_system_update($components) {
  * file if necessary, and adds it to the page.
  */
 function locale_js_alter(&$javascript) {
+  // If during install or update, we don't do anything if we don't have locale
+  // storage availabe.
+  if (!locale_storage()) {
+    return FALSE;
+  }
+
   $language_interface = language(LANGUAGE_TYPE_INTERFACE);
 
   $dir = 'public://' . variable_get('locale_js_directory', 'languages');

I'll be exploring other options like initializing the storage on locale_language_init(). Though the indicated place to do it is LocaleBundle, this seems not to be installer/update safe, which is too bad :-(

jose reyero’s picture

jose reyero commented

After some more research, it seems the locale storage is not a good candidate for a clean initialization because of the reasons outlined here, https://drupal.org/node/1807272#comment-6595134

jair’s picture

jair commented
Issue tags: +Needs reroll

Needs reroll

alansaviolobo’s picture

alansaviolobo commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new how_to_register_storage-1806756-11.patch4.39 KB
new interdiff.txt7.81 KB

Status: Needs review » Needs work

The last submitted patch, 11: how_to_register_storage-1806756-11.patch, failed testing.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 1806756-13.patch4.34 KB

rerolling...

Status: Needs review » Needs work

The last submitted patch, 13: 1806756-13.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

chishah92’s picture

chishah92 commented
Assigned: Unassigned » chishah92
chishah92’s picture

chishah92 commented
Status: Needs work » Needs review
StatusFileSize
new 1806756-17.patch4.41 KB

Rerolled the patch.

Status: Needs review » Needs work

The last submitted patch, 17: 1806756-17.patch, failed testing.

chishah92’s picture

chishah92 commented
Assigned: chishah92 » Unassigned

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

hitesh-jain’s picture

hitesh-jain commented
Assigned: Unassigned » hitesh-jain
yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location Pune(MH), INDIA 🇮🇳
commented
Status: Needs work » Needs review
StatusFileSize
new 1806756-22.patch3.29 KB

I have rerolled the patch to work with 8.2.x branch.

Status: Needs review » Needs work

The last submitted patch, 22: 1806756-22.patch, failed testing.

hitesh-jain’s picture

hitesh-jain commented
hitesh-jain’s picture

hitesh-jain commented
Version: 8.2.x-dev » 8.3.x-dev
Status: Needs work » Needs review
StatusFileSize
new how_to_register_storage-1806756-25.patch3.23 KB

Patch rerolled against 8.3.x branch.

Status: Needs review » Needs work

The last submitted patch, 25: how_to_register_storage-1806756-25.patch, failed testing.

manuel garcia’s picture

manuel garcia commented
Issue tags: -Needs reroll

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +stale-issue-cleanup

Thank you for sharing your idea for improving Drupal.

We are working to decide if this proposal meets the Criteria for evaluating proposed changes. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or there is no community support. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

smustgrave’s picture

smustgrave commented

Wanted to bump 1 more time, if no follow up could be closed in 3 months.