Skip to content

Add support for prefixed cache#51

Merged
Nyholm merged 2 commits intophp-cache:masterfrom
iainmckay:master
Mar 28, 2017
Merged

Add support for prefixed cache#51
Nyholm merged 2 commits intophp-cache:masterfrom
iainmckay:master

Conversation

@iainmckay
Copy link
Copy Markdown
Contributor

This adds support for the prefixed cache.

Example:

cache_adapter:
  providers:
    my_apc:
      factory: 'cache.factory.apc'
    my_prefixed:
      factory: 'cache.factory.prefixed'
      options:
        service: '@cache.provider.my_apc'
        prefix: 'some_prefix'

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 24, 2017

Codecov Report

Merging #51 into master will decrease coverage by -0.38%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
- Coverage    21.9%   21.52%   -0.38%     
==========================================
  Files          35       36       +1     
  Lines         630      641      +11     
==========================================
  Hits          138      138              
- Misses        492      503      +11
Impacted Files Coverage Δ
src/Factory/PrefixedFactory.php 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3d344c...f62733c. Read the comment docs.

@jewome62
Copy link
Copy Markdown

jewome62 commented Mar 3, 2017

Hi,

You code don't work for me :

[Cache\AdapterBundle\Exception\ConfigurationException]                                                                                                                                      
  Error while configure adapter apcu_prefixed. Verify your configuration at "cache_adapter.providers.apcu_prefixed.options". The option "service" with value "@cache.provider.apcu" is expect  
  ed to be of type "object", but is of type "string".                                                                                                                                                                                                                                                                                                                                     
                                                                                                                      
  [Symfony\Component\OptionsResolver\Exception\InvalidOptionsException]                                               
  The option "service" with value "cache.provider.apcu" is expected to be of type "object", but is of type "string". 
cache_adapter:
    providers:
        apcu:
            factory: 'cache.factory.apcu'
            aliases: ['cache.apcu']
        apcu_prefixed:
            factory: 'cache.factory.prefixed'
            options:
              service: "@cache.provider.apcu"
              prefix: 'qanda'
```

If y delete the constraint, into PrefixedFactory, that work.

@iainmckay
Copy link
Copy Markdown
Contributor Author

@jewome62 Try now, during my tests I was bootstrapping the system without using YAML so my references were all objects already and the options resolver was explicitly requiring objects.

I've removed that constraint and now the bundle is responsible for turning your service in to a reference.

@prisis
Copy link
Copy Markdown
Contributor

prisis commented Mar 3, 2017

@iainmckay can you right tests for this?

@prisis prisis requested review from Nyholm, cryptiklemur and prisis March 3, 2017 11:48
@iainmckay
Copy link
Copy Markdown
Contributor Author

@prisis I did intend to do this but couldn't locate your test suite to add my tests. Can you point me to where it resides and I'll add my tests to it?

@prisis
Copy link
Copy Markdown
Contributor

prisis commented Mar 3, 2017

@iainmckay
Copy link
Copy Markdown
Contributor Author

@prisis Correct me if I'm wrong but none of the other factories are tested here?

It would be helpful to see existing tests for something like the ChainFactory which this PR is based on so I can fit tests in with your testing methodology.

@prisis
Copy link
Copy Markdown
Contributor

prisis commented Mar 3, 2017

@Nyholm correct me if i'm wrong, but we have only a bundle test?

@iainmckay we dont have test for the other components in this adapter ... we need wait for @Nyholm or @aequasi we i can merge this without tests

@iainmckay
Copy link
Copy Markdown
Contributor Author

@prisis Is there a way to get this reviewed? We're maintaining a fork which is less than ideal.

@Nyholm
Copy link
Copy Markdown
Member

Nyholm commented Mar 28, 2017

I will have a look at this today. Sorry for the delay.

Copy link
Copy Markdown
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@Nyholm Nyholm merged commit bdaae57 into php-cache:master Mar 28, 2017
@prisis prisis removed their request for review March 28, 2017 08:15
@iainmckay
Copy link
Copy Markdown
Contributor Author

Great, thanks :)

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.

5 participants