Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

API Cleanup #49

Merged
merged 19 commits into from

3 participants

@tedivm
Owner

The focus of this PR is to clean up some outstanding API inconsistencies and legacy points. This includes a combination of nomenclature and function changes- basically, lots of class name changes and small changes geared around making the library easier to use.

It includes a number of changes-

  • Renames "Handlers" to "Drivers".
  • Changes the name of Stash\Cache to Stash\Item.
  • Renames Handlers\MultiHandler to Handlers\Composite (and finally Driver\Composite).
  • Adds a constructor to the Pool class that removes the need to call setHandler/setDriver.
  • Removes the Box and Manager classes.
  • Removes "purge" function from Item class, leaving the Pool version.
  • Revamped the Pool class's error handling.
  • Updated various comments.
tedivm added some commits
@tedivm tedivm Simplified Pool creation by adding constructor
It's now possible to pass the driver/handlers in as part of the
constructor, simplifying the creation of the Pool object a bit.
2012c30
@tedivm tedivm Changed name of Stash\Cache to Stash\Item
The "Cache" item was originally created before the separation of the
Pool and Item was made, and had it's old name mostly as a result of
that. This change should help distinguish the roles between the Pool
and Item a bit better.
75f04de
@tedivm tedivm Added composer "vendors" folder from composer. b3949a2
@tedivm tedivm Removed Box and Manager classes
The Box and Manager classes were remnents from before the Pool class
existed, and are no longer needed.
8c35500
@tedivm tedivm Changed the name of the MultiHandler to Composite. 61574d3
@tedivm tedivm Cleaned up some Test class names. 11c178a
@tedivm tedivm Changed "Handlers" into "Drivers"
The term "handler" has been used to describe the code that provides low
level least common denominator access to the various storage engines
used for caching. Changing them to "drivers" makes it more immediately
obvious what their function is, while also being more consistant with
similar systems.
d54aa28
@tedivm tedivm Finishing last commit
I'm not sure why these files weren't removed with the last commit, but
this was part of that previous commit related to changing "handlers"
into "drivers".
52211b7
@tedivm tedivm Adding the "report" directory into the git ignore list
The code coverage reports get generated into that folder, and don't
need to be shipped along with this library.
19a8900
@tedivm tedivm Removed purge function from Item
The purge function was just calling through to it's new home in the
pool class, and removing it makes things a bit cleaner.
19ec674
@tedivm tedivm Updated docblock aa62007
@tedivm tedivm Removed "purge" from Item Test, since the function was removed 10368a1
@tedivm tedivm Updated Pool class to disable itself when certain errors occur. 4d3bfd1
@tedivm tedivm Docblock updates. 3b00484
@tedivm tedivm Updated Readme with link to development documentation. 03f5682
@tedivm tedivm Added phpstorm files to gitignore. 3057572
@tedivm tedivm Made the "setupKey" function protected, added key to constructor
Removed the setupdKey from the "public" API for the Item class. It was
never meant to be called publically, just by the Pool object, but
moving the key to the constructor simplifies that quite a bit.
f5e168f
@tedivm tedivm Corrected issue in disabled stash test.
One of the tests checking whether stash was properly being disabled was
not actually having it's assertions tested. This wasn't critical due to
a more comprehensive test that is run, but should be corrected.
a1bf775
@tedivm tedivm Removed ability to create Item without a key.
Creating an Item without a key was an artifact from the pre-Pool days,
where certain actions (such as clearing) needed to be done rather than
actions on single items. Now that the pool exists to handle these it
makes little sense for the Item to be able to exist without a key.
703eefc
@schmittjoh

You might want to consider taking a look at:
https://github.com/schmittjoh/php-option

This should allow you to get rid of some of the code that is currently handling cache misses/hits and provides a powerful API to handle those elegantly in userland.

@tedivm
Owner

Alright, I'm happy with these changes. I'm going to merge these in and release the 0.10.1 over the next couple of days. However, I am going to continue refactoring in subsequent PR's- I just want people to be able to start playing with this as it is (particularly for the Stash Bundle folks doing the Symfony integration).

@tedivm tedivm merged commit 5c4ef1d into master
@gggeek

potential for collisions: if key is array( '', '/' ), it will save path in same mem slot as key array( '/', '' )

@gggeek

The "cas" function could probably be renamed, as it works differently from the memcached native "cas" function - or at least properly documented

@gggeek

If this call fails, we should probably not return $value.
Otoh according to the php manual, $token is only filled in when get() succeeds, so here we could/should use add() instead

@gggeek

What if another php script has cleared the cache for this path while this one is executing and has already the pathKey in memory? This one will still read the old data

@gggeek

maybe add a comment about the algorithm used for cache clearing?

Side note: it seems there is a potential for reading stale data here: if memcache is under pressure, it might drop from memory the value which we use to keep track of the "last valid version for items A/B/*".
When this happens, all values still present in memcache for version 0 of items in the subtree will suddenly become valid again

@gggeek

what if the inc() call fails with a transient error? should we return false?

@gggeek

the call to cas() is probably useless: the memcached API specifies an initial value for increment() if the key is not set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 10, 2012
  1. @tedivm

    Simplified Pool creation by adding constructor

    tedivm authored
    It's now possible to pass the driver/handlers in as part of the
    constructor, simplifying the creation of the Pool object a bit.
  2. @tedivm

    Changed name of Stash\Cache to Stash\Item

    tedivm authored
    The "Cache" item was originally created before the separation of the
    Pool and Item was made, and had it's old name mostly as a result of
    that. This change should help distinguish the roles between the Pool
    and Item a bit better.
  3. @tedivm
  4. @tedivm

    Removed Box and Manager classes

    tedivm authored
    The Box and Manager classes were remnents from before the Pool class
    existed, and are no longer needed.
  5. @tedivm
  6. @tedivm
  7. @tedivm

    Changed "Handlers" into "Drivers"

    tedivm authored
    The term "handler" has been used to describe the code that provides low
    level least common denominator access to the various storage engines
    used for caching. Changing them to "drivers" makes it more immediately
    obvious what their function is, while also being more consistant with
    similar systems.
  8. @tedivm

    Finishing last commit

    tedivm authored
    I'm not sure why these files weren't removed with the last commit, but
    this was part of that previous commit related to changing "handlers"
    into "drivers".
  9. @tedivm

    Adding the "report" directory into the git ignore list

    tedivm authored
    The code coverage reports get generated into that folder, and don't
    need to be shipped along with this library.
Commits on Nov 12, 2012
  1. @tedivm

    Removed purge function from Item

    tedivm authored
    The purge function was just calling through to it's new home in the
    pool class, and removing it makes things a bit cleaner.
  2. @tedivm

    Updated docblock

    tedivm authored
  3. @tedivm
  4. @tedivm
  5. @tedivm

    Docblock updates.

    tedivm authored
  6. @tedivm
Commits on Nov 14, 2012
  1. @tedivm
  2. @tedivm

    Made the "setupKey" function protected, added key to constructor

    tedivm authored
    Removed the setupdKey from the "public" API for the Item class. It was
    never meant to be called publically, just by the Pool object, but
    moving the key to the constructor simplifies that quite a bit.
  3. @tedivm

    Corrected issue in disabled stash test.

    tedivm authored
    One of the tests checking whether stash was properly being disabled was
    not actually having it's assertions tested. This wasn't critical due to
    a more comprehensive test that is run, but should be corrected.
  4. @tedivm

    Removed ability to create Item without a key.

    tedivm authored
    Creating an Item without a key was an artifact from the pre-Pool days,
    where certain actions (such as clearing) needed to be done rather than
    actions on single items. Now that the pool exists to handle these it
    makes little sense for the Item to be able to exist without a key.
Something went wrong with that request. Please try again.