Added ability to call server side functions using orientdb php library

[rosenjon]

-Added ability to call server side orientdb functions
-Fixed bug related to rawurl encoding database name improperly (breaks nuvolabase and some orientdb setups)

[odino]

hi @rosenjon, thanks for the PR!

What about adding the test cases and changing the code a bit to follow the coding standards?

Cheers!

[rosenjon]

no prob. I'll update it tomorrow.

On Mon, Feb 4, 2013 at 8:05 PM, Alessandro Nadalin <[email protected]

wrote:

hi @rosenjon https://github.com/rosenjon, thanks for the PR!

What about adding the test cases and changing the code a bit to follow the
coding standards?

Cheers!

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/161#issuecomment-13113746.

[nrk]

Why?

Aside from the fact that this is wrong since path parts in URLs must always be encoded (that alone makes me totally against this change), I did a quick test and I don't see any difference when using $ or %24 in the database name since I always get 401 Unauthorized. back from the server (tested on a Linux system using OrientDB 1.2.0 and Java 1.7.0_09), so I believe this is rather a problem of OrientDB with the handling of database names with certain characters.

[rosenjon]

See:
http://www.lunatech-research.com/archives/2009/02/03/what-every-web-developer-must-know-about-url-encoding

The relevant section:

• "?" is allowed unescaped anywhere within a query part,
• "/" is allowed unescaped anywhere within a query part,
• "=" is allowed unescaped anywhere within a path parameter or query
  parameter value, and within a path segment,
• ":@-.~!$&'()_+,;=" are allowed unescaped anywhere within a path
  segment part,*
• "/?:@-.~!$&'()_+,;=" are allowed unescaped anywhere within a
  fragmentpart.
  *

rawurlencode() changes $ to its url encoded version. It is arguable whether
OrientDB should handle this or not, given the rules for url encoding.
However, the fact is that the server currently does not handle this
character when url encoded.

I could write a function that excepts the $ character from url encoding,
while treating the rest the same. This might be the correct compromise
position.

The easiest way to test this is to create a free account on Nuvolabase (
www.nuvolabase.com), and to use this library to access your database
instance. Shared databases on orientdb use $ to separate path parameters,
because all database names must be globally unique on a server. This is the
most obvious case of where this is a problem. I don't think you can have $
in the database name itself... this is an issue of the path to reach the
database (i.e. myusername$myfolder$mydatabasename);

On Tue, Feb 5, 2013 at 3:23 AM, Daniele Alessandri <[email protected]

wrote:

In src/Doctrine/OrientDB/Binding/HttpBinding.php:

@@ -58,7 +58,7 @@ protected function getLocation($method, $database = null, array $arguments = nul
$location = "http://{$this->server}/$method";

     if ($database) {

•        $location .= '/' . rawurlencode($database);
  

•        $location .= '/' . $database;
  

Why?

Aside from the fact that this is wrong since path parts in URLs must *
always* be encoded (that alone makes me totally against this change), I
did a quick test and I don't see any difference when using $ or %24 in
the database name since I always get 401 Unauthorized. back from the
server (tested on a Linux system using OrientDB 1.2.0 and Java 1.7.0_09),
so I believe this is rather a problem of OrientDB with the handling of
database names with certain characters.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/161/files#r2891256.

[rosenjon]

OK added changes we discussed in this thread.

Thanks,

Jonathan

[rosenjon]

I added another bug fix here. If you call the query command with a fetchplan but no limit specified, the query will fail. This is due to $limit being set to null by default in the query function. This fix defaults the limit to 20 when a fetchplan is used, thus fixing the problem. It might also make sense to set the $limit default to 20 in the query function, which would also fix the problem.

[odino]

hi @rosenjon,

would you be able to fix /provide the required testcases?

[rosenjon]

I wrote unit tests. Can you specify what needs to be fixed?

On Tuesday, May 7, 2013, Alessandro Nadalin wrote:

hi @rosenjon https://github.com/rosenjon,

would you be able to fix /provide the required testcases?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/161#issuecomment-17563878
.

[odino]

yep! You can see that the build on travis ci is failing, any idea what it could be?

[rosenjon]

I'll take a look tonight. It's been a while since I wrote this so not sure
off the top of my head. Tests passed when I wrote them, so unclear what's
up.

On Tuesday, May 7, 2013, Alessandro Nadalin wrote:

yep! You can see that the build on travis ci is failinghttps://travis-ci.org/doctrine/orientdb-odm/jobs/4950338,
any idea what it could be?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/161#issuecomment-17568483
.