Problem/Motivation

When conditionally-split configuration that depends on completely-split configuration is exported, it becomes completely-split itself and is removed from the config sync directory. This is confusing and sometimes undesirable behavior.

I think if someone conditionally-splits configuration, the module should respect that and not remove it from the sync directory. Otherwise, the user interface is not reflective of what is actually happening.

Proposed resolution

Split the logic in calculateBlacklist to defer calculating dependencies until calculateGraylist has run.
Add a new method to write add dependencies to the blacklist that aren't already covered by the graylist.

So the flow would be;

  • calculateBlacklist (but not dependencies)
  • calculateGraylist
  • calculateBlacklistDependencies (ignoring items not graylisted)

Remaining tasks

  • Decide that this is worthwhile and a valid issue (done, this is already the second issue in a short time, let's do this!)
  • Write test demonstrating the issue
  • Code a fix
  • Reviews etc

User interface changes

None.

CommentFileSizeAuthor
#7 config_split-exclude_graylist-3117841-7.patch2.39 KBmaximpodorov
#5 exclude-graylist-from-blacklsit-3117841-5.patch994 byteshgeshev

Comments

richardbporter created an issue. See original summary.

bircher’s picture

bircher
Location 🇨🇿
commented

in \Drupal\config_split\Plugin\ConfigFilter\SplitFilter::filterWrite in the blacklist case, we would not return NULL but check if the config is in the graylist, the source exists and return what is read from it. (like is done a fe lines later for the graylist)
Here: https://git.drupalcode.org/project/config_split/-/blob/8.x-1.x/src/Plugi...

We would of course also need some test for that.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Marked #3124911: Blacklist dependencies aren't aware of graylist entries as a duplicate

bircher’s picture

bircher
Location 🇨🇿
commented
Issue summary: View changes

Ok, ported over some of the duplicates issue summary.

hgeshev’s picture

hgeshev commented
StatusFileSize
new exclude-graylist-from-blacklsit-3117841-5.patch994 bytes

My usecase is: I have a complete split for a new field, but the content type where i'm adding it is part of default configuration. I put those files that should remain in the default folder in the conditional split, but because of the blacklist dependencies those files are moved to the brand folder and deleted from default.
I'm proposing solution pretty much the same as the one in the desctiption: I removed configs that are present in the graylist from the blacklist. Please review it and give me feedback if this is the correct approach.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented 
+++ b/src/Plugin/ConfigFilter/SplitFilter.php
@@ -363,6 +364,17 @@ class SplitFilter extends ConfigFilterBase implements ContainerFactoryPluginInte
+      if (in_array($name, $this->graylist)) {

nit:this should use the third arg to in_array because we're dealing with strings

maximpodorov’s picture

maximpodorov commented
Version: 8.x-1.4 » 8.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new config_split-exclude_graylist-3117841-7.patch2.39 KB

In this patch, the behavior from the issue description is implemented.

richardbporter’s picture

richardbporter commented
Title: Graylisted configuration removed from sync directory » Conditionally-split configuration removed from sync directory
Issue summary: View changes
maximpodorov’s picture

maximpodorov commented

Any thoughts about the patch? :)

bircher’s picture

bircher
Location 🇨🇿
commented
Status: Needs review » Fixed

I re-wrote the patch a bit because or #3157901: SplitFilter should not load black/gray lists in constructor

I also added a test.

  • bircher committed eddbfde on 8.x-1.x 
    Issue #3117841 by maximpodorov, hgeshev, bircher: Conditionally-split...

  • bircher committed eddbfde on 2.0.x 
    Issue #3117841 by maximpodorov, hgeshev, bircher: Conditionally-split...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.