Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 5 - Roadmap #7

Closed
odan opened this issue Jul 31, 2020 · 7 comments
Closed

Version 5 - Roadmap #7

odan opened this issue Jul 31, 2020 · 7 comments

Comments

@odan
Copy link
Owner

odan commented Jul 31, 2020

Here are some initial ideas:

Breaking Changes

  • All classes will be “final” by default. So extending will not be possible in the future. ✔️
  • Move all interface into its own namespace: Odan\Session\Interfaces
  • Remove SessionDoublePassMiddleware ✔️
  • Rename class PhpSession to Session
  • Remove MemorySession class
  • Move SessionMiddleware into its own namespace: Odan\Session\Middleware ✔️
  • Add a native PHP session cookie handler, e.g. Odan\Session\Cookie
    • Remove setCookieParams and getCookieParams
  • Move this library to selective/session

New Features

  • Add flash massages ✔️
  • Add support for dot notation using selective/array-reader
  • Throw SessionException instead of returning false for errors ✔️

Please add additional thoughts below.

odan added a commit that referenced this issue Jul 31, 2020
odan added a commit that referenced this issue Jul 31, 2020
odan added a commit that referenced this issue Jul 31, 2020
odan added a commit that referenced this issue Aug 1, 2020
odan added a commit that referenced this issue Aug 2, 2020
odan added a commit that referenced this issue Aug 2, 2020
odan added a commit that referenced this issue Aug 2, 2020
odan added a commit that referenced this issue Aug 2, 2020
odan added a commit that referenced this issue Aug 2, 2020
odan added a commit that referenced this issue Aug 2, 2020
@jnessier
Copy link

jnessier commented Aug 3, 2020

I learned a lot about separation of concerns in the last few days: https://java-design-patterns.com/principles/#separation-of-concerns

What you think about implementing Neoflow/FlashMessages or Slim/Flash, instead of creating a integrated flash messages support?

@odan
Copy link
Owner Author

odan commented Aug 3, 2020

I have already played with the idea to sepearte the Flash component but also wanted to "keep it simple" at the same time. Most session implementations come with their built-in Flash functionality (see Symfony, Aura, etc.). On the other hand you should be able to use your own Flash component in combination with this component (see Readme).

The new readme contains a section how to integrate the slim/flash component by sharing the same "storage" (ArrayObject). Slim/Flash also uses an internal ArrayAccess interface as internal storage. So both components can work together without the need for a running session.

https://github.com/odan/session#slim-flash-integration

The downside of this concept is, that they don't share the same FlashInterface. There is also a new FlashInterface which could be implemented. But it's also important that both components a sharing or abstracting the same storage interface to make it interobable. Of course we could provide a Adapter for it. Maybe I add a proof of concept.

Do you have a good idea / concept how you can make it very easy to integrate other Flash components like yours?

@jnessier
Copy link

jnessier commented Aug 3, 2020

I think a specific FlashInterface is the easiest way to go, by providing a constructor argument and/or a set method public function setFlash(FlashInterface $flash); in session class.

Example with set method:

SessionInterface::class => function (ContainerInterface $container) {
    $settings = $container->get('settings');
    $flash = $container->get(FlashInterface::class);
    $session = new PhpSession();
    $session->setOptions((array)$settings['session']);
    $session->setFlash($flash);
    
    return $session;
},

Example with constructor argument:

SessionInterface::class => function (ContainerInterface $container) {
    $settings = $container->get('settings');
    $flash = $container->get(FlashInterface::class);
    $session = new PhpSession($flash);
    $session->setOptions((array)$settings['session']);
    
    return $session;
},

I would prefer to provide both options. And if no custom flash got set, the session uses the default flash solution.

@jnessier
Copy link

jnessier commented Aug 3, 2020

But to be honest, I would skip the flash messages and keep focused on the session library itself.

@darkalchemy
Copy link

@odan While you are still building out these changes, could you add a couple of methods?
getFirst() and getLast()
These are easy enough to implement on my own, if you don't want to add them.

Thanks

@jnessier
Copy link

jnessier commented Aug 4, 2020

Congratulations! I really like the simpleness of the flash support.

odan added a commit that referenced this issue Aug 5, 2020
odan added a commit that referenced this issue Aug 6, 2020
@odan
Copy link
Owner Author

odan commented Aug 6, 2020

Thanks to all for the feedback.

@odan odan closed this as completed Aug 6, 2020
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

No branches or pull requests

3 participants