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

add templating capabilities #3

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 89 additions & 20 deletions lib/request_interceptor.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require 'socket'
require 'json'
require 'logger'
require 'digest'
Expand All @@ -17,55 +18,121 @@ def initialize(app, logger: Logger.new('hawksi.log'), storage: FileStorage)
@app = app
@logger = logger
@storage = storage
@templates = {}
@templates_mutex = Mutex.new

@socket_path = '/tmp/hawksi.sock'
start_unix_socket_server
Comment on lines +24 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Make the Unix socket path configurable

Currently, the socket path @socket_path = '/tmp/hawksi.sock' is hardcoded, which may lead to conflicts or reduce flexibility in different environments. Consider making it configurable through an environment variable or an initialization parameter.

Apply this diff to make the socket path configurable:

 def initialize(app, logger: Logger.new('hawksi.log'), storage: FileStorage)
   @app = app
   @logger = logger
   @storage = storage
   @templates = {}
   @templates_mutex = Mutex.new
-  @socket_path = '/tmp/hawksi.sock'
+  @socket_path = ENV['HAWKSI_SOCKET_PATH'] || '/tmp/hawksi.sock'
   start_unix_socket_server
 end
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@socket_path = '/tmp/hawksi.sock'
start_unix_socket_server
@socket_path = ENV['HAWKSI_SOCKET_PATH'] || '/tmp/hawksi.sock'
start_unix_socket_server

end

def call(env)
request = Rack::Request.new(env)
request_key = generate_request_key(request) # Generate a key for the request

# Check if there's a template for this request
template = nil
@templates_mutex.synchronize do
template = @templates[request_key]
end
if template
status, headers, _temp = @app.call(env)

# Serve the template
headers['Etag'] = Digest::MD5.hexdigest(template)
return [status, headers, [template]]
Comment on lines +38 to +42
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid unnecessary application calls when serving templates

When a template is found, the code still calls @app.call(env) to obtain the status and headers, which can be inefficient if the application is resource-intensive. Consider defining default status and headers to avoid unnecessary application calls.

Apply this diff to bypass calling the application when serving a template:

-        status, headers, _temp = @app.call(env)
-
+        status = 200
+        headers = { 'Content-Type' => 'application/json' }
         # Serve the template
         headers['Etag'] = Digest::MD5.hexdigest(template)
         return [status, headers, [template]]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
status, headers, _temp = @app.call(env)
# Serve the template
headers['Etag'] = Digest::MD5.hexdigest(template)
return [status, headers, [template]]
status = 200
headers = { 'Content-Type' => 'application/json' }
# Serve the template
headers['Etag'] = Digest::MD5.hexdigest(template)
return [status, headers, [template]]

end

# Original code
return MocksiHandler.handle(request) if request.path.end_with?('/favicon.ico')

if request.path.start_with?('/mocksi') || request.path.start_with?('/_') || request.path.start_with?('/api')
return MocksiHandler.handle(request)
end

request_hash = generate_request_hash(request) # Generate a hash of the request
log_request(request, request_hash)
log_request(request, request_key)

status, headers, response = @app.call(env)

log_response(status, headers, response, request_hash)
log_response(status, headers, response, request_key)
[status, headers, response]
end

private

def generate_request_hash(request)
# Generate a hash based on request method, path, query string, and body
hash_input = [
request.request_method,
request.path,
request.query_string,
request.body&.read # Read the body content to include in the hash
].join
def start_unix_socket_server
# Remove the socket file if it already exists
File.unlink(@socket_path) if File.exist?(@socket_path)

# Reset the body input stream for future use
request.body&.rewind
at_exit do
File.unlink(@socket_path) if File.exist?(@socket_path)
end

@server_thread = Thread.new do
@server = UNIXServer.new(@socket_path)
@logger.info("Unix socket server started at #{@socket_path}")
puts "Unix socket server started at #{@socket_path}"
loop do
client = @server.accept
Thread.new(client) do |conn|
handle_client(conn)
end
end
rescue StandardError => e
@logger.error("Unix socket server error: #{e.message}")
ensure
@server.close if @server
end
end

# Return a SHA256 hash of the concatenated string
Digest::SHA256.hexdigest(hash_input)
def handle_client(conn)
while message = conn.gets
# Process the message
process_message(message.chomp)
end
rescue StandardError => e
@logger.error("Error handling client: #{e.message}")
ensure
conn.close
end

def log_request(request, request_hash) # rubocop:disable Metrics/AbcSize,Metrics/MethodLength
def process_message(message)
data = JSON.parse(message)
case data['action']
when 'set_template'
key = data['key']
template = data['template']
@templates_mutex.synchronize do
@templates[key] = template
end
@logger.info("Template set for key: #{key}")
when 'remove_template'
key = data['key']
@templates_mutex.synchronize do
@templates.delete(key)
end
@logger.info("Template removed for key: #{key}")
else
@logger.warn("Unknown action: #{data['action']}")
end
rescue JSON::ParserError => e
puts("JSON parsing error: #{e.message}")
@logger.error("JSON parsing error: #{e.message}")
end

def generate_request_key(request)
"#{request.request_method}_#{request.path}"
end
Comment on lines +122 to +124
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Include query parameters in request key to ensure uniqueness

The generate_request_key method uses request.request_method and request.path, which might not be unique if the same path is used with different query parameters. To avoid key collisions, consider including the query string in the request key.

Apply this diff to include query parameters:

 def generate_request_key(request)
-  "#{request.request_method}_#{request.path}"
+  "#{request.request_method}_#{request.fullpath}"
 end
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def generate_request_key(request)
"#{request.request_method}_#{request.path}"
end
def generate_request_key(request)
"#{request.request_method}_#{request.fullpath}"
end


def log_request(request, request_key) # rubocop:disable Metrics/AbcSize,Metrics/MethodLength
data = {
request_hash: request_hash, # Include the request hash in the logged data
request_key: request_key, # Include the request key in the logged data
method: request.request_method,
path: request.path,
query_string: request.query_string,
url: request.url,
scheme: request.scheme,
host: request.host,
port: request.port,
# Log only specific parts of the env hash to avoid circular references
env: {
'REQUEST_METHOD' => request.env['REQUEST_METHOD'],
'SCRIPT_NAME' => request.env['SCRIPT_NAME'],
Expand Down Expand Up @@ -95,16 +162,18 @@ def log_request(request, request_hash) # rubocop:disable Metrics/AbcSize,Metrics
@storage.store('requests', data)
rescue StandardError => e
@logger.error("Error logging request: #{e.message}")
ensure
request.body&.rewind
end

def log_response(status, headers, response, request_hash) # rubocop:disable Metrics/MethodLength
def log_response(status, headers, response, request_key) # rubocop:disable Metrics/MethodLength
body = if response.respond_to?(:body)
response.body.join.to_s
else
response.join.to_s
end
data = {
request_hash: request_hash, # Include the request hash in the response log
request_key: request_key, # Include the request key in the response log
status: status,
headers: headers,
body: body,
Expand Down
39 changes: 39 additions & 0 deletions lib/uploads_cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def initialize(*args)
@client_uuid = current_client_uuid
@file_uploader = FileUploader.new(@logger, @client_uuid)
@command_executor = CommandExecutor.new(@logger, @client_uuid)
@socket_path = '/tmp/hawksi.sock'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider improving socket path configuration and error handling.

The socket path is currently hardcoded, which might limit flexibility across different environments. Consider the following improvements:

  1. Make the socket path configurable, possibly through an environment variable or a configuration file.
  2. Add a check to ensure the socket file exists and is accessible when initializing the class.

Here's a suggested implementation:

def initialize(*args)
  super
  # ... existing code ...
  @socket_path = ENV['HAWKSI_SOCKET_PATH'] || '/tmp/hawksi.sock'
  validate_socket_path
end

private

def validate_socket_path
  unless File.exist?(@socket_path) && File.socket?(@socket_path)
    raise "Socket file not found or invalid: #{@socket_path}"
  end
end

This change allows for configuration via an environment variable and includes a basic validation of the socket file.

end

desc 'update', 'Update uploaded requests and responses'
Expand Down Expand Up @@ -62,6 +63,44 @@ def execute(command, *params)
@command_executor.execute_command(command, params)
end

desc 'set_template KEY FILENAME', 'Set a template for the given key using the content of the specified file'
option :base_dir, type: :string,
desc: 'Base directory for storing intercepted data. Defaults to ./tmp/intercepted_data'
def set_template(key, filename)
set_base_dir
unless File.exist?(filename)
@logger.error "File not found: #{filename}"
return
end

# Read the template content from the file
template_content = File.read(filename)

message = {
action: 'set_template',
key: key,
template: template_content
}
# Send the message to the /tmp/hawksi.sock socket
socket = UNIXSocket.new(@socket_path)
socket.write(message.to_json)
socket.close
end
Comment on lines +66 to +88
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor set_template method for improved error handling and readability.

The set_template method could benefit from some refactoring to improve error handling, reduce method length, and ensure proper socket communication. Consider the following suggestions:

  1. Wrap the socket communication in a begin-rescue block to handle potential errors.
  2. Ensure the socket is always closed, even in case of an error.
  3. Extract the file reading and socket communication into separate private methods to reduce the main method's length.

Here's a suggested refactoring:

def set_template(key, filename)
  set_base_dir
  template_content = read_template_file(filename)
  send_template_message(key, template_content) if template_content
end

private

def read_template_file(filename)
  File.read(filename)
rescue Errno::ENOENT
  @logger.error "File not found: #{filename}"
  nil
end

def send_template_message(key, template_content)
  message = {
    action: 'set_template',
    key: key,
    template: template_content
  }
  send_socket_message(message)
end

def send_socket_message(message)
  socket = UNIXSocket.new(@socket_path)
  socket.write(message.to_json)
rescue StandardError => e
  @logger.error "Error sending message to socket: #{e.message}"
ensure
  socket&.close
end

This refactoring improves error handling, ensures the socket is always closed, and reduces the main method's length by extracting functionality into separate methods.

🧰 Tools
🪛 rubocop

[convention] 69-88: Method has too many lines. [14/10]

(Metrics/MethodLength)


desc 'remove_template KEY', 'Remove the template for the given key'
option :base_dir, type: :string,
desc: 'Base directory for storing intercepted data. Defaults to ./tmp/intercepted_data'
def remove_template(key)
set_base_dir
message = {
action: 'remove_template',
key: key
}
socket = UNIXSocket.new(@socket_path)
socket.write(message.to_json)
socket.close
end
Comment on lines +90 to +102
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve remove_template method with error handling and code reuse.

The remove_template method could benefit from improved error handling and code reuse. Consider the following suggestions:

  1. Reuse the send_socket_message method suggested in the previous comment for consistent error handling and socket management.
  2. Keep the method focused on its specific task by extracting the message creation.

Here's a suggested refactoring:

def remove_template(key)
  set_base_dir
  message = create_remove_template_message(key)
  send_socket_message(message)
end

private

def create_remove_template_message(key)
  {
    action: 'remove_template',
    key: key
  }
end

# Use the send_socket_message method from the previous comment

This refactoring improves error handling, ensures consistent socket management, and promotes code reuse between set_template and remove_template methods.


private

def set_base_dir
Expand Down
Loading