When trying to revert a feature the following errors can occur:

[pieter@archlinux]$ drush fr test_domains -y --force
Do you really want to revert domain? (y/n): y
Illegal string offset 'machine_name' domain.features.inc:77
Illegal string offset 'subdomain' domain.module:923
Illegal string offset 'machine_name' domain.module:923
Illegal string offset 'machine_name' domain.module:926
Illegal string offset 'subdomain' domain.module:951
Illegal string offset 'subdomain' domain.module:954
Reverted domain.
Do you really want to revert domain_alias? (y/n): y
Reverted domain_alias.

This only occurs when an option has been saved in the feature (for example the "wipe-domain-tables" option). When the feature is being rebuilt these options are iterated over as if they were domain data.

CommentFileSizeAuthor
#19 1689692-domain-feature-offset-follow.patch1.58 KBagentrickard
#14 1689692-14-domain-feature_revert_string_offset.patch740 bytespfrenssen
#9 1689692-9-domain-feature_revert_string_offset.patch797 bytespfrenssen
#6 1689692-features-domain-offset.patch540 bytesagentrickard
#1 1689692-1-domain-feature_revert_string_offset.patch1.75 KBpfrenssen

Comments

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Active » Needs review
StatusFileSize
new 1689692-1-domain-feature_revert_string_offset.patch1.75 KB

Here's a fix.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

This would affect to the other exportables as well, yes?

pfrenssen’s picture

pfrenssen
Location Sofia
commented

I just exported basic domains and aliases, and this fix only covers this use case. I have not looked at the other submodules to see if they have feature support and would be affected by this. More specifically this patch fixes the function domain_features_rebuild() which is called when domains are reverted.

I will be doing more work with the Domain Access module this week but am on a rather tight schedule so I'm not sure if I'll have the time to look at this again.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

OK. We will need to check the other modules, though.

pfrenssen’s picture

pfrenssen
Location Sofia
commented

Just a note, I just checked Domain Theme and it reverts without warnings.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
StatusFileSize
new 1689692-features-domain-offset.patch540 bytes

In the other modules, we use this much simpler code. Since the 'wipe-domain-tables' string cannot be a maching name, we can use the simple version for consistency.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs review » Fixed

Committed!

4ae703e..515e0c1 7.x-3.x -> 7.x-3.x

Status: Fixed » Closed (fixed)

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

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Closed (fixed) » Needs review
StatusFileSize
new 1689692-9-domain-feature_revert_string_offset.patch797 bytes

This is not fully solved. The patch in #6 checks for a valid domain id but still executes a domain_save() if no valid id was found. Attached patch avoids this.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

That part is deliberate, unless it is throwing an error. What that code says is "if a domain_id is defined, use it."

domain_save() checks for whether a domain_id exists or not.

Without that code, you could never create new domains from a feature.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

More accurately, the code says:

-- Check to see if this machine_name (the $key) matches an existing entry.
-- If so, add the domain_id so domain_save() runs an update.
-- Else, run domain_save() to create the domain.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Assigned: Unassigned » pfrenssen
Status: Needs review » Postponed (maintainer needs more info)

I am in the process of updating to 7.x-3.7 and the "Illegal string offset" errors reappear (I was using my patch from #1 up to now). If I remember correctly domain_save() was being called with the settings from "wipe-domain-tables" instead of a valid array of domain settings. I'll provide some more info when I get back to the office on Monday.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

The "wipe" functionality should be separate. That code checks to see that all domains in the target table have machine names that match those in the feature. If not, the domain is deleted.

The save routine then runs for each domain in the feature.

It is still possible, I suppose, to have an offset error, but I don't see where.

pfrenssen’s picture

pfrenssen
Location Sofia
commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new 1689692-14-domain-feature_revert_string_offset.patch740 bytes

You're right, this would prevent domains from being created. The solution is to simply unset the 'wipe-domain-tables' key after domains are wiped so this key is not looped over as if it were a domain.

On a side note, if it is not too much trouble, could you pass on the --author flag when committing contributed patches so these appear in the the developer's user profile? I love seeing these, and it motivates me to contribute more in the future :)

You can find some more information and the right --author flag to pass in my user profile under "Git attribution". Thanks!!

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs review » Reviewed & tested by the community

That makes sense.

I flag --author on patches I consider to be significant effort. A one-liner doesn't normally qualify.

pfrenssen’s picture

pfrenssen
Location Sofia
commented

No problem at all, I thought you were perhaps not aware that git attribution was possible.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

:-) https://drupal.org/node/99990/committers

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Committed.

   ce709e0..7cd7e52  7.x-3.x -> 7.x-3.x

This actually would affect all the features handlers, so follow-up patch coming.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Reviewed & tested by the community » Fixed
StatusFileSize
new 1689692-domain-feature-offset-follow.patch1.58 KB

And the followup.

   7cd7e52..fcaa70c  7.x-3.x -> 7.x-3.x

Status: Fixed » Closed (fixed)

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