Skip to content
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

Refactor transport classes #620

Open
milan opened this issue May 25, 2014 · 5 comments
Open

Refactor transport classes #620

milan opened this issue May 25, 2014 · 5 comments

Comments

@milan
Copy link
Contributor

milan commented May 25, 2014

With the addition of Elastica/Transport/Guzzle (in #618 ) which is an alternative to Elastica/Transport/Http (and Https), there are two classes that implement the ElasticSearch JSON over HTTP protocol. Which should allow some level of refactoring, following the discussion in #618 how about something very roughly along the lines of this:

abstract class AbstractHttpTransport extends AbstractTransport {

public function exec (Request $request, array $params);
    $connection = $this->_getHttpConnection();
    $request = $this->_createRequest();
    $response = $this->_sendRequest($request);
}

protected function _getBaseUri() {
    $url = $connection->hasConfig('url') ? $connection->getConfig('url') : '';
    if (!empty($url)) {
        $baseUri = $url;
    } else {
        $baseUri = $this->_scheme . '://' . $connection->getHost() . ':' . $connection->getPort() . '/' . $connection->getPath();
    }
    return rtrim($baseUri, '/');
}

protected function _setContent() {
    return (is_array($data)) ? JSON::stringify($data, 'JSON_ELASTICSEARCH') : $data;
}

abstract protected function _getHttpConnection();
abstract protected function _createRequest();
abstract protected function _sendRequest();
}

Then have Http and Guzzle extend AbstractHttpTransport and implement the abstract methods, although I'm fine with just extracting the common code into a BaseHttp class?

@ruflin
Copy link
Owner

ruflin commented May 26, 2014

The way you approach it above looks good to me. We should probably call it AbstractHttp.

@milan
Copy link
Contributor Author

milan commented May 26, 2014

Cool, i'll try and get this done this week.

@ruflin
Copy link
Owner

ruflin commented Dec 31, 2015

@milan I know, quite old. But are you still interested to push some changes here?

@killerwolf
Copy link

We should rely on PSR7/17/18

@ruflin
Copy link
Owner

ruflin commented Aug 12, 2019

I'm not too familiar with the new PSR standards but +1 on adopting them to standardise and make it easier for our users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants