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

Convert VirusScanner class into stateless service #372

Merged
merged 10 commits into from
Jan 3, 2018

Commits on Dec 29, 2017

  1. Remove unnecessary optimisation in VirusScanner

    The VirusScanner class is only ever instantiated in the VirusScanWorker
    and in that case, the #clean method is only called once on that
    instance. Thus this optimisation is only ever exercised in the
    VirusScanner spec example which I have removed.
    
    My motivation for making this change is that I want to convert
    VirusScanner into a stateless service to bring it into line with
    S3Storage and services in other GOV.UK apps. This change is a step in
    that direction.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    5fd137a View commit details
    Browse the repository at this point in the history
  2. Inline VirusScanner#clean? into #scan & make #scan public

    The #clean method is no longer adding much value and doing this will
    make it easier to convert this class into a stateless service.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    516a737 View commit details
    Browse the repository at this point in the history
  3. DRY up VirusScanner spec

    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    d31ae76 View commit details
    Browse the repository at this point in the history
  4. Move file_path arg from VirusScanner constructor to #scan

    This is a further step towards converting this class into a stateless
    service.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    44ba887 View commit details
    Browse the repository at this point in the history
  5. Raise exception when VirusScanner#scan detects a virus

    Previously this method was returning `false` if a virus was detected.
    I'm converting this class into a stateless service and so I want to
    avoid having to store `virus_info` when a virus is detected. We can do
    this by raising the exception and handling it in the VirusScanWorker.
    Handling the exception in the worker is important, because we want to
    avoid triggering the Sidekiq retry mechanism.
    
    Note that the `virus_info` string is now passed as the exception message
    and since we send the original exception to GovukError.notify, there's
    no longer any need to include it in the `extra` Hash.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    1861d12 View commit details
    Browse the repository at this point in the history
  6. Simplify VirusScanner#scan

    By removing this last remaining state variable, the class is now a
    stateless service.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    f71c3fd View commit details
    Browse the repository at this point in the history
  7. Remove unnecessary comment from VirusScanner class

    Now that we are explicitly handling VirusScanner::InfectedFile
    exceptions in VirusScanWorker#perform, I think this comment is less
    useful and just clutters up the code.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    9bf7ac4 View commit details
    Browse the repository at this point in the history
  8. Improvements to VirusScanner spec

    Extract common code into `before` & `let` blocks and use `context`
    blocks to make the appriopriate changes to the context for each example.
    
    This also has the benefit of removing some long lines.
    
    I've also replaced the `status` double with an instance double which is
    more likely to catch problems.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    9c98651 View commit details
    Browse the repository at this point in the history
  9. Improvements to VirusScanWorker spec

    Extract common code into `before` & `let` blocks and use `context`
    blocks to make the appriopriate changes to the context for each example.
    
    This also has the benefit of removing some long lines.
    
    I've also replaced the `scanner` double with an instance double which is
    more likely to catch problems.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    5ed5625 View commit details
    Browse the repository at this point in the history
  10. Make VirusScanner available as a service c.f. S3Storage

    Now that the VirusScanner class is a stateless service, we can include
    it in the standard Services module to bring this app more into line with
    other GOV.UK apps.
    floehopper committed Dec 29, 2017
    Configuration menu
    Copy the full SHA
    03a756c View commit details
    Browse the repository at this point in the history