For 1.0 we should try to sync the PHP client library so we can use the current rev.
In this issue, the library maintainer decided to use the method name deleteByMultipleIds() rather than what I suggested and put in the Drupal class, deleteMultipleById()
http://code.google.com/p/solr-php-client/issues/detail?id=21
We shoudl probably change our method name to match so that we can simply drop in the updated library and remove the method from the Drupal class.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | apachesolr-client-r22-628080-D5.patch | 15.82 KB | claudiu.cristea |
| #19 | comment_search_htmlentities.patch | 1.93 KB | robertdouglass |
| #18 | revision.patch | 1.11 KB | robertdouglass |
| #16 | drush.patch | 895 bytes | robertdouglass |
| #15 | phpclient.patch | 14.93 KB | robertdouglass |
Comments
Comment #1
pwolanin commentedComment #2
pwolanin commentedAnyone?
We should also think about syncing with the client PHP library before 6.x-1.0 is final.
Comment #3
pwolanin commentedoops - missed a call.
Comment #4
pwolanin commentedActually, Donovan made a new tarball available of r22: http://solr-php-client.googlecode.com/files/SolrPhpClient.r22.2009-11-09...
listed at http://code.google.com/p/solr-php-client/downloads/list
that includes this new multiple method (in r20). This version of the client also exposes it's svn revision, so we can potentially start using hook_requirements to make sure the library is present AND that it has the right version.
Apache_Solr_Service::SVN_REVISION is '$Revision: 22 $'
So - I propose we try to update to this r22 client, since that way we can also point end-users to the tarball download rather than forcing them to get it from svn.
Comment #5
pwolanin commentedAh, beating my head on a wall for a bit. It turns out that the hook_requirements implementation MUST be in the .install file for it to be checked when installing the module.
apachesolr_strip_ctl_chars() can be removed now since the final xml document is filtered using our regex thanks to an upstream fix.
Comment #6
pwolanin commentedAlso update the README
Comment #7
JacobSingh commentedBarring testing that the new library actually works for the delete by function, this looks good to me.
Comment #8
pwolanin commentedWe can test that - generally I kill Solr and delete a few nodes frmo the admin interface. I tested the 1st patch this way with the simple rename, and the code in the PHp library looks right, so it shoudl be easy enough to re-confirm before rolling a release.
committed this patch to 6.x-1.x, let's see if it applies to 6.x-2.x...
Comment #9
pwolanin commentedapplies pretty clenaly, but need to manually remove the hook_requirements from apachesolr.module - guess it was elsewhere in the file.
Comment #10
pwolanin commentedoh CRAP - CVS messes with the $Revision $ keyword. need a minor fix.
Comment #11
pwolanin commentedHere's a fix.
Comment #12
pwolanin commentedcommitted that fix, here's a 6.x-2.x version also with that fix.
Comment #13
pwolanin commentedfix typo: CLient -> Client
committed directly to 6.x-1.x
Comment #14
robertdouglass commentedThis still needs to be committed to 6.2 right?
Comment #15
robertdouglass commentedThis is the patch I applied to 6.2.
Comment #16
robertdouglass commentedThe 6.1 branch missed the drush command.
Comment #17
robertdouglass commentedComment #18
robertdouglass commentedHmm. My patch also didn't have the revision checking fix in it. Applying this to 6.2 as well.
Comment #19
robertdouglass commentedComment search module in DRUPAL-6--2 has two instances of return htmlspecialchars(html_entity_decode($text, ENT_NOQUOTES, 'UTF-8'), ENT_NOQUOTES, 'UTF-8');
Comment #20
claudiu.cristeaCommitted in #317570.
And the patch against DRUPAL-5--2.