Hi there,

I tried to make your module work and so far it fails... I do not get anything different from before!? So I'm still looking into the reason why that would happen...

Anyway, while testing, I added a domain (really, two sub-domains: http://linux.m2osw.com and http://win32.m2osw.com ) and I was trying to get the Win32 to say: Windows Geek instead of Linux Geek and have a different home page with such corrections. I always get the Linux page which is the default domain so that makes sense, but it means it fails finding the Windows entry.

Thinking maybe my setup did not work, I tried to re-edit the settings and I saw that it was all reset to defaults. I tried entering the data again and again it failed. I checked the DB and I could see the data now... Hmmm...

The problem with the Domain Conf is that you defined the settings field as "bytea" (binary) and that fails badly with PostgreSQL when you use special characters in strings. Since I have newlines in my footer and whatever other entry, it transformed those to \015 and \013 or something like that. The serializes worked but kept \r\n and when saved in the database, changed to \013\015 which means, all of a sudden, the serialized data is invalid (i.e. the strings have an extra 6 characters for each newline.)

To fix the problem I use text fields instead of bytea. Changed the %b into '%s' and now that part works. Although, I still don't get the domain switch... Just thought maybe if I were to clear the entire cache it would work better... let's see...

Anyway, if you accept to change the bytea to text, let me know, I can produce a patch for you.

Thank you.
Alexis Wilke

CommentFileSizeAuthor
#15 686146-decode-blob.patch7.71 KBagentrickard
#12 686146-decode-blob.patch7.97 KBagentrickard
#4 domain-2.1-postgresql-6.x.patch7.46 KBAlexisWilke
#3 685872-pgsql-b.patch5.66 KBagentrickard

Comments

AlexisWilke’s picture

AlexisWilke commented

Okay... clearing the cache did not make any difference.

Which function do you call to know which domain to use? Is that done in the bootstrap or later?

I saw domain_lookup_simple() that looked like the right function. It isn't called for the win32 site, but it is for the linux site... Strange!

Thank you.
Alexis

agentrickard’s picture

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

Yes, we need pgSQL testers. (And Windows server testers, too, I suspect.)

But wouldn't the %b failure cause core problems, too? Or is it just a problem because of how the INSERT is written? Lines 165 of domain_conf.admin.inc:

  if ($check > 0) {
    $sql = "UPDATE {domain_conf} SET settings = %b WHERE domain_id = %d";
    db_query($sql, serialize($settings), $form_state['values']['domain_id']);
  }
  else {
    $sql = "INSERT INTO {domain_conf} (domain_id, settings) VALUES (%d, %b)";
    db_query($sql, $form_state['values']['domain_id'], serialize($settings));
  }

So a patch that changes %b to '%s' seems reasonable to me.

As for loading the domains, make sure you added the include lines to settings.php correctly. DA has to add its own bootstrap phase.

And please check the tone of your comments. It's a little off-putting.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Active » Needs review
StatusFileSize
new 685872-pgsql-b.patch5.66 KB

Here's a patch against 6--2 branch. May not work against the release tarball, due to 'attic' files in cvs.

If this doesn't break anything in MySQL, it should be good to go.

The second problem is likely server misconfiguration and should be addressed in a separate issue after checking INSTALL.txt.

AlexisWilke’s picture

AlexisWilke commented
StatusFileSize
new domain-2.1-postgresql-6.x.patch7.46 KB

Agentrickard,

Note that just changing %b into '%s' isn't enough. I also changed the tables to use 'text' instead of 'blob'. And that may be a big problem with converting an existing table to a new type of table. As far as I know you cannot just change the column type. You'll have to do that in four steps:

1. Create a new column
2. Copy the existing data from the old column to the new column
3. Drop the old column
4. Rename the new column as expected (i.e. old column name)

And this for the two tables that use this feature.

Now, on why bytea (blob) in PostgreSQL does not work right, I'm not too sure. I think this is a bug in the Core and not in your module, but before the core fixes it... a lot of water will flow under the bridge. (Hmmm... French expression?!) I have had problems with that type with two other modules.

In this case, it specifically breaks when there are \r\n in the strings. If you don't have any newlines, then it is likely that it will work just fine.

I'm attaching my own patch which is very close to yours. It adds a couple more %b -> '%s' fixes and fixes to the tables.

However, the problem will be to get people from 'blob' to 'text' in order to support the %s. For this, I think you need several people to test the patch on both PostgreSQL (although for them, reinstalling would probably be just fine...) and MySQL.

Thank you.
Alexis Wilke

P.S. There may be a way to fix the blob, btw. I just did not find one before...

agentrickard’s picture

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

Which begs the question: Why do you have '/r/n/' strings in these text fields?

This may simply need to be noted and fixed for 7.x.

agentrickard’s picture

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

I looked back at D6 core, and this decision in DA was modeled on the {cache} table in Drupal, which is a longblob,

agentrickard’s picture

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

And it is in D7 as well.

AlexisWilke’s picture

AlexisWilke commented

Those fields show as textareas that are transformed into FCKeditor areas automatically. I don't see a problem with that since it lets me enter text with all sorts of things such as bold, italic, paragraphs, etc.

That will pretty much always include a \r\n to separate paragraphs.

Now, if you want to keep the blob, you may want to do the same thing as the cache is doing:

      $cache->data = db_decode_blob($cache->data);
      if ($cache->serialized) {
        $cache->data = unserialize($cache->data);
      }

I think that db_decode_blob() is what would fix the \r\n back to the way it needs to be, before the call to the unserialize() function. That could be a much better fix!

What do you think?

Thank you.
Alexis

agentrickard’s picture

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

I think WYSIWYG editors are the bane of civilization.

A patch with db_decode_blob() would be great.

Even better, abstract it a bit so we have domain_decode_blob() that we can call from any of the modules (Domain Theme, Domain Conf) that use it.

e.g.

$result = // QUERY;
$data = domain_decode_blob($result);

-or-

$data = domain_get_blob($query);
agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs review » Needs work
AlexisWilke’s picture

AlexisWilke commented

Okay, that sounds good! I will have to convert my database back though, thus it will take a little time. I should have a patch within a few days.

On my end, I love the WYSIWYG editors. It makes life so easy! Much of the formatting I do with that would not be done if I did not have it. It is a given.

Thank you.
Alexis

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs work » Needs review
StatusFileSize
new 686146-decode-blob.patch7.97 KB

And a patch!

Works fine on MySQL.

davidwhthomas’s picture

davidwhthomas commented
Status: Needs review » Reviewed & tested by the community

Thanks!

This patch solved a tricky issue I was having with domain_conf and getting domain settings with domain_conf_variable_get

The only adjustment was I needed to put domain_unserialize in the file domain.bootstrap.inc - otherwise the function wouldn't be found.

Tested and working.

DT

agentrickard’s picture

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

I had thought about that. Shouldn't domain.module be loaded? O well. Back to needs work, since that's a change.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented
Status: Needs work » Needs review
StatusFileSize
new 686146-decode-blob.patch7.71 KB

Revised patch. I'd like to commit this and release 6.x.2.2.....

davidwhthomas’s picture

davidwhthomas commented
Status: Needs review » Reviewed & tested by the community

New patch is all good.

Thanks again for the great module.

DT

agentrickard’s picture

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

Committed!

AlexisWilke’s picture

AlexisWilke commented

Thanks guys for taking the time on this one... I got sucked up on other things on my end!?

Best,
Alexis

Status: Fixed » Closed (fixed)

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