Download & Extend

Illegal string offset errors when performing a feature revert

Project:Domain Access
Version:7.x-3.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:pfrenssen
Status:closed (fixed)

Issue Summary

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.

Comments

#1

Status:active» needs review

Here's a fix.

AttachmentSize
1689692-1-domain-feature_revert_string_offset.patch 1.75 KB

#2

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

#3

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.

#4

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

#5

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

#6

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.

AttachmentSize
1689692-features-domain-offset.patch 540 bytes

#7

Status:needs review» fixed

Committed!

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

#8

Status:fixed» closed (fixed)

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

#9

Status:closed (fixed)» needs review

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.

AttachmentSize
1689692-9-domain-feature_revert_string_offset.patch 797 bytes

#10

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.

#11

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.

#12

Assigned to:Anonymous» 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.

#13

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.

#14

Status:postponed (maintainer needs more info)» needs review

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!!

AttachmentSize
1689692-14-domain-feature_revert_string_offset.patch 740 bytes

#15

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.

#16

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

#17

#18

Committed.

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

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

#19

Status:reviewed & tested by the community» fixed

And the followup.

   7cd7e52..fcaa70c  7.x-3.x -> 7.x-3.x
AttachmentSize
1689692-domain-feature-offset-follow.patch 1.58 KB

#20

Status:fixed» closed (fixed)

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