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

Why to copy ConnectionFactory on each create #73

Open
igreenfield opened this issue Feb 13, 2017 · 7 comments
Open

Why to copy ConnectionFactory on each create #73

igreenfield opened this issue Feb 13, 2017 · 7 comments

Comments

@igreenfield
Copy link
Contributor

I see in class Connections
lines 108 - 118:

public static ConfigurableConnection create(ConnectionOptions options, Config config, ClassLoader classLoader)
      throws IOException, TimeoutException {
    Assert.notNull(options, "options");
    Assert.notNull(config, "config");
    Assert.notNull(classLoader, CLASS_LOADER_PARAMETER_NAME);
    ConnectionHandler handler = new ConnectionHandler(options.copy(), new Config(config), classLoader);
    ConfigurableConnection proxy = (ConfigurableConnection) Proxy.newProxyInstance(
        classLoader, CONNECTION_TYPES, handler);
    handler.createConnection(proxy);
    return proxy;
  }

Why on line 113 you copy the options? or inside this class copy the ConnectionFactory and not reuse it?

@igreenfield
Copy link
Contributor Author

Hi @jhalterman do you know why the lib always copy? hence when we use new version of the factory it may have new properties and if we forget to copy this it may be problem....

@jhalterman
Copy link
Owner

@igreenfield The thinking was that options are things that help you create a Connection, which cannot be changed once the connection is made, and Config are things that can be changed after the Connection is made. A new version of a connection factory should require a new connection, right?

@igreenfield
Copy link
Contributor Author

igreenfield commented Feb 15, 2017

@jhalterman New Connection yes, but hear you create connection from options the user pass to you... so why you copy it first?
even if you do copy do you need deep copy?

@jhalterman
Copy link
Owner

@igreenfield The thinking was I didn't want changes to the Options object to effect future connection attempts which is why we make a copy.

What sort of use case do you have where this is a problem?

@igreenfield
Copy link
Contributor Author

@jhalterman The 'ConnectionOptions' object expose the factory so as user I can change it and I will expect that my changes will take effect but with the copy it won't happen...

@jhalterman
Copy link
Owner

@igreenfield That's true. You would prefer ConnectionFactory changes to take effect if the connection is ever lost and recovered? In that case, why not make a new connection? As for the current behavior, do you think this could use more clarification in the docs?

@igreenfield
Copy link
Contributor Author

@jhalterman If I use the withConnectionFactory option and pass in my factory I would think the lib will use it but in the current implementation the lib will copy part of it and use other instance... this is not so good...

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

2 participants