Skip to content

Commit

Permalink
feat(cts): add client test for retry strategy (#2633)
Browse files Browse the repository at this point in the history
  • Loading branch information
millotp committed Feb 14, 2024
1 parent dd5abab commit 65bd154
Show file tree
Hide file tree
Showing 71 changed files with 1,377 additions and 506 deletions.
2 changes: 1 addition & 1 deletion .github/.cache_version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.0.11
1.0.13
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,43 @@ final class Host {
/// Host url.
final String url;

// url port
final int? port;

/// Host protocol (i.e. `http`, `https`)
final String scheme;

/// Whether this host should be used for [CallType.read] or [CallType.write] requests.
final CallType? callType;

/// Constructs a [Host] instance with the provided parameters.
const Host({required this.url, this.scheme = 'https', this.callType});
const Host({required this.url, this.port, this.scheme = 'https', this.callType});

factory Host.create({required String url, String scheme = 'https', CallType? callType}) {
if (url.contains(':')) {
final parts = url.split(':');
return Host(url: parts[0], port: int.parse(parts[1]), scheme: scheme, callType: callType);
}

return Host(url: url, scheme: scheme, callType: callType);
}

@override
bool operator ==(Object other) =>
identical(this, other) ||
other is Host &&
runtimeType == other.runtimeType &&
url == other.url &&
port == other.port &&
scheme == other.scheme &&
callType == other.callType;

@override
int get hashCode => url.hashCode ^ scheme.hashCode ^ callType.hashCode;
int get hashCode => url.hashCode ^ (port ?? 1) ^ scheme.hashCode ^ callType.hashCode;

@override
String toString() {
return 'Host{url: $url, scheme: $scheme, callType: $callType}';
return 'Host{url: $url, port: $port, scheme: $scheme, callType: $callType}';
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class DioRequester implements Requester {
Uri uri = Uri(
scheme: request.host.scheme,
host: request.host.url,
port: request.host.port,
path: request.path,
);
if (request.queryParameters.isNotEmpty) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
public final class Host {

private final String url;
private final int port;

private final Set<CallType> callTypes;

Expand All @@ -16,7 +17,12 @@ public Host(@Nonnull String url, @Nonnull Set<CallType> callType) {
}

public Host(String url, Set<CallType> callType, String scheme) {
this(url, callType, scheme, -1);
}

public Host(String url, Set<CallType> callType, String scheme, int port) {
this.url = url;
this.port = port;
this.callTypes = callType;
this.scheme = scheme;
}
Expand All @@ -26,6 +32,10 @@ public String getUrl() {
return url;
}

public int getPort() {
return port;
}

@Nonnull
public Set<CallType> getCallTypes() {
return callTypes;
Expand All @@ -44,13 +54,15 @@ public boolean equals(Object o) {
Host host = (Host) o;

if (!url.equals(host.url)) return false;
if (port != host.port) return false;
if (!callTypes.equals(host.callTypes)) return false;
return scheme.equals(host.scheme);
}

@Override
public int hashCode() {
int result = url.hashCode();
result = 31 * result + port;
result = 31 * result + callTypes.hashCode();
result = 31 * result + scheme.hashCode();
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ public String getHost() {
return host.getUrl();
}

public int getPort() {
return host.getPort();
}

public String getScheme() {
return this.host.getScheme();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ public Response intercept(@Nonnull Chain chain) {
/** Processes the request for a given host. */
@Nonnull
private Response processRequest(@Nonnull Chain chain, @Nonnull Request request, StatefulHost host) throws IOException {
HttpUrl newUrl = request.url().newBuilder().scheme(host.getScheme()).host(host.getHost()).build();
HttpUrl.Builder urlBuilder = request.url().newBuilder().scheme(host.getScheme()).host(host.getHost());
if (host.getPort() != -1) {
urlBuilder.port(host.getPort());
}
HttpUrl newUrl = urlBuilder.build();
Request newRequest = request.newBuilder().url(newUrl).build();
chain.withConnectTimeout(chain.connectTimeoutMillis() * (host.getRetryCount() + 1), TimeUnit.MILLISECONDS);
Response response = chain.proceed(newRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export function serializeUrl(
queryParameters: QueryParameters
): string {
const queryParametersAsString = serializeQueryParameters(queryParameters);
let url = `${host.protocol}://${host.url}/${
let url = `${host.protocol}://${host.url}${host.port ? `:${host.port}` : ''}/${
path.charAt(0) === '/' ? path.substr(1) : path
}`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ export type Host = {
* The protocol of the host URL.
*/
protocol: 'http' | 'https';

/**
* The port of the host URL.
*/
port?: number;
};

export type StatefulHost = Host & {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ package com.algolia.client.configuration
public data class Host(
public val url: String,
public val callType: CallType? = null,
public val protocol: String = "https",
public val port: Int? = null,
)
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ public class KtorRequester(
val requestBuilder = httpRequestBuilderOf(requestConfig, requestOptions)

for (host in hosts) {
requestBuilder.url.protocol = URLProtocol.createOrDefault(host.protocol)
requestBuilder.url.host = host.url
if (host.port != null) {
requestBuilder.url.port = host.port!!
}
requestBuilder.setTimeout(requestOptions, callType, host)
try {
val response = httpClient.request(requestBuilder)
Expand Down Expand Up @@ -132,8 +136,6 @@ public class KtorRequester(
): HttpRequestBuilder {
return HttpRequestBuilder().apply {
url {
protocol = URLProtocol.HTTPS
port = URLProtocol.HTTPS.defaultPort
pathSegments = requestConfig.path
}
method = requestConfig.method.ktorHttpMethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ import com.algolia.client.configuration.Host
internal data class RetryableHost(
private val host: Host,
) {

val url
get() = host.url
val protocol
get() = host.protocol
val port
get() = host.port
val callType
get() = host.callType

Expand Down
9 changes: 0 additions & 9 deletions clients/algoliasearch-client-php/lib/Http/Psr7/Uri.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,6 @@ class Uri implements UriInterface
private static $defaultPorts = [
'http' => 80,
'https' => 443,
'ftp' => 21,
'gopher' => 70,
'nntp' => 119,
'news' => 119,
'telnet' => 23,
'tn3270' => 23,
'imap' => 143,
'pop' => 110,
'ldap' => 389,
];

private static $charUnreserved = 'a-zA-Z0-9_\-\.~';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ final class ApiWrapper implements ApiWrapperInterface
*/
private $clusterHosts;

/**
* @var Configuration
*/
private $config;

/**
* @var RequestOptionsFactory
*/
Expand All @@ -57,6 +62,7 @@ public function __construct(
) {
$this->http = $http;
$this->clusterHosts = $clusterHosts;
$this->config = $config;
$this->requestOptionsFactory =
$RqstOptsFactory ?: new RequestOptionsFactory($config);
$this->logger = $logger ?: Algolia::getLogger();
Expand Down Expand Up @@ -148,7 +154,16 @@ private function request(

$retry = 1;
foreach ($hosts as $host) {
$uri = $uri->withHost($host);
if ($this->config->getHasFullHosts()) {
$host = explode(':', $host);
$uri = $uri->withHost(trim($host[1], '/'))
->withScheme($host[0])
->withPort($host[2])
;
} else {
$uri = $uri->withHost($host);
}

$request = null;
$logParams['retryNumber'] = $retry;
$logParams['host'] = (string) $uri;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public static function createFromAppId($applicationId)
$read[$applicationId.'-dsn.algolia.net'] = 10;
$write[$applicationId.'.algolia.net'] = 10;

return self::create($read, $write);
return self::create($read, $write)->shuffle();
}

public static function createFromCache($cacheKey)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ final class HostCollection
public function __construct(array $hosts)
{
$this->hosts = $hosts;

$this->shuffle();
}

public static function create(array $urlsWithPriority)
Expand Down
17 changes: 12 additions & 5 deletions clients/algoliasearch-client-python/algoliasearch/http/hosts.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@ class Host:
TTL = 300.0

def __init__(
self, url: str, priority: Optional[int] = 0, accept: Optional[int] = None
self,
url: str,
scheme: str = "https",
port: Optional[int] = None,
priority: Optional[int] = 0,
accept: Optional[int] = None,
) -> None:
self.url = url
self.scheme = scheme
self.port = port
self.priority = cast(int, priority)
self.accept = (CallType.WRITE | CallType.READ) if accept is None else accept

Expand All @@ -21,15 +28,15 @@ def reset(self) -> None:


class HostsCollection:
def __init__(self, hosts: List[Host]) -> None:
def __init__(self, hosts: List[Host], reorder_hosts=False) -> None:
self._hosts = hosts

for host in self._hosts:
host.reset()

shuffle(self._hosts)

self._hosts = sorted(self._hosts, key=lambda x: x.priority, reverse=True)
if reorder_hosts:
shuffle(self._hosts)
self._hosts = sorted(self._hosts, key=lambda x: x.priority, reverse=True)

def read(self) -> List[Host]:
return [host for host in self._hosts if host.accept & CallType.READ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ async def request(
)

for host in self._retry_strategy.valid_hosts(self._hosts):
url = "https://{}{}".format(host.url, path)
url = "{}://{}{}".format(host.scheme, host.url + (":{}".format(host.port) if host.port else ""), path)

proxy = None
if url.startswith("https"):
Expand All @@ -102,7 +102,7 @@ async def request(
proxy = self._config.proxies.get("http")

try:
async with timeout(self._timeout):
async with timeout(self._timeout / 1000):
resp = await self._session.request(
method=verb,
url=url,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def send_request(host, method, path, body, query_params, headers, timeout, conne
# @return [Faraday::Connection]
#
def connection(host)
@connections[host.url] ||= Faraday.new(build_url(host)) do |f|
@connections[build_url(host)] ||= Faraday.new(build_url(host)) do |f|
f.adapter @adapter.to_sym
end
end
Expand All @@ -68,7 +68,7 @@ def connection(host)
# @return [String]
#
def build_url(host)
host.protocol + host.url
host.protocol + host.url + (host.port.nil? ? '' : ":#{host.port}")
end

# Convert query_params to a full query string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module Transport
class StatefulHost
include CallType

attr_reader :url, :protocol, :accept
attr_reader :url, :protocol, :accept, :port
attr_accessor :last_use, :retry_count, :up

# @param url [String] host url
Expand All @@ -15,6 +15,7 @@ class StatefulHost
def initialize(url, opts = {})
@url = url
@protocol = opts[:protocol] || 'https://'
@port = opts[:port]
@accept = opts[:accept] || (READ | WRITE)
@last_use = opts[:last_use] || Time.now.utc
@retry_count = opts[:retry_count] || 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ def request(call_type, method, path, body, opts = {})
outcome = @retry_strategy.decide(host, http_response_code: response.status, is_timed_out: response.has_timed_out, network_failure: response.network_failure)
if outcome == FAILURE
decoded_error = JSON.parse(response.error, :symbolize_names => true)
raise AlgoliaHttpError.new(decoded_error[:status], decoded_error[:message])
raise Algolia::AlgoliaHttpError.new(decoded_error[:status], decoded_error[:message])
end
return response unless outcome == RETRY
end

raise Algolia::AlgoliaUnreachableHostError('Unreachable hosts')
raise Algolia::AlgoliaUnreachableHostError.new('Unreachable hosts')
end

private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ package algoliasearch.config
* Whether this host should be used for read and/or write requests.
* @param scheme
* Host protocol (i.e. `http`, `https`)
* @param port
* Host port
*/
case class Host(url: String, callTypes: Set[CallType], scheme: String = "https")
case class Host(url: String, callTypes: Set[CallType], scheme: String = "https", port: Option[Int] = None)
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ private[algoliasearch] class StatefulHost(private val host: Host) {

def getScheme: String = host.scheme

def getPort: Option[Int] = host.port

def isUp: Boolean = up

def getRetryCount: Int = retryCount
Expand Down
Loading

0 comments on commit 65bd154

Please sign in to comment.