design code suggestions?

parser_worker.rb

class ToolParserWorker
  include Sidekiq::Worker

  def perform(scan)

    ## On a near futute NessusParser will die an become a ParserFactory based on scan tool type
    report = NessusParser.execute(self.file.current_path) # execute returns a Report object

    scan.update_parsed_report(report.to_json) # scan is mongo document
  end
end

Curti a alternativa do @rtopitt, pois é mais minimalista.

Eu faria assim:

class ToolParserWorker
  include Sidekiq::Worker
  def perform(scan_id)
    UpdateScanParser.new(scan_id).execute
  end
end

class UpdateScanParser
  def initialize(scan_id)
    @scan_id = scan_id
  end
  def execute
    scan.update_parsed_report(report.to_json)
  end
  private
    def report
      NessusParser.execute(scan.file.current_path)
    end
    def scan
      @scan ||= Scan.find(@scan_id)
    end
end

Acho que a ideia do pellegrino é a mais legal. A.classe de UpdateScanParser deve receber um objeto de parser o que vai deixar ela bem generica para fazer.a.persistencia de.qualquer tipo de parser. So nao achei legal inicializar o objeto de parser na inicialização. Acho que é melhor dar um tratamento com uma exception se nao for o objeto, do tipo, ou exatamente o que.vc.espera.

Gostei da sugestão do @pellegrino, apenas uma correção em UpdateScanParser#parse:

class ToolParserWorker
  include Sidekiq::Worker

  def perform(scan_id)
    UpdateScanParser.new(scan_id).parse
  end
end

class UpdateScanParser

  def initialize(scan_id, parser=NessusParser.new)
    @scan_id = scan_id
    @parser  = parser
  end 

  def parse
    scan = Scan.find(id: scan_id)
    @parser.parse(scan) # aqui é parse, e não execute
  end
end

class NessusParser
  def parse(scan)
     report = execute(scan.file_path)
     scan.update_parsed_report(report.to_json)
  end 
  private
  def execute(path)
     # ....
     return report
  end 
end

O que me agrada nessa solução é que as responsabilidades estão bem definidas em cada uma das classes. A observação dele quanto a scan.file_path vs. scan.file.current_path também é bem legal, pois obedece a Lei de Demeter ("Only talk to your immediate friends.").

O problema que existe nessa solução é que de qualquer forma o Model ficará acoplando com o Parser, que é justamente o que estou querendo evitar. O NessusParser é só um, de um conjunto de parsers de xml, que eu não queria que tivessem dependências com a regra da aplicação, ou seja, só faz parsear o xml e retorna um objeto report. O que motivou a criar os parsers próprios e não utilizar os existentes, foi o os módules de parser terem dependência com ORM.

Dado esse cenário, utilizando a sugestão do @pellegrino, o que pensei:

class ToolParserWorker
  include Sidekiq::Worker

  def perform(scan_id)
    UpdateScanParser.new(scan_id).parse
  end
end

class UpdateScanParser
  def initialize(scan_id)
    @scan_id = scan_id
  end

  def parse
    report = parser.parse(scan)
    scan.update_parsed_report(report.to_json)
  end

  private
  def scan
    scan ||= Scan.find(id: @scan_id)
  end

  def parser
    parser ||= ParserFactory.instance(scan.tool)
  end
end

class NessusParser
  def execute(path)
     # ....
     return report
  end
end

class Scan
  #...
  def update_parsed_report(json)
    self.parsed_document = json
    sef.save!
  end
end

Dessa forma, fica obscuro para que está chamando UpdateScanParser que ele chama uma fábrica de Parser internamente, o que me incomoda é que método UpdateScanParser#parse, parece desrespeitar o SRP, mas ao mesmo tempo não acho que deveria criar mais uma classe pra tratar a o update externamente.

@tapajos coloquei o que o update_parsed_report faz ;)

Não tinha entendido que haveriam vários parsers, neste caso fico com a solução do @pellegrino, reescrita pelo @caike