Add support for IbmDb2 (IBM i) #565
Conversation
… line 101, to be compatible with ibm_db2
…ray keys (due to caps field names) -- some debug
…QL/DDL section, getBuildSql().
|
|
||
| // this is how ZF2 handles connection errors | ||
| if ($connection === false) { | ||
| throw new Exception\RuntimeException(sprintf( |
There was a problem hiding this comment.
This will throw a namespace error, as it will be looking for OAuth2\Storage\ Exception\RuntimeException
There was a problem hiding this comment.
Would \Exception\RuntimeException (add slash in front) be acceptable here?
There was a problem hiding this comment.
no, the PHP class is \RuntimeException (see here)
There was a problem hiding this comment.
Yes, I tested \RuntimeException successfully today. Thanks.
|
Wow, this is impressive! I am unfamiliar with IBM DB2... how can we add tests for this? It is easy to add tests for new storage engines in the test suite... you just need to add the class to the BaseTest::provideStorage method, and all the applicable storage tests will be run. |
|
It seems that Pdo's tests use sqlite; DynamoDb uses mock objects. Are there any tests with something similar to ibm_db2 that requires a real server? The ibm_db2 extension is not OO, so might not be easy to mock. |
|
@alanseiden DynamoDB doesn't use mock objects in the storage tests - it calls a real DynamoDB instance that I've set up with my Amazon credentials. I had to add an exception so it only runs against php 5.5 in travis ci - otherwise the tests ran 6 times and conflicted with each other. Is it possible to test against a local (travis) or remote (cloud) instance of IBM DB2? |
|
|
||
| // use persistent or not | ||
| $isPersistent = $connection['persistent']; | ||
| $connectFunction = ((bool) $isPersistent) ? 'db2_pconnect' : 'db2_connect'; |
There was a problem hiding this comment.
we should verify here that these functions exist - if they don't, we should throw an exception requiring the ibm_db2 extension
There was a problem hiding this comment.
I've added a check for the ibm_db2 extension using extension_loaded(), the technique used in ZF2. Sufficient or should we also check for the functions?
|
Is there anything I can do to help this along? I am waiting to hear back from you concerning the testability of IMB-DB2 |
|
IBM Db2 requires a real server. Clark Everetts of Zend wrote tests against a real IBM i server for ZF2's Zend_Db, so it's possible to do. |
|
I'm going to see Clark this week at a conference. Perhaps we can put our heads together. |
|
@alanseiden any progress on this? |
|
Today I did find a couple of bugs in the DDL and know how to correct the Brent Shaffer wrote:
|
| $result = db2_fetch_assoc($stmt); | ||
|
|
||
| // make this extensible | ||
| return $result && $result['client_secret'] == $client_secret; |
There was a problem hiding this comment.
Is the purpose of the expression to test if $result exists, and if so, perform the == comparison? The == comparison operator has higher precedence than && logical operator, meaning if I have the intent correct, the expression should actually be $result && ($result['client_secret'] == $client_secret). In this way, PHP's short-circuit boolean evaluation can be applied without undefined variable notice. Actually, the effect is the same: == will be evaluated before &&. I wonder if isset() is required here; will have to test. Or have I missed something obvious (it wouldn't be the first time)?
|
PDO is able to work with IBM DB2 http://php.net/manual/en/ref.pdo-ibm.php |
|
excellent find! We should add IBM DB2 to the travis tests then, and we can close this sucker. |
|
I don't understand how PDO would help with testing of the ibm_db2 extension. |
|
Using the PDO storage class and the |
|
Hello @alanseiden We insert the public / Private keys into the oauth_public_keys table. If we want to generate a new access token, we got an error. When i change this to maybe its a performance improvements if we add |
Create storage class for the ibm_db2 extension, which is commonly used with DB2 on IBM i. The class includes DDL optimized for IBM i (chiefly the "for system name" clause).