Add orderby key to all sequences in core

I was going to propose that we could add orderby: key natural (or similar and also for values) but I just checked the filter formats and the FilterPluginCollection. And it is doing a much more complex sorting taking into account weight, status, the provider and finally the plugin id. So if we want to sort it exactly the same way with the schema we would have to add the option to add a static method or other function to sort with.

But I wonder if that really matters? When \Drupal\filter\Entity\FilterFormat::filters is called the FilterPluginCollection is sorted, so the running code is sorting the filters properly, they don't need to be saved in config with the same order necessarily.
In #2852557: Config export key order is not predictable, use config schema to order keys for maps we re-order config already because it is inconsistent otherwise. Most places could be updated by sorting the schema the way the code expects it, but in other places we had to do it the other way around because inherited schemas also inherit the order.

So of course in isolation the two patches (here and in #2852557) will necessarily conflict a lot since both will try to re-order things. But since the sorting can be scripted for the sake of the patch generation it can also be re-rolled relatively easily. (I just hope the tests don't conflict too much)

So my two cent RE #10: that is fine, the alternative would be to introduce a static sorting function defined in the schema. But since the different sorting just affects how the yaml is saved in git (and the array in the database) and not how it is used in code, I think it is worth the tradeoff when we can predictably sort a config array just given its schema.

I made a little script to analyze the default config of umami and standard. here is its output:

summary
=======

value sort: 4
key sort: 9
different sorting: 23
not enough info (one element): 32
not enough info (empty): 31

value sort
==========

config_dependencies (Dependencies) list (Module dependencies)
mapping (Cache metadata) list (Cache contexts)
user.role.* (User role settings) list (Permissions)
workflows.transition (Transitions) list (From state IDs)

key sort
========

block.settings.field_block:*:*:* (Block settings) list (Context assignments)
core.entity_form_display.*.*.* (Entity form display) list (Field widgets)
core.entity_form_display.*.*.* (Entity form display) list (Hidden)
core.entity_view_display.*.*.* (Entity display) list (Field formatters)
mapping (Group) list (Group items)
mapping (Language from the URL (Path prefix or domain).) list (Domain configuration)
mapping (Language from the URL (Path prefix or domain).) list (Path prefix configuration)
views.display.default (Default display options) list (No results behavior)
views.filter.bundle (Bundle) list (Values)

different sorting
=================

config_dependencies (Dependencies) list (Configuration entity dependencies)
core.entity_view_display.*.*.* (Entity display) list (Field display setting)
core.entity_view_display.*.*.*.third_party.layout_builder (Per-view-mode Layout Builder settings) list (Sequence)
field.storage_settings.list_string (List (text) settings) list (Allowed values list)
filter.format.* (Text formats) list (Enabled filters)
layout_builder.section (Layout section) list (Components)
list (Rows) list (Button groups)
mapping (Button group) list (Buttons)
mapping (Expose) list (User roles)
mapping (Exposed) list (User roles)
media.source.oembed:* (oEmbed media source configuration) list (Allowed oEmbed providers)
rdf.mapping.*.* (RDF mapping) list (Field mappings)
responsive_image.image_mapping_type.sizes (Mapping) list (Image styles to be used when using the 'sizes' attribute)
tour.tour.* (Tour settings) list (Tips)
views.display.attachment (Attachment display options) list (Filters)
views.display.block (Block display options) list (Filters)
views.display.default (Default display options) list (Fields)
views.display.default (Default display options) list (Filters)
views.display.default (Default display options) list (Sorts)
views.display.feed (Feed display options) list (The feed icon will be available only to the selected displays.)
views.style.table (Table) list (Columns info)
views.style.table (Table) list (Columns)
views.view.* (View) list (Displays)

not enough info (one element)
=============================

block.block.* (Block) list (Visibility Conditions)
block.settings.extra_field_block:*:*:* (Block settings) list (Context assignments)
condition.plugin.entity_bundle:* (Visibility Condition) list (Context assignments)
condition.plugin.entity_bundle:* (Visibility Condition) list (Sequence)
config_dependencies (Dependencies) list (Content entity dependencies)
config_dependencies (Dependencies) list (Theme dependencies)
config_dependencies_base (Enforced configuration dependencies) list (Module dependencies)
contact.form.* (Contact form) list (Recipients)
core.base_field_override.*.*.* (Base field bundle override) list (Default values)
core.entity_view_display.*.*.* (Entity display) list (Third party settings)
core.menu.static_menu_link_overrides (Static menu link overrides) list (Definitions)
editor.settings.ckeditor (CKEditor settings) list (Plugins)
entity_reference_selection.default:* (Entity reference selection plugin settings) list (types)
field.field.*.*.* (Field) list (Default values)
field.storage.*.* (Field) list (Indexes)
filter.format.* (Text formats) list (Roles)
image.style.* (Image style) list (Sequence)
language.content_settings.*.* (Content Language Settings) list (Third party settings)
list (Indexes) list (Indexes)
mapping (Groups) list (Groups)
mapping (Toolbar configuration) list (Rows)
media.type.* (Media type) list (Field map)
node.type.* (Content type) list (Third party settings)
rdf.mapping.*.* (RDF mapping) list (Types)
rdf_field_mapping (RDF mapping) list (Properties)
responsive_image.styles.* (Responsive image style) list (Image style mappings)
tour.tour.* (Tour settings) list (Route settings)
views.display.attachment (Attachment display options) list (Attach to)
views.display.default (Default display options) list (Arguments)
views.display.default (Default display options) list (Header)
views.display.page (Page display options) list (Header)
views.filter.language (Language) list (Values)

not enough info (empty)
=======================

condition.plugin.request_path (Visibility Condition) list (Context assignments)
field.widget.settings.media_library_widget (Settings) list (Allowed media types, in display order)
field_formatter (Field formatter) list (Third party settings)
field_formatter.entity_view_display (Field formatter) list (Third party settings)
filter (Filter) list (Filter settings)
layout_builder.section (Layout section) list (Third party settings)
mapping (Cache metadata) list (Cache tags)
mapping (Expose) list (List of available operators)
mapping (Exposed) list (List of available operators)
mapping (Field widget) list (Third party settings)
mapping (Group) list (Defaults)
node.type.*.third_party.menu_ui (Per-content type menu settings) list (Available menus)
views.argument.node_nid (Node ID) list (Validate options)
views.argument_validator.entity:taxonomy_term (Validate options) list (Bundles)
views.cache.tag (Tag based caching) list (Options)
views.display.attachment (Attachment display options) list (Display extenders)
views.display.block (Block display options) list (Display extenders)
views.display.default (Default display options) list (Display extenders)
views.display.default (Default display options) list (Footer)
views.display.default (Default display options) list (Relationships)
views.display.feed (Feed display options) list (Display extenders)
views.display.page (Page display options) list (Display extenders)
views.field.bulk_form (Bulk form) list (Available actions)
views.field.field (Views entity field handler) list (Group by columns)
views.field.term_name (Views entity field handler) list (Group by columns)
views.query.views_query (Views query) list (Query Tags)
views.style.default (Unformatted list) list (Grouping field number %i)
views.style.grid (Grid) list (Grouping field number %i)
views.style.rss (RSS Feed) list (Grouping field number %i)
views.style.table (Table) list (Grouping field number %i)
workflow.type_settings.content_moderation (Mapping) list (Entity types)

(edited for some more de-duplication)

Updating tag.

Bump — this would still be very valuable to do 🤓

So this is why I am still getting false-positive configuration diffs in my production deploys? I would have thought that #2361539: Config export key order is not predictable for sequences, add orderby property to config schema would have addressed this but this issue indicates to me there are some edge cases still remaining?

The weird part to me is that on export in my local development environment, I can export and then do a drush cim and there are no changes. But deploy that same code to production and I get the same set of consistent config entities that appear changed but actually aren't.

www-data@app-7947bcd569-rnz4j:~/html$ drush cim
+------------+---------------------------------------------------------------------+-----------+
| Collection | Config                                                              | Operation |
+------------+---------------------------------------------------------------------+-----------+
|            | commerce_product.commerce_product_variation_type.default            | Update    |
|            | commerce_shipping.commerce_shipment_type.default                    | Update    |
|            | core.entity_form_display.commerce_product_variation.default.default | Update    |
|            | core.entity_view_display.commerce_product_variation.default.cart    | Update    |
|            | core.entity_view_display.commerce_shipment.default.checkout         | Update    |
|            | core.entity_view_display.commerce_shipment.default.user             | Update    |
|            | search_api_solr.solr_field_type.text_en_6_0_0                       | Update    |
|            | search_api_solr.solr_field_type.text_en_7_0_0                       | Update    |
|            | search_api_solr.solr_field_type.text_und_6_0_0                      | Update    |
|            | search_api_solr.solr_field_type.text_und_7_0_0                      | Update    |
|            | views.view.commerce_cart_block                                      | Update    |
|            | views.view.commerce_cart_form                                       | Update    |
|            | views.view.commerce_carts                                           | Update    |
|            | views.view.commerce_products                                        | Update    |
|            | views.view.order_shipments                                          | Update    |
+------------+---------------------------------------------------------------------+-----------+

It's more noisy than anything but it would also be nice to have my CD logs only reflect actual changes. Or am I missing something here?

Hunch: different PHP versions sort certain arrays differently? What PHP version are you using in prod vs local?

Hi Wim, thanks for chiming in. This is containerized so everything "should" be the same and I just did a pull of my image locally to confirm and yes, same PHP versions locally and on production. I also thought perhaps the host environment was like, changing some sort of localization settings but I don't see any LC* environment variables... though I'm way out of my area of expertise there.

I was looking into this for a few annoying instances in our sites, and am not finding PHP differences. I'm thinking perhaps database differences? SELECTs with no ORDER BY...

We most often hit this issue on text format filters and static menu overrides. Today I'm seeing it on an entity_view_display layout builder settings.

I think this is going to need to be a plan because we really ought to add test coverage of everything.

I propose to add a new method to ConfigEntityValidationTestBase (and perhaps also \Drupal\Tests\jsonapi\Functional\ConfigEntityResourceTestBase to test it in an end-to-end/integration test). That way we don't have to start from scratch.

Most (but not all) type: sequence occurs in config entities anyway. For simple config, I think #2952037: [meta] Add constraints to all simple configuration is relevant.

I think that we should make sure that we have an database index for every query we add orderby().

@daffie Hah, I get that that triggered you but … it's completely unrelated 😅 This is about \Drupal\Core\Config\Schema\SequenceDataDefinition::getOrderBy(), which is used only in \Drupal\Core\Config\StorableConfigBase::castValue(). It never relates to database queries.

@Wim Leers: Ok, than it is fine. Just want to make sure that Drupal does not get slower. :)

Could we start adding predictability, at least, to config entity dependencies? See #3050223: Index configurations are changing from export to export.

On a large site with multiple developers I see the order of block visibility plugins switching around quite a lot. I have a CI step that ensures that config can be exported cleanly after import, which catches a bunch of issues, but sometimes block visibility order is different between developer laptops and CI for no obvious reason.

What do we need to do to move this forward?

I think the easiest would be to add some option to Wim Leers' config inspector to surface sequences that have no orderby setting.

Then the medium hard part is to find out which are the easy ones that could just have one of the currently available options of "key" or "value".
Then the hard part is to identify new "orderby" options (like key-then-value which core.extension:module is using) or strnatcasecmp for filter formats (see #10)
Maybe another one could by "property:weight" for sequence items that are sorted by weight.
Maybe we should also add "none" for sequences where the sort order in-itself is important data, which would allow us to tell other systems to leave it alone.
And after identifying more sort options we need to convert the sorting on save to a sorting by config schema

And finally we celebrate!

Note that adding a callback as a sorting option was dismissed because it would make the config schema more dependent on having a running drupal instance.

Thanks, opened #3479107: Identify sequences that have no orderby setting

Wouldn't it be preferable to deprecate the absence of orderby? 🤔 (Perhaps only for config schemas defined in Drupal core initially?)

I don't see what doing this in the Config Inspector module would do to help move this forward in a way that it won't regress again.

Currently working through issues

I see that this issue about 'sequence' configuration.

But is there a similar issue about 'mapping' or mapping-like records like config_entity?
It would be quite safe to order these by key, I reckon.

RE 41
The mappings are sorted by the order in which they are defined in the schema, so mappings are and can be completely predictable.

Re #42 (bircher)

The mappings are sorted by the order in which they are defined in the schema, so mappings are and can be completely predictable.

This would make sense.
And yet I see these keys getting shuffled.
Maybe the order is not working correctly in some operations.

I should collect more information and open a new issue.

EDIT: Seems if you import from a config/install/ where keys have the wrong order, that wrong order remains when you then export to config/sync. I notice this in a core.entity_form_display.*.*.*:content.*, where the keys like type, weight, region can be in the wrong order. (I don't know why the original import file was not ordered properly.)

I created #3576316: Config keys sometimes not ordered by schema on export.
I will do some more investigation there.