Skip to content
This repository has been archived by the owner on Feb 5, 2024. It is now read-only.

Shared key flattening #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EmanueleMinotto
Copy link
Member

Closes #63

I'm not fully sure about the method output because it isn't based on a RFC nor a standard, but this could be done in the v2.

@billschaller
Copy link
Member

👍

@billschaller
Copy link
Member

Looking at this a bit more, it might be better to roll find, insert, update, delete up into AbstractStorage as well. Once the key is flattened, you can have a simpler internal storage API that simply accepts a string key. For storages that implement composite keys without flattening, they can simply skip inheriting AbstractStorage and directly implement Storage.

What do you think?

class AbstractStorage implements Storage
{
...
    /**
     * {@inheritDoc}
     */
    public function insert($storageName, $key, array $data)
    {
        $key = $this->flattenKey($storageName, $key);
        $this->doInsert($storageName, $key, $data);
    }

    /**
     * @param string $storageName
     * @param string $key
     * @param array $data
     */
    abstract protected function doInsert($storageName, $key, $data);
...
}
@EmanueleMinotto
Copy link
Member Author

That's interesting! So basically do you suggest to follow internally the same doctrine/cache implementation?

But imo the number of abstractions should be as low as possible until a complete overview of the features/requirements is not done, for example:

  • the flattenKey method is implemented by 2/11 of the currently available storages
  • the RangeQueryStorage interface is implemented by 3/11 of the currently available storages

Honestly I don't know which are the implications (in a long term) of that approach in a system that could become more complex than the doctrine/cache.

So imo that approach should be considered only after the v1.
/cc @beberlei @Ocramius

@billschaller
Copy link
Member

@EmanueleMinotto

Yeah, upon further consideration this morning maybe that's not the best route.

Currently, SingleIdHandler is taking array keys and returning only the member matching the first identifier metadata. I think if you flatten keys at all, they should be flattened in the IdHandler instead of at the Storage level.

Looking at the CouchDB Storage, it seems as though this code in flattenKey never even runs, because the SingleIdHandler flattens the key already:

    private function flattenKey($storageName, $key)
        ...
        if ( ! is_array($key)) {
            throw new \InvalidArgumentException('The key should be a string or a flat array.');
        }

        foreach ($key as $property => $value) {
            $finalKey .= sprintf('%s:%s-', $property, $value);
        }

        return $finalKey;
    }
@EmanueleMinotto EmanueleMinotto modified the milestones: 1.0.0, 2.0.0 Sep 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
2 participants