Avoid to retain SessionIdGenerator in session object

[quaff]

session id generation shouldn't be the responsibility of session object, since SessionIdGenerator held by session object is only used by changeSessionId(), we could introduce overload method passing it as parameter to avoid that.

[quaff]

Could you review this PR? @rwinch @marcusdacoregio @vpavic

[quaff]

Rebased and split to multiple commits for better code review.

[marcusdacoregio]

Hi, @quaff. I am not so sure about the pros and cons here. Imagine a scenario where I am migrating users from a legacy db to a new db and both have different strategies to generate session ids.

The problem would arise when a session need to changeSessionId(). Currently, the session implementation will use the strategy provided by the repository, in the scenario above, the legacy or new strategy. If we move the responsibility to the filter, how do we know which strategy to use when changing the session id? The interface contract does not provide any context about the session.

[quaff]

Hi, @quaff. I am not so sure about the pros and cons here. Imagine a scenario where I am migrating users from a legacy db to a new db and both have different strategies to generate session ids.

The problem would arise when a session need to changeSessionId(). Currently, the session implementation will use the strategy provided by the repository, in the scenario above, the legacy or new strategy. If we move the responsibility to the filter, how do we know which strategy to use when changing the session id? The interface contract does not provide any context about the session.

We could introduce a default method for SessionIdGenerator

public interface SessionIdGenerator {

	@NonNull
	String generate();

	@NonNull
	default String regenerate(Session session) {
		return generate();
	}

}

	@Override
	public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
		String newSessionId = sessionIdGenerator.regenerate(this);
		setId(newSessionId);
		return newSessionId;
	}

Now we can choose strategy base on original session id.

EDIT: I've pushed a new commit to accomplish this.

[quaff]

I really think it's not a good choice that domain object holds non-domain object reference, any thoughts?

[marcusdacoregio]

Hi, @quaff. This is not something that we want to do now. I'd prefer to receive more feedback from the community and see where the SessionIdGenerator support has its flaws and then change accordingly.

It is also strange that a low-level SessionRepositoryFilter depends on SessionIdGenerator. That would make it impossible to have a strategy per session repository.

With that said, I'll close this issue and wait for community feedback to proceed with further changes related to it.