Skip to content
This repository was archived by the owner on Apr 5, 2020. It is now read-only.

Changes on google search client #18

Open
ui-kreinhard wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Changes on google search client #18

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
baae8a5
Expanded Search Client for retrieving clustered search responses
Sep 5, 2012
f82f8ee
Corrected api doc
Sep 5, 2012
e4518ed
Fixed apidocs, spacing
Sep 6, 2012
9583a53
fixed tab spacing
Sep 7, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions google/src/main/php/com/google/search/custom/GoogleSearchClient.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@
* @see http://www.google.com/cse/docs/resultsxml.html
*/
class GoogleSearchClient extends Object {
protected $conn= NULL;
protected $gsaURL= NULL;
protected $unmarshaller= NULL;

/**
* Constructor
*
* @param var conn either a HttpConnection or a string
Copy link
Member

@ohinckel ohinckel Sep 5, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API doc is wrong here, since the constructor only accepts an URL string now.

Copy link
Author

@ui-kreinhard ui-kreinhard Sep 5, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the hint :-)

*/
public function __construct($conn) {
$this->conn= $conn instanceof HttpConnection ? $conn : new HttpConnection($conn);
public function __construct($gsaURL) {
Copy link
Member

@thekid thekid Sep 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes it harder to test, because we can now no longer inject a MockHttpConnection for example. There are ways to get around this, but they suck.

Copy link
Member

@thekid thekid Sep 10, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a protected method to return a new connection instance, e.g.:

<?php
  // ...

  /**
   * Returns a new connection for a given URL. Override for testing purposes,
   * for example.
   *
   * @param  string url
   * @return  peer.http.HttpConnection
   */
  protected function connectionTo($url) {
    return new HttpConnection($url);
  }

  // ...
?>

We can then test by using a DummyHttpConnection for example by using:

<?php
  $fixture= newinstance('com.google.custom.GoogleSearchClient', array($url), '{
     protected function connectionTo($url) {
       return new DummyHttpConnection($url);
     }
  }');
?>

$this->gsaURL= $gsaURL;
$this->unmarshaller= new Unmarshaller();
}

Expand All @@ -49,6 +49,7 @@ public function __construct($conn) {
* @see http://www.google.com/cse/docs/resultsxml.html#WebSearch_Query_Parameter_Definitions
*/
public function searchFor(GoogleSearchQuery $query, $params= array()) {
$conn= new HttpConnection($this->gsaURL . '/search');

// Build query parameter list
$params['output']= 'xml_no_dtd';
Expand All @@ -59,17 +60,37 @@ public function searchFor(GoogleSearchQuery $query, $params= array()) {
($s= $query->getStart()) && $params['start']= $s;

// Retrieve result as XML
$r= $this->conn->get($params);
$r= $conn->get($params);
if (HttpConstants::STATUS_OK !== $r->statusCode()) {
throw new IOException('Non-OK response code '.$r->statusCode().': '.$r->message());
}

// Unmarshal result
return $this->unmarshaller->unmarshalFrom(
new StreamInputSource($r->getInputStream(), $this->conn->toString()),
new StreamInputSource($r->getInputStream(), $conn->toString()),
'com.google.search.custom.types.Response'
);
}

public function getCluster(GoogleSearchQuery $query, $params= array()) {
Copy link
Member

@thekid thekid Sep 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some APIdocs here - and think about including a link to the documentation (e.g. https://developers.google.com/search-appliance/documentation/52/QuickStart/quick_start_se#dynamicresclust - maybe use the newest version, but you get the picture), as I'm not sure everyone knows what search "clusters" are. The term "cluster" usually refers to a load-balanced group of 2..n machines configured in the same way and serving the same purpose, not "group[ing] search results by topic".

Copy link
Author

@ui-kreinhard ui-kreinhard Sep 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well , partly true - cluster is in textmining/datamining clustering has a different meaning. But you're definitly right, that there have to be a link to the google documentation

$conn= new HttpConnection($this->gsaURL . '/cluster');

// Build query parameter list
$params['output']= 'xml_no_dtd';
$params['coutput'] = 'xml';
$params['q']= $query->getTerm();
// Retrieve result as XML
Copy link
Member

@thekid thekid Sep 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline before comment according to our coding standards: "The line above a comment has to be an empty line."

$r= $conn->get($params);
if (HttpConstants::STATUS_OK !== $r->statusCode()) {
throw new IOException('Non-OK response code '.$r->statusCode().': '.$r->message());
}
// Unmarshal result
return $this->unmarshaller->unmarshalFrom(
new StreamInputSource($r->getInputStream(), $conn->toString()),
'com.google.search.custom.types.ClusterSearchResponse'
);

}

/**
* Creates a string representation of this object
Expand Down
43 changes: 43 additions & 0 deletions google/src/main/php/com/google/search/custom/types/ClusterSearchResponse.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/* This class is part of the XP framework
*
* $Id$
*/

$package= 'com.google.search.custom.types';

/**
* Search result
*
*/
class com�google�search�custom�types�ClusterSearchResponse extends Object {
protected $time= 0.0;
protected $clusterProposals=array();
/**
Copy link
Member

@thekid thekid Sep 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+Whitespace

* Set result set
*
* @param com.google.search.custom.types.ResultSet res
*/
#[@xmlmapping(element= '/toplevel/Response/cluster/gcluster/label/@data')]
public function setResultSet($proposal) {
$this->clusterProposals[] = $proposal;
}

/**
* Returns result set
*
* @return com.google.search.custom.types.ResultSet
*/
public function getClusterProposals() {
return $this->clusterProposals;
}

/**
* Creates a string representation of this result set
*
* @return string
*/
public function toString() {
}
Copy link
Member

@thekid thekid Sep 6, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @iigorr states - please make this return something meaningful

Copy link
Member

@ohinckel ohinckel Sep 7, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the toString() function is using a wrong indentation (maybe tabs instead of spaces).

}
?>