[WIP] Riak 2.x rewrite

[FabioBatSilva]

As we discussed earlier on : https://github.com/basho/riak-php-client/issues/85

This rewrite is inspired by the 2.0 java api,

Some of the new features :

• conflict resolvers
• domain object converters
• guzzlehttp to communicate with riak via http.
• riak-pb / protobuf-php to communicate with riak via Protocol Buffers.

Example :

use Basho\Riak\Cap\RiakOption;
use Basho\Riak\Command\Kv\StoreValue;
use Basho\Riak\Core\Query\RiakObject;
use Basho\Riak\Core\Query\RiakLocation;
use Basho\Riak\Core\Query\RiakNamespace;

// create a client/cluster
$builder = new RiakClientBuilder();
$client  = $builder
    ->withNodeUri('http://192.168.1.1:8098')
    ->withNodeUri('proto://192.168.1.2:8087')
    ->build();

// object location
$namespace = new RiakNamespace('bucket_name', 'bucket_type');
$location  = new RiakLocation($namespace, 'object_key');

// store object
$object   = new RiakObject();
$store    = StoreValue::builder($location, $object)
    ->withOption(RiakOption::PW, 1)
    ->withOption(RiakOption::W, 2)
    ->build();

$object->setValue('[1,1,1]');
$object->setContentType('application/json');

$client->execute($store);

// fetch object
$fetch  = FetchValue::builder($location)
    ->withOption(RiakOption::NOTFOUND_OK, true)
    ->withOption(RiakOption::R, 1)
    ->build();

$result = $client->execute($fetch);
$object = $result->getValue();

// delete object
$delete  = DeleteValue::builder($location)
    ->withOption(RiakOption::PW, 1)
    ->withOption(RiakOption::W, 2)
    ->build();

$client->execute($delete);

[FabioBatSilva]

ping @christophermancini @MarcoDeBortoli

[christophermancini]

Hey @FabioBatSilva, sorry for the delay I took some time off plus we had a company holiday for MLK day. So, I haven't had time to review your code in full depth yet but I certainly see some components that you have done that are an improvement over what I have. I will review it in more depth over the next few days so that we can identify exactly what components of the code we want to merge.

I know for the short term, we are not going to incorporate PB into the initial 2.x client release due to the requirement of third party PHP extensions. I realize this isn't a requirement for centraldesktop's PB lib, but I felt that his lib was the least user friendly of the available options. We intend to make the API interface layer extendable so that PB can be utilized by developers on their own or implemented as part of the lib by us at a later date. It reduces the barriers of entry and keeps the library simpler.

[FabioBatSilva]

No Problem @christophermancini,

At this point I'd like to suggest you do the other way around, My fork is missing several commands but basho:2.0.x is still a bit far to be a testable client.

So i think that would be better to implement in my fork anything you find necessary to get the client API working and then we can continue developing the remaining missing commands.

For the transport layer i already have a point of extension RiakAdapter hides the transport layer from the client and allow as to plug any transport layer we want. See : proto / http

As for the centraldesktop's PB it does not require the PHP extensions to work it use protoc plugin to generate PHP classes but after that is just php code.

I'm not sure about other implementations of PB for PHP all other implementations a found are based on drslump/Protobuf-PHP AND/OR very young and immature.

i'd like to get the PB implementation as part of the lib. Can't think on a better place for it.. :) but for now we should probably focus on http and later we can finish the proto as long as wee keep all the transport logic inside RiakProtoAdpter and RiakHttpAdpter implementing one or another should be easy.

[marksteele]

This is awesome Fabio! The old code was really not ideal from an architecture perspective, this is light-years better.

[debo]

Yeah sorry for the delay on my side too but I've been very busy lately. I think it will take me a while to go through this PR it's quite big, but well done for the effort. I'll give you some feedback as soon as I'm done.

A first suggestion however might be to split this PR in smaller one so it's going to be easier to decide what to keep from your fork and what from Christopher one.

[debo]

@marksteele the old code wasn't ideal... full stop ;)

[marksteele]

@MarcoDeBortoli Just glad that some of the ideas are finally moving forward. I had a working protocol buffer client 2 years ago that supported the entire pb interface at the time as well as streaming result sets. (https://github.com/marksteele/riak-php). Mind you, my code was terrible...

[debo]

@marksteele definitely! Don't get me wrong, I'm very happy that things are moving forward now and that there is a more active interest from Basho to push things forward also on the PHP side. Previously it didn't seem like there was a dedicated dev on their end to accept PRs promptly enough and even my old PRs took a while to get accepted.

[christophermancini]

We are working hard to rally support behind PHP (my focus), C#, and Node. We are also making pushes on better multi-environment testing that more closely match what our customers use in production. Unfortunately, these changes take time with a small team (that keeps growing, I came onboard in October).

So, the major downside of this PR is that it is completely incompatible with the rewrite I started back in October (branch - 2.0.x). This creates a difficult situation because I either have to scrap my work or tell you no, and neither one is exciting. There are many items to consider objectively.

The ultimate goals of the 2.x rewrite are to build a simple, extensible and stable library for the PHP community. By keeping it simple, it improves maintainability, keeps low barriers for community members to make contributions, and reduces hurdles experienced when expanding support for new versions of PHP. Making the lib highly extensible allows app developers to make the lib work the way they need it to while keeping the lib lightweight. Awareness is low for Riak in the PHP community as a whole, which means that we need to ensure that this new library is stable as first impressions are everything.

Here are the reasons I can see to approve this PR:

• Quicker time to market with a usable refactored PHP client
• Would encourage other community contributions
• Port of the well thought out Java client
• Use of docblock comments, code coverage and tests

Here are the reasons I can see to deny this PR:

• File / class naming.
  • Multiple (intentional?) misspellings, e.g. "RiakHttpAdpter implements RiakAdapter", RiakCusterTest, etc.
  • Lack of descriptors on abstract classes and interfaces (writing this identified I have to fix my abstract classes...)
  • Repetition, e.g. \Basho\Riak\RiakClient, \Basho\Riak\Core\RiakClient
  • Inconsistencies, e.g. \Basho\Riak\Core\Operation\Bucket\FetchPropertiesOperation vs \Basho\Riak\Core\Query\Crdt\CounterOp, use of Crdt and DataType, "class DomainObjectList extends RiakList" (Riak prefix).
• Several areas for improving code reuse, could have made use of traits
• Issues with centraldesktop/Protobuf-PHP lib's message class generator / wrapper which result hacking the classes into the lib. Proto files for Riak change frequently leads to maintenance hell. Libs like chobie/php-protocolbuffers or allegro/php-protobuf have better message generators but require a third party extension for operation.
• Feels like a Java lib. Complex for devs who only know PHP which forces relying heavily on inline documentation. Reduces readability, maintainability and community involvement (no one wants to write docs).

@FabioBatSilva your effort in such a short time is awesome and I truly appreciate your contributions. Although your code offers strong extensibility, I fear that it is not simple, which affects community involvement, maintenance requirements, and finding issues that affect stability. I am going to have to deny this PR.

I do think however that the community at large can benefit from your library and encourage you to make it available as its own lib as well as adding it to packagist.

[FabioBatSilva]

It's a shame basho is not open to contributions, Riak is an awesome product that i've been using if for quite some time.

But the official client 1.4 for php is terribly written and maintain and now i'm seeing the same thing happening again, based on what i've seen so far on the basho:2.0.x rewrite i can expect the same poorly written/tested code for the 2.0 client. Witch in my opinion is terrible for all php community using riak and for basho as a company.

I'm open to improve/discuss all your points but you closing the PR shows that basho is clearly not open for contributions. I'm a active contributor on the php community and i know for sure that this is not the right way to maintain a open source project.

[mbbroberg]

@FabioBatSilva I want to thank you for your feedback and offer a little context.

Client libraries have fluctuated between company supported and community supported with blurry lines. @christophermancini recently joined to give the PHP version a makeover and he has done a great deal of work behind closed doors. It's really unfortunate that we didn't communicate that to you and other members of the community so that you didn't get stuck in this in-between time. Your code adds so much value that we need to keep in mind.

We at Basho can and will do better than this in the future.

I'm working on clarifying PR strategy and still need to coordinate how we ensure our libraries are in a state that makes public contribution easy. I'm sincerely sorry to see this happen and would love any and all feedback for the future: mbrender at basho.com or message me on Twitter.