Convert queries to use DBTNG syntax

Me too having same issue. I am not able to add/edit/delete any node.

The current state of Expire 7.x is incomplete and still contains a lot of depricated 6.x database api calls. It's a work in progress not ready for production use. I hope to produce a better version in a few weeks time.

Here's a patch that fixes all occurences of db_result and db_rewrite_sql

This patch wasn't committed to the latest dev release which still contains all this legacy code?

First query should use query parameters properly. Doesn't make sense to concatenate the variables in and run check_plain on them.

I've re-rolled the patch from #5 and made a couple small changes. For the moment I've ignored the query that is in expire_node() which regards expiring CCK references as the 'nodereference' module from Drupal 6 CCK doesn't exist in Drupal 7 (See #1151684: Drupal 7 version of Cache Expiration? for more info).

I haven't looked at the queries in expire_get_domains() either in this patch as that concerns integration with the Domain module which I haven't used yet so I cannot test.

Patch in #9 does not fix the issue. Instead you'll get

Call to undefined function taxonomy_node_get_terms() in /drupal/docroot/sites/all/modules/expire/expire.module on line 307

Adding these links for later reference on replacing taxonomy_node_get_terms()

http://www.sagetree.net/news/getting-nodes-taxonomy-terms-drupal-7
http://drupal.org/node/909968

For reference about taxonomy_node_get_terms() deprecated in D7, see http://drupal.org/node/224333#taxonomy_node

A more complete patch has been posted on a duplicate issue here: http://drupal.org/node/1427668#comment-5600298

Re-posting Stefan Freudenberg's patch from the issue I mentioned in #13 above.

I've reviewed the patch and it works well for me.

The current 7.x-1.x-dev release is horribly broken. I'd like to see this committed so it can be used as a starting point to work towards a 7.x-1.0 release.

To be clear, this isn't my patch, I reviewed it, renamed it, and reuploaded it to this issue for clarity, hence setting this to RTBC.

+++ b/expire.module
@@ -598,7 +568,7 @@ function expire_path_redirect_load($where = array(), $args = array(), $sort = ar
     if (is_string($key)) {
       $args[] = $value;
-      $where[$key] = $key .' = '. (is_numeric($value) ? '%d' : "'%s'");
+      $where[$key] = $key .' = '. (is_numeric($value) ? '?' : '?');
     }

This should probably be rewritten properly to use the query builder for the whole query. At least the check is now pointless, a simple ? would be enough. I guess the query builder stuff can be postponed to another issue, unless you want to tackle it here..

+++ b/expire.module
@@ -688,14 +658,14 @@ function expire_get_base_urls(&$node) {
   if ($node->nid) {
-    $result = db_query("SELECT gid FROM {domain_access} WHERE nid = %d", $node->nid);
+    $result = db_query("SELECT gid FROM {domain_access} WHERE nid = :nid", array($node->nid));
     while ($row = db_fetch_array($result)) {
       $gid = $row['gid'];
       $domains[$gid] = $gid;
     }
   }
   elseif ($node->mail && $node->name) {
-    $result = db_query("SELECT domain_id FROM {domain_editor} WHERE uid = %d", $node->uid);
+    $result = db_query("SELECT domain_id FROM {domain_editor} WHERE uid = :uid", array($node->uid));

These two are missing :uid/:nid key in the arguments.

Otherwise looks ok, haven't done a detailed review on the field stuff.

I've update and reworked the patch from #14.
I have:

• updated the patch, per feedback from #15
• spun out non-core functionality where possible
• gotten rid of the path_redirect functions, per: http://drupal.org/node/451790
• I've incorporated http://drupal.org/node/1393258 for votingapi

Can someone review this?

Hi. I have improved the field stuff. Now uses the accessor function field_get_items and checks for return type to avoid warnings.

Accidental double post.

The last patch seems to be missing the newly added files. You need to use git add and then diff against HEAD. Or do a commit (e.g in a feature branch) and then diff against the main branch or against origin/$branch.

Sorry.

After git add I should use git diff --cached.

I see there's a directive in #21 to patch expire.votingapi.inc, which doesn't exist in the distro. I tried patching (using patch -p0 <) and got errors when I tried to save a new node. Am I doing something wrong? Following... thank you for doing this!

Does it work for you with #16?

Thanks, Stefan - the patch from #16 does execute successfully.

But on saving new content (in this case, an Event node), I get the following error:

Fatal error: Call to undefined function expire_get_node_references() in /home/[site]/public_html/sites/all/modules/expire/expire.module on line 295

I cleared caches before and after enabling Expire.

Hope this helps...

Hm. I have applied my patch against a clean HEAD of 7.x-1.x and it worked. It's the patch from #16 with only a few lines changed. Can you make sure your patch applied correctly and confirm the fatal error.

The patch did apply successfully and everything seemed OK until I tried to save a node with new content - it was at that point I got the error.

The way I'm patching is by running from command line in Terminal from within the module folder of the .dev release:

patch -P0 < expire-d7conversion-1333894-16.patch

I have to manually type in the individual files it's looking for, but with #16 everything proceeded without errors. More than this I don't know how to do - I don't understand .git, sad to say, and I have no idea how to patch against any kind of HEAD, clean or otherwise.

Have you tried saving new content after applying the patch? (I assume you have...)

Oh dear... sorry for being such a non-techie - we all do as best as we can... thanks!

Don't worry. I have used the new git patching procedure (http://drupal.org/project/expire/git-instructions), so you have to apply the patch with -p1. The error I think is introduced in #16. I'll look into this.

Thank you, Stefan. So just to be clear - I should wait until you've looked into the error and then apply the updated patch using -p1. I should not try patching using -p1 until you've investigated further, correct?

Thank you for the fast response and for all the work you're doing on this!

Yes. I have not fixed the issue you reported yet. I'll file a new patch.

@bcobin:

The easiest way to apply the patch is:
git apply expire-d7conversion-1333894-21.patch

The easiest way to apply the patch is:

Not for me, unfortunately. http://drupal.org/node/803746 looks like an absolute nightmare compared to how I work using Drush and patching syntax and, frankly, I've barely been able to make heads nor tails of the whole .git thing.

Could you possibly just post the patch file? Thanks much!

I'm beginning to think we should be attacking this more piecemeal, and simply rip out all domain, node_reference functionality, etc until someone tests it properly. However in the meantime...

I've taken Stefan's patch from #21, theoretically fixed the bits I updated (eg. inclusion of domain/node_reference code), refactored some of the domain code a little bit, and re-rolled. All wildly untested, but this should be better.

@bcobin:
You should be able to simply:
patch -p1 < expire-d7conversion-1333894-31.patch

Also see http://progit.org/book/ch5-2.html and http://progit.org/book/ch5-3.html

If you have errors with the patch, any help you can provide, such as watchdog messages, etc would be helpful.

Thank you, halyconCorsair - I've applied the patch successfully with the following message:

(Stripping trailing CRs from patch.)
patching file expire.admin.inc
(Stripping trailing CRs from patch.)
patching file expire.domain.inc
(Stripping trailing CRs from patch.)
patching file expire.info
Hunk #1 succeeded at 6 with fuzz 2.
(Stripping trailing CRs from patch.)
patching file expire.install
(Stripping trailing CRs from patch.)
patching file expire.module
(Stripping trailing CRs from patch.)
patching file expire.node_reference.inc

That's the good news. The bad news is that nodes don't get created. I tried creating a new node (in my case, an Event node) and saving returned to the add content page - no node was created. (And no error message.) After disabling expire, I was able to successfully create the node. Eep.

I did clear caches at pretty much every stage of the operation, just to make sure. So, at least in my case, it's not soup yet. :(

Thanks for the rapid response here... getting there, we hope!

Couldn't sleep, so I did some actual testing.

Attached is a patch which:

• fixes a bug (possibly the cause of #26) preventing creation of nodes with taxonomy
• radically reworks redirect and node_reference functionality to work correctly
• gets rid of a few bits of crufty code
• keeps all the good work we did in previous patches

Domains and votingapi support remains untested.

Patch applied fine, enabled the module and went to delete a test node. It did not delete.

Disabled the module and successfully deleted the node - so it looks like we've still got problems here... sorry to be the bearer of bad news!

P.S. - Did clear cache every step of the way...

New patch fixes a small warning caused by the last one possibly attempting to array_merge a nothing with an array.

@bcobin:
Couldn't reproduce your issue locally, can you provide more info?

Reroll the patch to include some bits that were accidentally left out.

Ack, reroll

@bcobin: This should fix your node deletion issue.

Applied #39 and did a simple test:

1. Get Varnish HITS
2. Add a new node promoted to the front page
3. New node appears OK on next refresh with a Varnish MISS
4. Next refresh gives Varnish HITS again as expected

Great. Then I tried simply changing a word in a body field for an article on the front page and while it's been successfully updated when hitting it directly, it wouldn't show updated on the front page even after several (hard) refresh.

Will test more next week...

@anavarre:

I performed the same test you did, and it works for me, I even checked the ban list which for me had:
req.http.host ~ test.domain.com && req.url ~ ^/node/9$|^/content/tawet$|^/$|^/rss.xml$|^/node$

eg. expire:

node/9
content/tawet
/
/rss.xml
/node

Setting fixed as part of 7.x-1.0-alpha1.

@anavarre:
If you are still having your issue, please file a new issue with enough details to reproduce.