michael-e/storage

Symphony extension to manage session storage

storage's Issues

Differences

Just to note a thing, @michael-e and I discussed on the phone a while back.
When using set-count, the event displays the difference as follows:

<events>
    <storage-action type="set-count" result="success">
        <request-values>
            <group id="basket">
                <item id="article1" difference="+3" />
            </group>
        </request-values>
    </storage-action>
</events>

The question is: should positive differences include the +? Negative differences include the – of course. Is difference a good wording at all or should there be two different attribute depending on the algebraic sign: addition and subtraction?

Dropping an item that has no value doesn't drop it

If you pass an item without any value alongside with the drop action, the item will not be dropped from the storage.

Documentation update

The documentation in the readme and event state storage-action[update] but update is not an action.

Changing the actions

I'm at the final stages of a basket/checkout system, and one of the requirements is to be able to display the count, and edit this number to change the count.

Currently, as far as I can see, if I change the number and use set-count, it will just add to the existing number.

As I am storing more than just the count as a key, if I use set, it will replace the whole key with the new value, so a simple form (as I would expect) needs to contain all of the stored values again.

It makes sense to me therefore, to suggest a change in the actions naming to a more understandable convention, like such:

storage-action[set] (sets the whole item, replacing)
storage-action[set-count] (sets the whole count, replacing)
storage-action[increase-count] (increase the count key of the item)
storage-action[decrease-count] (decrease the count key of the item)
storage-action[drop] (drops the item)

Any thoughts on this?

For my current case, I will have to implement this, as there is currently no other way to simply change the number without a mathematical addition taking place.

Differing Size and Colour attributes issue

Not sure if features such as Size and Colour will work for storage as a cart when you have the same item with multiple choices of Colours and sizes... i.e

How do I identity what item has what attributes of colour and size when 2 of the same item gets added to the basket?

<basket>
<group id="basket">
  <item id="3">
    <item id="Nerk-Shirt!" count-positive="2">
      <item id="size">
        <item id="large" count="0" />
        <item id="medium" count="1" />
        <item id="small" count="1" />
        <item id="xlarge" count="0" />
        <item id="xxlarge" count="0" />
      </item>
      <item id="colour">
        <item id="black" count="1" />
        <item id="blue" count="1" />
      </item>
      <item id="price">18.00</item>
      <item id="shipping-uk">3.10</item>
      <item id="shipping-us">3.60</item>
      <item id="shipping-row">4.80</item>
    </item>
  </item>
</group>

I'm using the below as a method for adding the size and colour positive attributes to the basket...

But as you can see from above, There is no way of determining what colour is associated with which size in the cart.

What is the best approach here? or does Storage not cater for this scenario?

I may have approached the array structure wrong, but wanted to point out what may be an obstacle.. (in my little brain, anyways ) :)

Dropping count-positive doesn't work

Since we are "mapping" both count and count-positive keys to the same session array key (count), we currently have problems unsetting the count from the session array when only the count-positive key is found.

No big deal, maybe. But it should be done in a way that the preprocessing rule (mapping those values to s ingle one) doesn't occur twice. Hmmm, I will do my very best.

Release

As soon as #16 and #37 are solved, we are save to say that the current codebase is stable enough for a (private) release: the API is defined and works as expected (thanks @michael-e!).

I pushed the initial code as version 1.0 to Github, so the first stable release will be version 1.1.

Output parameters can not be used to filter other DSs

Due to the nature of output parameters, you can not use them to filter other datasources without hassle. The reason is: The dependency will point to a non-existing datasource, e.g. $ds-storage-cart in Symphony 2.2.5 resp. the real DS name in Symphony 2.3.x.

In order to use the params for filtering, you have to manually edit your second DS (and include the actual name of your Storage DS in the dependencies array).

I don't have any solution for that, however. Someone else?

count-positive/count convention only works for set-count action

This issue is related to the michael-e branch.

When using the set action, the count-positive/count convention (make count-positive have precedence) is not implemented. So you will get different (thus unexpected) results.

Event filter: Drop from storage

When building shopping carts, you might want to drop all cart items when an order has been saved successfully. Right now, this is only possible by redirecting to a "drop URL" (which uses an appropriate GET param).

It would be nice to have an event filter to do this.

I will push this addition soon.

DS Params

I have to say guys that I'm liking working with this extension. Hats off!

I think the only little niggle is the fact that the ds parameters are only one level deep. This is more likely a Symphony thing, but would it be possible to just add all nested items to the same level in the parameter output? Maybe have an option in the editor for this?

Evaluate branch michael-e

The current master branch is really broken, so please evaluate the michael-e branch.

Please, if you test this extension, use a tool like "Rested.app". Most of the bugs were so obvious using Rested, but you won't see them if you just pop the extension into your website (with just some special use cases). Instead you will probably search for issues in the wrong places.

For testing, I posted to a simple XML page which had the event and the datasource attached.

<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet version="1.0"
    xmlns:xsl="http://www.w3.org/1999/XSL/Transform">

<xsl:output method="xml"
    omit-xml-declaration="no"
    encoding="UTF-8"
    indent="yes" />

<xsl:template match="/">
    <data>
        <params>
            <xsl:copy-of select="data/params/*[starts-with(name(), 'ds-m')]"/>
        </params>
        <xsl:copy-of select="data/events"/>
        <xsl:copy-of select="data/m-storage"/>
    </data>
</xsl:template>

</xsl:stylesheet>

Fell free to break my solution. :-)

Action/item/session array handling is conceptually broken

This is the last remaining "big bug". I am posting it here to be able to reference it in my commit when I have fixed it. (I already know how to fix it.)

Storage is using two counting "actions" (triggered by keys), count and count-positive. At the moment these keys trigger special "item parsing", which means that the posted key/value pairs are modified (using values from the session array), then they are set in the session array (overwriting existing values).

The most obvious problem currently is: The approach results in both, count and count-positive key/value pairs in the session array. The extension attempts to "get this right" at the datasource level. Now imagine using several forms and switching between "count" and "count-positive". Guess what? It will never work as expected.

At the moment I am evaluating where to fix this conceptional flaw. One might work around it at the current "item parsing" stage, or one might find a more straightforward solution. (Please note: I assume that we still want to have a single "count" only, which also means a single "count" attribute in our XML. A fix for this issue would be much easier to use two different attributes, but after some thinking I consider this pure nonsense.)

In my original kick-off code I implemented a different approach:

• Don't try to modify items (keys/values) to map the counting logic
• Implement the counting logic at a later stage, i.e, when items are set in the session array. (Use a single "count" key in the session array, of course.) This requires recursive array manipulation.
• The datasource can be straightforward and output the "count" as attribute.

Due to some improvements we (sepecially Nils) made during development, implementing the above wouldn't just be a matter of copy and paste. I would do this, however, if I find that this is the best solution.

As I said, I am still evaluating solutions...

Restriction to whole numbers

With all the silly little issues I've raised recently, I think that the only genuine one I have is that the limitation of only allowing whole numbers is a wrong decision from a flexibility perspective.

I would like to be able to calculate prices too, not just quantities.

Using undefined action results in "success" event response

The title says it all.

Consider this fixed.

Negative value may be set using "count-positive"

Using a count-positive param and the set-count action, you can set a negative value. While the event's result will be error, the value will still be set.

Set-count is broken

Here is some example XML I get using the set-count action:

<events>
    <storage-action type="set-count" result="success">
        <request-values>
            <group id="basket">
                <item id="article1" difference="+3"/>
            </group>
        </request-values>
    </storage-action>
</events>
<m-storage>
    <group id="basket">
        <item id="article1" count="3"/>
    </group>
</m-storage>

Looks good for the first submit. :-)

The problem is: It's exactly the same for subsequent submits. It doesn't increase the count at all.

Count-positive not working as expected

Imagine an item count of -3. Now you use a [count-positive] key on this item with a value of 4. Currently the result is 0, but it should be 4.

Consider this fixed, I just post it here for reference.

Incorrect behaviour if request contains multiple storage actions

If you send multiple storage actions, the current code uses the one which occurs first in the request. In my eyes this is behaviour is incorrect. I suggest to use the last ocurrence of a storage action instead.

The fix is pretty simple, and I can send a pull request or directly push it if you like.

Symphony 2.2

That's a question for @michael-e:
Would this extension work (with a few missing features) on Symphony 2.2.5, if we added an about array to the driver?

Deleting items doesn't work a s expected

This is a minor bug with the current michael-e branch. You probably won't notice it in practice, but I will fix it nevertheless.

Currently, if you pass items with different "array depth", only the "deepest" items will be dropped. In addition to that, "higher" items will be set by the array iteration — even they haven't been there before.

Precendence rule needed if both count-type keys are used

If the request contains both a "count" and a "count-positive" key for the same item, we shouldn't use both. The reason is: The result of the calculation would depend on the order of these keys (i.e. which one is coming first).

I suggest to implement a precedence rule: If both keys are used in the same item, count-positive will have precedence (due to its more "special" nature).

Event Filter: Add to storage

How awesome would that be?

Event: Drop Storage

I just finished updating an old custom shopping cart to Storage: after checkout, the shopping cart has to be emptied. Of course I can just modify my event to drop the storage on success but shouldn't we provide a default option to handle this case?

• A second event that does nothing but dropping the storage that can be attached to a success page (event redirect).
• An event filter that drops the storage on success.

I have no idea how to handle namespaces though. But a generic drop all might be sufficient for most pages and would be better than nothing.

Thoughts?

Session Lifetime

I may be completely wrong here, but shouldn't as session only last until the browser is closed? Our storage sessions last longer than this (14 days as it seems) – I wonder if this has to do with Symphony?

count-positive

Does count-positive mean to increase the value of the item count? If so, would increase not be better?

Also, should there not be a decrease also?

Setting items is kind of "unpredictable"

I broke it again. :-)

I created this:

<group id="basket">
    <item id="article1" count="33">
        <item id="count-positive">
            <item id="lala">3</item>
        </item>
    </item>
</group>

You say that it is nonsense? OK, but I managed to do it!

Then I tried to overwrite the count-positivein my next request:

<events>
    <storage-action type="set-count" result="success">
        <request-values>
            <group id="basket">
                <item id="article1">
                    <item id="count-positive">3</item>
                    <item id="count">-2</item>
                </item>
            </group>
            <group id="quiz">
                <item id="q1">4</item>
            </group>
        </request-values>
    </storage-action>
</events>
<m-storage>
    <group id="basket">
        <item id="article1" count="36">
            <item id="count-positive">
                <item id="lala">3</item>
            </item>
        </item>
    </group>
    <group id="quiz">
        <item id="q1">4</item>
    </group>
</m-storage>

You see what I mean? The countis recalculated, but the count-positive item still lives there.

Issues like this may partly be related to our concept regarding counts, which is too complicated. Mapping two different item keys to a single session array key and a single XML attribute.

I will spend some hours on the German autobahn today — time to think. I may suggest (and code) a simpler concept regarding the count keys. (I don't know if this would fix the above whcih might also be realted to the array_replace_recursive() function, but anyway I think it is time to question the count concept. I have the feeling that I could find many more bugs if I only try... So maybe we have made this too complicated.)

Do we really need to map several "special keys" to a single XML attribute in the end? Couldn't we simplify this and say that every "special key" has a "special XML attribute"?

Do we really need to have these counts as attributes in our XML? (Technically, this is just an output rule, so it wouldn't help much to change this. Anyway.)

Error on dropping anything using GET method

array_keys() expects parameter 1 to be array, string given

Using a query string such as /?storage-action=drop-all

The above throws an error indicating that there is an issue on this line: 34 $action_keys = array_keys($_REQUEST['storage-action']);

Drop is broken

When I try to drop

storage[basket][{@id}]

to drop only the item under basket that matches the passed id, the drop action drops

storage[basket]

and the whole basket is gone :(

Testing for integer values doesn't work reliably

In the michael-e branch, testing is for integer values is done like that:

$is_int = ctype_digit((string)abs($value));

However, this also returns true for strings (e.g. 'hello test').

It is hard to understand why PHP makes it so difficult to perform such simple task... Nevertheless I will try again.

Renaming the event function has broken 2.2.x compatibility

In 2.2.x., the event must implement the __trigger() function, otherwise is breaks:

<br />
<b>Fatal error</b>: Class eventstorage_action contains 1 abstract method
and must therefore be declared abstract or implement the remaining methods
(Event::__trigger) in
<b>/Users/me/Sites/dev/me/sym22/extensions/storage/events/event.storage_action.php</b>on
line
<b>120</b>
<br />

It would be the simplest solution to revert to __trigger(). Otherwise we would have an additional commit for the 2.2.x branch.

What do you think?

Add 1.3 and 1.4 features to the README

There are some features which should be explained in the README.

• Honor redirect param if the event is successful
• Enable passing groups or items with the drop action

Example code would be cool.

Storing nonsense is possible

It is possible to store the following:

<item id="count-positive">
    <item id="lala">3</item>
</item>

This should not be possible. All current count type item keys should be dropped if they don't contain an integer.

README: examples for custom use case

Add two examples to the readme how to handle the case mentioned here using either JavaScript or a custom event.

Versions

The extension's meta file is missing information about the latest version updates.

Sanitizing seems broken

In the event, we attempt to sanitize the user input:

array_walk_recursive($items, 'General::sanitize');

Actually, according to the PHP docs, $items should probably be passed by reference:

array_walk_recursive(&$items, 'General::sanitize');

Still, it seems to have no effect at all. Attempting to store a value of blue> will save blue> to the session array. Attempting to save <blue> will break XSLT transformation.

set vs set-count

I don't understand how to use set vs set-count.

Wouldn't I always need to use set-count as I would need to pass a value?

Can you show me how you are using set vs set-count as, as far as I can see, set would not require a value to be passed, but therefore be an empty array key.

I'm confused.

I think I got used to the older shopping cart extension where you needed two fields, one with the item, and one with the count. I do feel that this would be more appropriate somehow. That way there would be an array of items, and an array of counts, keyed by item.

Suggestion: Rename it

I suggest to rename this extension to "Session Storage". This describes much better what it does (compared to using "local storage", for example).

Counting products: same kind, different qualities

Things are getting tricky. I'm trying to upgrade an old shop which uses the following interface:

The article id (= colour id) and the amount are stored in different fields – the first in a select box, the second in an input. What I need in the end is storage[basket][product-id][count] = 1 which is not possible with this interface: I can only create two separate values, storage[basket][product] = colour and storage[basket][product][count] = 1 – which doesn't make sense because I loose the connection between my selected colour and the count as soon as I add different colours of the same product to the basket.

The interface shouldn't change, so I see only two options:

1. Create separate forms per product colour in the HTML, combining these to a single interface using JavaScript to mimic the current interface.
2. Create a custom event that handles this setup.

The first approach feels bloated and the second one isn't that elegant either now that we created a flexible default event to prevent custom coding. I just shouldn't have to create a custom event.

Damn.

Drop the complete storage?

It might be nice to have a drop-all action which drops the entire storage from the session.

Error handling is broken again

When I added the "invalid action" error, I broke error handling. Sorry for that. Will fix.

Storage is hardly ever dropped completely

With the latest improvements to Symphony's cookie/session handling, Symphony will drop a user's cookie as soon as the corresponding session is empty. With the Storage extension, however, there will always remain an empty "storage" array in the user's session (unless you use the drop-all action, which is dangerous because you probably won't know if multiple storage groups a.k.a. namespaces are present in the user's session).

This means that if a user has used "Storage" (e.g. for a shopping cart), he will never again get rid if his cookie unless it expires.

The solution is: Drop the storage array once it gets empty. This should be an easy fix, and I don't foresee any negative side effects.

I will do it.

Setting counts breaks if there is already a string value

Set storage[basket][item1] to test.

Then try and set storage[basket][item1][count-positive] to 3.

Resulting value: 3est. Submit again, result: 6est.

Funny, isn't it? I have no idea why this happens, but I start researching now.

Drop action always drops the complete group

As the title says, the drop action seems to drop complete groups instead of dedicated items.

Symphony Cookie class

Any reason why this doesn't use the Symphony Cookie class? I know there are a couple of issues with it, but it should be sufficient for this kind of implementation.

Completely unsetting storage doesn't work if it is passed by reference

This is just a reminder, I am working on this.

You can not unset an array which has been passed by reference.

Add GET possibilities (esp. for dropping) to the README

See #43 – und gute Besserung, Michael.

Shouldn't count-positive drop items with resulting "zero" counts?

While implementing this extension (which works really nice, btw) I noticed that when using count-positive, you may end up having items with count="0" in your storage. This is not very logical, is it? Apart from that, it requires additional XSLT code to remove those items from the basket when displaying it. Shouldn't items with a resulting count of 0 be removed from the storage in the count-positive case?

Unfortunately I have no idea how to solve it, because the current item preprocessing is "unaware" of the session array. Still I would like to discuss this.

In my eyes it's a bug. What do you think?

Dropping count-positive values doesn't work as expected

We already invented a mechanism to drop items if a count-positive value does not result in a positive storage value.

A similar problem occurs when you drop a count-positive value. You have two options:

1. Just drop the count and leave the item intact. (This is how it currently works.)
2. Drop the item as well, because by dropping the count-positive value you probably intend to remove the item from the storage. (In other words: Dropping count-positive should have the same result as setting it to 0.)

The second solution is an assumption, of course. But think of a shopping cart (the main use case of count-positive values. You would probably want to add a button to clear the complete cart to your form, wouldn't you? Well, at the moment this is not possible. You can either drop the counts and leave the items in the cart, or you can drop the complete storage using the drop-all action (which is really bad, because you don't know what is stored in addition to your cart data).

So for a shopping cart (with a form sending count-positive values), the second possibility would be super-cool. You could use the [drop] action to simply remove all the items from the cart.

So I suggest to implement this "assumption logic".

Any obligations?

README: explain count/count-positive keys and related XML attributes