kemal-session icon indicating copy to clipboard operation
kemal-session copied to clipboard

Simplify....

Open crisward opened this issue 7 years ago • 7 comments

After reading over the code, I can't help thing kemal-session could be significantly simplified by just saving a single object. That is to say, removing all the strings, int32, int64, floats, bools etc.

You could tell users they had to define a storable object. They could then define the structure of the data they want to save. I typically want to save a user, so as long as my user has a to_json method (which is probable if I run an endpoint on it) I can the just nest in as a sub object.

ie

class SessionSchema
    def initialize
    end

    JSON.mapping({
      user:   {type: User, nilable: true},
      basket: {type: Basket, nilable: true},
      flash:{type:String, nilable: true},
    })
    include Session::StorableObject
  end

This would remove many of the current macros and possibly make it easier for developers to add other store engines.

What do you think?

crisward avatar Feb 27 '17 13:02 crisward

Not sure about this. Let's ask @neovintage and @Thyra

sdogruyol avatar Feb 27 '17 13:02 sdogruyol

@neovintage flash plugin would need modifying a little. He'd need to ask the author to add a flash key to their session schema for his flash object. But that wouldn't be that bad.

crisward avatar Feb 27 '17 14:02 crisward

Phew, that's a difficult question. I don't think it would really make such a big difference for us: There would be less macros, but the methods for managing primitive types are still needed; we don't really drop anything, we just rearrange. For me it is rather a question of spirit: Right now everything is very flexible and loose, your proposal is more structured and serious: The user needs to plan his program beforehand (What variables do I need in my session?) and then stick to this structure. That is not necessarily bad, it also helps to avoid bugs and so on, but it feels not as playful anymore and I wouldn't want to make this project Java-flavoured. So I can't yet decide whether I like it or not, but this is a massive design choice and we should give it some thought first (and pay special attention to what is best for the user).

Thyra avatar Mar 03 '17 22:03 Thyra

Thanks for considering this. I know what you mean about the j-word thing... however the schema does give you one place to look when you want to see what is being stored. It makes it self documenting and enables you to have default values etc.

Good luck with the decision...

crisward avatar Mar 03 '17 23:03 crisward

Do you think there is a reasonable way to switch to this new structure while somewhat keeping backward compatibility? I'm thinking a default schema where primitive arrays are already defined (so that session.ints["foobar"] works out of the box) and the user can extend or replace the schema if he wants to use a more structured approach. That way we can provide all of the benefits of your idea but it is also possible to use kemal-session the traditional way without ever having to learn what a schema is.

Thyra avatar Mar 04 '17 08:03 Thyra

That makes perfect sense.

crisward avatar Mar 04 '17 09:03 crisward

my sessions table looks like this lol:

            session_id            |                                   data                                    |         updated_at         
----------------------------------+---------------------------------------------------------------------------+----------------------------
 e8909dff65e214892a1b5076bc55c56f | {"ints":{},"bigints":{},"strings":{},"floats":{},"bools":{},"objects":{}} | 2021-12-08 16:07:29.339638
 fe6137cdbf7fd3ad58195f47350e3f2c | {"ints":{},"bigints":{},"strings":{},"floats":{},"bools":{},"objects":{}} | 2021-12-09 15:41:22.388829
 610097aed2ad2ba00afcbfc392996312 | {"ints":{},"bigints":{},"strings":{},"floats":{},"bools":{},"objects":{}} | 2021-12-08 16:07:30.466987
 0619333b8f70702fa4bf7d53f343d18a | {"ints":{},"bigints":{},"strings":{},"floats":{},"bools":{},"objects":{}} | 2021-12-09 16:01:19.181023
 a09aca9cb4cf4cd2b989786c0f2a74c6 | {"ints":{},"bigints":{},"strings":{},"floats":{},"bools":{},"objects":{}} | 2021-12-08 16:07:31.886765

I would like a "define your own storage object" version. Maybe the {"ints":{},"bigints":{},"strings":{},"floats":{},"bools":{},"objects":{}} should be moved to an optional macro.

carcinocron avatar Dec 14 '21 17:12 carcinocron