-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added a new file for Simple PHP interaction with Summon API #2
Conversation
1. Added more options while making a http request. 2. Modified code to make use of variable '$raw'.
A simple example for PHP implementation: <?php
$q = $_GET["query"];
if(isset($q)){
$format = "json";
header('Content-type: application/'.$format);
$apiId = "<api id>";
$apiKey = "< api key >";
require_once dirname(__FILE__) . '/php_classes/PHP.php';
$query = new SerialsSolutions_Summon_Query($q);
$summon = new SerialsSolutions_Summon_PHP($apiId, $apiKey, array('responseType' => $format));
echo $summon->query($query,'search' ,'GET',true);
} else {
?>
<html>
<head>
<title>Summon Finder</title>
</head>
<body>
<form>
<input type="text" name="query" /> <input type="submit" value="Go" />
</form>
</body>
</html>
<?php } ?> |
My only concern with this is that it changes the signature of query() in such a way that backward-compatibility is broken. Would you object to keeping $returnErr as the second parameter and adding the new parameters to the end of the list so that existing code won't break? |
Agreed. I have no objections to that. |
Great, thanks -- I'll take a closer look at this in the next day or two and get it merged soon. |
Awesome. Appreciate your fast response. :) |
I have incorporated most of your changes into master, with a handful of changes: 1.) I renamed your PHP class to CURL to be more specific -- since there are several different possible PHP approaches to HTTP, it seemed best to specify that you are using CURL. 2.) I removed the CURL class constructor since it was not necessary. 3.) I did not add all of the new parameters to the query() method. Only $raw seemed like a useful addition, since you don't send queries to a different endpoint, and using POST instead of GET seems to cause an error. I also added a $raw parameter to getRecord() for further flexibility. Please let me know if there is a use case I am overlooking. 4.) I made a few minor style changes to comply with PEAR standards (which have been used on all of the other files in this project). There is one small outstanding issue which may not be of great importance: your CURL class currently ignores the $method parameter of the httpRequest() method, so it always does a GET request. As far as I know, the Summon API does not currently work with POST requests, so this support may not be necessary -- but for future reference, it may be worth implementing it at some point. |
For #3, the variable '$service' seems useful to be parameterized as summon also supports other services like image and availability. A different handling may be required for different services; I will have a look at it in a few days. Different handling is required for different operations, and since summon only supports GET, I didn't implement POST/PUT/DELETE. In the future, at any point, if Summon releases support for other requests, we can incorporate them. |
If you want to access a service other than search, it would be better to simply access call() directly. Since the query() routine includes query processing logic that is irrelevant to services like image and availability, it doesn't seem appropriate to use it for those lookups. Perhaps the best solution is to simply make call() a public rather than a protected method, or to create some more generic public wrapper around it. |
I can see two options at the moment. 1.) We make call() public. I prefer option 2 as we can handle the services independently and leverage call() in those methods. |
Option 2 makes sense to me. Depending on the complexity of each call, it may or may not be necessary to define a class for input parameters -- in some cases it may be acceptable to simply use an array or a set of function parameters. In any case, if you want to build some methods that meet your needs and open new pull requests, I'll be happy to offer further suggestions. |
No description provided.