michealzh / solr-php-client

Automatically exported from code.google.com/p/solr-php-client
Other
0 stars 0 forks source link

Plugable Query Writer Interface #3

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Since Solr (1.4) now offers php output, I decided to get the class to use
it.  In addition, I tried to make it a little flexible so that you can
switch which wt to use.

See the attached path.  It should not affect current behavior in anyway. 
The constant in the class is now SOLR_DEFAULT_WRITER and is set to php
(serialized php didn't work for me, PHP was also a royal PITA due to the
array to object conversion stuff).

If you want to change back to using json, simply call setQueryWriter('json').

Original issue reported on code.google.com by jacobsi...@gmail.com on 9 Mar 2009 at 9:01

Attachments:

GoogleCodeExporter commented 9 years ago
Well, discussing with Jacob now - I think using the PHP type is a bit hazardous 
in
terms of security.

There is a flag to pass to json_decode to get it to emit all arrays - otherwise 
we
should try to fix upstream in Solr so that it emits serialized objects.

Original comment by pwola...@gmail.com on 9 Mar 2009 at 9:10

GoogleCodeExporter commented 9 years ago
Added another patch.  This sets the default back to json and also includes a 
fall
back library for < 5.2 PHP installs.

Original comment by jacobsi...@gmail.com on 9 Mar 2009 at 9:26

Attachments:

GoogleCodeExporter commented 9 years ago
There seem to be a number of whitespace issues with the patch (tab vs space).

An alternative to that class would be the (likely better supported) Zend class: 

http://framework.zend.com/manual/en/zend.json.html

However, I'm not sure in either case that one should include the external 
library
with into this one - a PHP lib should be a last resort for those on PHP 5.1 who
cannot install the PECL extension.

Rather than coercing the PHP response into objects, the extra param could be 
based to
json_decode() to make it return arrays.

Original comment by pwola...@gmail.com on 10 Mar 2009 at 12:10

GoogleCodeExporter commented 9 years ago
The Zend class is fine.  It doesn't really matter to me.  We just need some 
type of
backup which works and quickly.  This one seems to.  If you feel so called to 
figure
out the Zend plugin / licensing, go ahead but it seems like not an optimal use 
of time.

I feel we should bundle the backup along with the JSON writer.  That's pretty 
much
where it belongs.  It's not a huge crime.

I don't like the json_decode issue because it breaks everyone's client code who 
is
using this module.  I feel like BC is important and this is not worth 
introducing risk.

Original comment by jacobsi...@gmail.com on 10 Mar 2009 at 1:13

GoogleCodeExporter commented 9 years ago
here's a simple patch to require the Zend class if using PHP 5.1 without the 
PECL
extension.  Not actually tested yet, since I have PHP 5.2, nor am I even sure 
this
should be committed - PHP 5.1 users could just patch or do this themselves.

Original comment by pwola...@gmail.com on 10 Mar 2009 at 10:17

Attachments:

GoogleCodeExporter commented 9 years ago
I *believe* Zend_Json_Decoder will use json_decode if it exists, so the 
Zend_Json patch would create infinite 
recursion.

As for the plugable parsers, that was actually how the client was originally 
written. I simplified it to just JSON 
after feedback from the Solr developers (Yonik and Grant). I'd prefer to not 
have to support more than one 
format. As for whether that format will be PHP, Serialized PHP, or JSON should 
be based on the native parsing 
speed of each and greatest overall support (across PHP versions and Solr 
versions).

Keep in mind also that the inclusion of json_decode might not be the only 
function in the code that requires 
PHP >= 5.2. I will have to run through it and check, but if there are other 
functions that require a PHP 5 
version then the point of using PHP result formats for 4.3 compatibility may be 
moot. They should only be 
considered if they can marshalled by the server and parsed by the client faster 
than JSON responses. 

Original comment by donovan....@gmail.com on 13 Mar 2009 at 10:56

GoogleCodeExporter commented 9 years ago
Hi Donovan,

The only way we can harness our user base (which does include some 5.1 people) 
to
test it is to apply a json compat layer.

Whatever we provide, it would be nice to bundle it with the library for 
usability
purposes.  Since the Zend Framework is BSD, this should be fine, right?

Best,
Jacob

Original comment by jacobsi...@gmail.com on 18 Mar 2009 at 6:22