This post originated from an RSS feed registered with Ruby Buzz by Jamis Buck. Original Post: Refactoring Net::SSH: Part 4 Feed Title: the buckblogs here Feed URL: http://weblog.jamisbuck.org/blog.cgi/programming/index.rss Feed Description: Jamis Buck's corner of the blogging universe. Mostly about ruby, but includes ramblings on a variety of topics. Latest Ruby Buzz Posts Latest Ruby Buzz Posts by Jamis Buck Latest Posts From the buckblogs here

The adventure continues, as I persist in refactoring Net::SSH to take advantage of dependency injection, using Needle. This issue of the adventure will detail a snag in the refactoring, and some of my current thinking to overcome it.

I’m down to the last piece of the transport layer implementation. (For those that care, the transport layer is the lowest level of the SSH2 protocol, managing the sending and receiving of packets. This includes compression, encryption, and validation of packets.) This last piece is the Net::SSH::Transport::Session class.

The session is the piece that ties all the different parts of the transport layer together, sitting them all down at the table and making them work together. Specifically, the transport layer’s responsibilities include:

1. Establish a network connection to an SSH server.
2. Exchange version information with the other end of the connection.
3. Negotiate (with the remote node) the preferred algorithms to use for ciphers, key exchanging, compression and so forth.
4. Exchange keys
5. Provide an abstraction for sending and receiving packets

To accomplish these responsibilities, the session has dependencies on the following services:

• outgoing_packet_stream (for sending packets)
• incoming_packet_stream (for receiving packets)
• kex_names (for identifying the kex algorithm implementation to use)
• cipher_factories (for returning cipher implementations appropriate for the selected cryptography backend)
• key_factories (for returning key implementations)
• hmac_factories (for returning hmac implementations)
• buffer_factories (for returning buffer implementations)
• compression_algorithms (for returning compression algorithm implementations)
• decompression_algorithms (for returning decompression algorithm implementations)
• logs (for obtaining a new logger reference)
• socket_factory (for opening a network connection to the requested host/port)

There may even be a few more, as I dig deeper into refactoring this beast.

The challenge is this: nearly every one of those services is needed in the session’s constructor. That means that injecting the dependencies as setters won’t work—they don’t get set soon enough. Every single one of th 1000 ese must be passed as an argument to the constructor.

Yech.

So, I’ve spent some time thinking about this. There are a few different ways to go about this.

Cave. Just pass them to the constructor.

The first option is to just do it the “academic” way. “The process recommends approach X, so I’ll do approach X.”

I don’t think anyone will blame me from balking at this. How many parameters does that mean the constructor takes? Eleven?!

  transport.register( :session ) do |c,p|
    Net::SSH::Transport::Session.new(
      c.outgoing_packet_stream, c.incoming_packet_stream,
      c.kex_names, c.cipher_factories[c.crypto_backend],
      c.key_factories[c.crypto_backend], c.hmac_factories[c.crypto_backend],
      c.buffer_factories[c.crypto_backend], c.compression_algorithms,
      c.decompression_algorithms, c.logs.get(p.fullname), c.socket_factory
    )
  end

No.

True, the burden of this could be mitigated somewhat by sending a hash to the constructor and injecting the parameters that way. That almost makes it look like setter injection, too:

  transport.register( :session ) do |c,p|
    Net::SSH::Transport::Session.new(
      :outgoing_packet_stream => c.outgoing_packet_stream,
      :incoming_packet_stream => c.incoming_packet_stream,
      :kex_names              => c.kex_names,
      :cipher_factory         => c.cipher_factories[ c.crypto_backend ],
      ...
    )
  end

A little better. I might at least consider such an approach. But there’s got to be a better way!

Use setters and a custom ‘init’ method.

Constructor injection is fine—don’t get me wrong—but my personal preference is setter injection. When using constructor injection, it becomes hard to deal with many dependencies (as demonstrated above). Also, you can’t name the dependencies (unless you use a hash, which is too prone to typos).

So, another option I considered is to just use setter injection. The problem with this in this case, as I mentioned, is that the logic that needs the dependencies is in the constructor, which is called before the setters can be called.

The solution, then, is to put the construction logic that needs the dependencies in a separate method, which is invoked explicitly after the setters have been taken care of.

  transport.register( :session ) do |c,p|
    session = Net::SSH::Transport::Session.new
    session.outgoing_packet_stream = c.outgoing_packet_stream
    session.incoming_packet_stream = c.incoming_packet_stream
    session.kex_names              = c.kex_names
    session.cipher_factory         = c.cipher_factories[ c.crypto_backend ]
    ...
    session.initialize_service
  end

I like this better, because I can use my beloved setter injection. However, having to invoke the “initialize_service” method explicitly feels a bit kludgy. It is also prone to error, since that step would be easy to forget. (Fortunately, it would only need to exist in a very few places in the code, so that’s not as big of a drawback as it could be, in this case.)

Should Needle handle this case?

So the question that came to my mind was, “Should Needle handle this case?” Needle could, of course, be modified to check each newly instantiated service to see if it reponds to a special method name (like “initialize_service”), and if it does, invoke that method automatically. It could, but s 1000 hould it?

My initial urge was to say, “sure!” and jump in head first, coding like mad. This approach is, after all, supported by Copland and HiveMind. It works nicely in those frameworks. Upon reflection, though, I think it does not belong in Needle, and here’s why:

Copland and HiveMind try to be application frameworks. They try to do a lot of things, most of it magic, to make life easier for the developer. This makes those frameworks large and complex (which is not necessarily a bad thing).

The goal of Needle is to be a framework framework. That is to say, other frameworks may be built on top of Needle, adding complexity as needed. Needle itself should stay small, and relatively simple. This allows Needle to be fast as well, since the common case does not need (for example) to invoke an initialize_service method and should not be slowed down by an unnecessary call to responds_to?.

This means that developers using Needle directly may need to do a little more work (in some of the less common situations) than they would with other frameworks, like Copland. However, for most tasks, Needle will be at least as easy to use as those frameworks, and will scale much better, to boot.

If someone wanted to be able to add the automatic call to “initialize_service”, they could do it as a new service model. Or, as a new element in the instantiation pipeline, to use the terminology I described in Thoughts on Service Models.

So what approach will I take with the Transport::Session service? Not sure yet. It’ll be either the send-hash-to-constructor approach, or the setters-with-explicity-init approach. Tune in next time to hear the resolution to this dilemma…