Skip to content

allow inheriting from TimeSeries class #32

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

Closed
wants to merge 1 commit into from

Conversation

glaszig
Copy link

@glaszig glaszig commented Jun 23, 2023

i needed to subclass TimeSeries to add functionality but then was bitten by privateness of things as those properties are exclusive to the defining scope.

this opens up the class to make it easier to quickly modify behavior.

@palicao
Copy link
Owner

palicao commented Jun 27, 2023

Hi @glaszig, and thank you for your PRs! May I ask you which use case is solved by extending the class, which is not possible to solve using composition?
Whenever possible, we should prefer composition to inheritance!

@glaszig
Copy link
Author

glaszig commented Jun 28, 2023

to quickly implement the patch in #33. i ended up copying the entire class. but nevermind, i moved to use a delegator/decorator. implementation for reference:

<?php

namespace App;

use Redis;
use Palicao\PhpRedisTimeSeries\AggregationRule;
use Palicao\PhpRedisTimeSeries\Label;
use Palicao\PhpRedisTimeSeries\Metadata;
use Palicao\PhpRedisTimeSeries\Sample;
use Palicao\PhpRedisTimeSeries\TimeSeries;
use Palicao\PhpRedisTimeSeries\Client\RedisClient;
use Palicao\PhpRedisTimeSeries\Client\RedisConnectionParams;
use RedisException;

class RedisTimeSeries
{
    protected $redis, $ts;

    public function __construct()
    {
        $this->redis = new RedisClient(
            new Redis(),
            new RedisConnectionParams()
        );
        $this->ts = new TimeSeries($this->redis);
    }

    public function __call(string $name, array $args)
    {
        return call_user_func_array([$this->ts, $name], $args);
    }

    public function info(string $key): Metadata
    {
        $result = $this->redis->executeCommand(['TS.INFO', $key]);
        $result = $this->parseInfo($result);

        $labels = [];
        foreach ($result['labels'] as $label) {
            $labels[] = new Label($label[0], $label[1]);
        }

        $sourceKey = $result['sourceKey'] === false ? null : $result['sourceKey'];

        $rules = [];
        foreach ($result['rules'] as $rule) {
            $rules[$rule[0]] = new AggregationRule($rule[2], $rule[1]);
        }

        return Metadata::fromRedis(
            $result['lastTimestamp'],
            $result['retentionTime'],
            $result['chunkCount'],
            $result['chunkSize'],
            $labels,
            $sourceKey,
            $rules
        );
    }

    protected function parseInfo($info)
    {
        $chunks = array_chunk($info, 2);
        $props = [];
        foreach ($chunks as $chunk) {
            $props[$chunk[0]] = $chunk[1];
        }

        // maxSamplesPerChunk has been replaced with chunkSize in 1.4.4
        // https://github.com/RedisTimeSeries/RedisTimeSeries/commit/5896a509e5ec30929c04cd96a7f8b2c9e9651ed9
        $props['chunkSize'] = isset($props['chunkSize'])
            ? $props['chunkSize']
            : $props['maxSamplesPerChunk']
            ;

        return $props;
    }
}

@glaszig glaszig closed this Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants