Skip to content

Add test code for system/Cache/Handlers/RedisHandler.php#585

Merged
lonnieezell merged 3 commits intocodeigniter4:developfrom
KazuakiM:patch-8
Jul 17, 2017
Merged

Add test code for system/Cache/Handlers/RedisHandler.php#585
lonnieezell merged 3 commits intocodeigniter4:developfrom
KazuakiM:patch-8

Conversation

@KazuakiM
Copy link
Copy Markdown
Contributor

@KazuakiM KazuakiM commented Jul 4, 2017

  • system/Cache/Handlers/RedisHandler.php
    I changed to fix to consider multiple Redis servers.
    And we set the response format of getMetaData same as MemcachedHandler.php.

  • tests/system/Cache/Handlers/RedisHandlerTest.php (new)
    I added a test code, but it is not very good.
    I think that memory capacity and access speed are not good because all are stored in Hash structure.

Comment thread system/Cache/Handlers/RedisHandler.php Outdated
$this->prefix = $config['prefix'] ?? '';

if (isset($config->redis))
if (!empty($config))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's nit-picky, but according to our style guide, there should be a space after the exclamation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I update coding style and previous RP code too.

$this->assertFalse($this->redisHandler->delete(self::$dummy));
}

//FIXME: I don't like all Hash logic very much. It's wasting memory.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which part are you concerned about here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redis strcture is following now. So 'data' field is not exists.

'key' : [
    '__ci_type' : xxxxx,
    '__ci_value': yyyyy
]

And I think Redis have many functions. I want to use set, get and etc too.
set is very simple and a small amount of memory.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like you know more than I do here. I'm actually not very familiar with the intricacies of Redis, and have only used it a little.

@jim-parry
Copy link
Copy Markdown
Contributor

not to be too picky, but we expect commits to be GPG-signed :)

@KazuakiM
Copy link
Copy Markdown
Contributor Author

Hi @jim-parry , @lonnieezell
I thought that GPG was signed.

@jim-parry
Copy link
Copy Markdown
Contributor

@KazuakiM Your last commit is indeed GPG-signed, thank you :) (the earlier two weren't)

@lonnieezell
Copy link
Copy Markdown
Member

@KazuakiM Are you done with this or still working on it?

@KazuakiM
Copy link
Copy Markdown
Contributor Author

@lloricode
This PR has been completed.

@lonnieezell
Copy link
Copy Markdown
Member

Thanks. Merging in.

@lonnieezell lonnieezell merged commit 755de6b into codeigniter4:develop Jul 17, 2017
@KazuakiM KazuakiM deleted the patch-8 branch July 17, 2017 21:43
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.

3 participants