exo-core

Use strong contract in ConversationRegistry

Details

  • Type: Task Task
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: cor-2.1, cor-2.1.1
  • Fix Version/s: cor-2.2
  • Component/s: component.security
  • Labels:
  • Similar issues:
  • Description:
    Hide

    The ConversationRegistry use java.lang.Object as the key to register states.

    We currently use it to store String http session ids.
    But previous (and deprecated) behaviour was to store user ids i the registry.

    The contract is weakly type and may be source of errors. To avoid this I suggest the following design :

    In ConversationRegistry deprecate
    public void register(Object key, ConversationState state)
    And add instead
    public void register(StateKey key, ConversationState state)

    public interface StateKey...

    public final class HttpSessionStateKey {
    public SessionStateKey(HttpSession session) {}
    }

    Show
    The ConversationRegistry use java.lang.Object as the key to register states. We currently use it to store String http session ids. But previous (and deprecated) behaviour was to store user ids i the registry. The contract is weakly type and may be source of errors. To avoid this I suggest the following design : In ConversationRegistry deprecate public void register(Object key, ConversationState state) And add instead public void register(StateKey key, ConversationState state) public interface StateKey... public final class HttpSessionStateKey { public SessionStateKey(HttpSession session) {} }

Activity

Hide
Patrice Lamarque (old, use patrice_lamarque) added a comment - 15/Sep/08 4:19 PM

This design will allow to catch some errors at compile time.

For instance, currently, AuthenticationServiceImpl.broadcast() (deprecated) makes bad use of registry by passing a user id.

It would no longer be allowed to be written.

Show
Patrice Lamarque (old, use patrice_lamarque) added a comment - 15/Sep/08 4:19 PM This design will allow to catch some errors at compile time. For instance, currently, AuthenticationServiceImpl.broadcast() (deprecated) makes bad use of registry by passing a user id. It would no longer be allowed to be written.
Hide
Patrice Lamarque (old, use patrice_lamarque) added a comment - 10/Jan/09 9:20 AM

In order to avoid breaking compatibility, shouldn't we schedule this API change for a minor version (ex: 2.2) instead of a maintenance version (2.1.4) ?

Show
Patrice Lamarque (old, use patrice_lamarque) added a comment - 10/Jan/09 9:20 AM In order to avoid breaking compatibility, shouldn't we schedule this API change for a minor version (ex: 2.2) instead of a maintenance version (2.1.4) ?

People

Dates

  • Created:
    15/Sep/08 4:17 PM
    Updated:
    06/Feb/09 1:45 PM
    Resolved:
    04/Feb/09 1:53 PM
    Date of First Response:
    15/Sep/08 7:12 PM