Skip to content

Commit

Permalink
Cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
jmthomas committed Nov 20, 2024
1 parent c877945 commit c4909e6
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 53 deletions.
30 changes: 14 additions & 16 deletions openc3/lib/openc3/interfaces/http_client_interface.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,23 +188,21 @@ def read_interface
# Called to convert the read data into a OpenC3 Packet object
#
# @param data [String] Raw packet data
# @param extra [Hash] Key/Value pairs of extra data
# @param extra [Hash] Contains the following keys:
# HTTP_HEADERS - Hash of response headers
# HTTP_STATUS - Integer response status code
# HTTP_REQUEST - [data, extra]
# where data is the request data and extra contains:
# HTTP_REQUEST_TARGET_NAME - String request target name
# HTTP_URI - String request URI based on HTTP_PATH
# HTTP_PATH - String request path
# HTTP_METHOD - String request method
# HTTP_PACKET - String response packet name
# HTTP_ERROR_PACKET - Optional string error packet name
# HTTP_QUERIES - Optional hash of request queries
# HTTP_HEADERS - Optional hash of request headers
# @return [Packet] OpenC3 Packet with buffer filled with data
def convert_data_to_packet(data, extra = nil)
# extra contains the following keys:
# HTTP_HEADERS - Hash of response headers
# HTTP_STATUS - Integer response status code
# HTTP_REQUEST - [data, extra]
# where data is the request data and extra contains:
# HTTP_REQUEST_TARGET_NAME - String request target name
# HTTP_URI - String request URI based on HTTP_PATH
# HTTP_PATH - String request path
# HTTP_METHOD - String request method
# HTTP_PACKET - String response packet name
# HTTP_ERROR_PACKET - Optional string error packet name
# HTTP_QUERIES - Optional hash of request queries
# HTTP_HEADERS - Optional hash of request headers

packet = Packet.new(nil, nil, :BIG_ENDIAN, nil, data.to_s)
packet.accessor = HttpAccessor.new(packet)
# Grab the request extra set in the write_interface method
Expand All @@ -216,7 +214,7 @@ def convert_data_to_packet(data, extra = nil)
request_target_name = request_target_name.to_s.upcase
response_packet_name = request_extra['HTTP_PACKET']
error_packet_name = request_extra['HTTP_ERROR_PACKET']
# HTTP_STATUS was set in the base response_extra
# HTTP_STATUS was set in the base extra
status = extra['HTTP_STATUS'].to_i
if status >= 300 and error_packet_name
# Handle error special case response packet
Expand Down
78 changes: 49 additions & 29 deletions openc3/python/openc3/interfaces/http_client_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,21 @@ def disconnect(self):
super().disconnect()
self.response_queue.put(None)

# Reads from the socket if the read_port is defined
def read_interface(self):
data, extra = self.response_queue.get(block=True)
if data is None:
return None
self.read_interface_base(data, extra)
# Called to convert a packet into a data buffer. Write protocols then
# potentially modify the data in their write_data methods. Finally
# write_interface is called to send the data to the target.
#
# @param packet [Packet] Packet to extract data from
# @return data, extra
def convert_packet_to_data(self, packet):
extra = packet.extra
extra = extra or {}
data = packet.buffer # Copy buffer so logged command isn't modified
extra["HTTP_URI"] = f"{self.url}{packet.read('HTTP_PATH')}"
extra["HTTP_REQUEST_TARGET_NAME"] = packet.target_name
return data, extra

# Writes to the socket
# @param data [Hash] For the HTTP Interface, data is a hash with the needed request info
# Calls the http request method to send the data to the target
def write_interface(self, data, extra=None):
extra = extra or {}
params = extra.get("HTTP_QUERIES")
Expand Down Expand Up @@ -137,20 +142,51 @@ def write_interface(self, data, extra=None):
self.write_interface_base(data, extra)
return data, extra

# Returns the response data and extra from the interface
# which was queued up by the write_interface method.
# Read protocols can then potentially modify the data in their read_data methods.
# Then convert_data_to_packet is called to convert the data into a Packet object.
# Finally the read protocols read_packet methods are called.
def read_interface(self):
data, extra = self.response_queue.get(block=True)
if data is None:
return None
self.read_interface_base(data, extra)
return data, extra


# Called to convert the read data into a OpenC3 Packet object
#
# @param data [String] Raw packet data
# @param extra [dict] Contains the following keys:
# HTTP_HEADERS - Hash of response headers
# HTTP_STATUS - Integer response status code
# HTTP_REQUEST - [data, extra]
# where data is the request data and extra contains:
# HTTP_REQUEST_TARGET_NAME - String request target name
# HTTP_URI - String request URI based on HTTP_PATH
# HTTP_PATH - String request path
# HTTP_METHOD - String request method
# HTTP_PACKET - String response packet name
# HTTP_ERROR_PACKET - Optional string error packet name
# HTTP_QUERIES - Optional hash of request queries
# HTTP_HEADERS - Optional hash of request headers
# @return [Packet] OpenC3 Packet with buffer filled with data
def convert_data_to_packet(self, data, extra=None):
packet = Packet(None, None, "BIG_ENDIAN", None, data)
packet.accessor = HttpAccessor(packet)
if extra is not None:
# Grab the request extra set in the write_interface method
request_extra = None
if extra and extra['HTTP_REQUEST']:
request_extra = extra['HTTP_REQUEST'][1]
if request_extra is not None:
# Identify the response
request_target_name = extra.get("HTTP_REQUEST_TARGET_NAME")
request_target_name = request_extra.get("HTTP_REQUEST_TARGET_NAME")
if request_target_name is not None:
request_target_name = str(request_target_name).upper()
response_packet_name = extra.get("HTTP_PACKET")
error_packet_name = extra.get("HTTP_ERROR_PACKET")
response_packet_name = request_extra.get("HTTP_PACKET")
error_packet_name = request_extra.get("HTTP_ERROR_PACKET")
# HTTP_STATUS was set in the base extra
status = int(extra["HTTP_STATUS"])
if status >= 300 and error_packet_name is not None:
# Handle error special case response packet
Expand All @@ -163,21 +199,5 @@ def convert_data_to_packet(self, data, extra=None):

if not self.include_request_in_response:
extra.pop("HTTP_REQUEST", None)
extra.pop("HTTP_REQUEST_TARGET_NAME", None)
extra.pop("HTTP_REQUEST_PACKET_NAME", None)
packet.extra = extra

packet.extra = extra
return packet

# Called to convert a packet into the data to send
#
# @param packet [Packet] Packet to extract data from
# @return data
def convert_packet_to_data(self, packet):
extra = packet.extra
extra = extra or {}
data = packet.buffer # Copy buffer so logged command isn't modified
extra["HTTP_URI"] = f"{self.url}{packet.read('HTTP_PATH')}"
extra["HTTP_REQUEST_TARGET_NAME"] = packet.target_name
extra["HTTP_REQUEST_PACKET_NAME"] = packet.packet_name
return data, extra
14 changes: 6 additions & 8 deletions openc3/spec/interfaces/http_client_interface_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ module OpenC3
allow(mock_response).to receive(:headers).and_return({})
allow(mock_response).to receive(:status).and_return(204)
allow(mock_response).to receive(:body).and_return('')

expect(@interface.instance_variable_get(:@http)).to receive(:delete)
.with("#{@api_resource}/1", { 'confirm' => 'true' }, { 'Authorization' => 'Bearer token' })
.and_return(mock_response)
expect(@interface.instance_variable_get(:@http)).to receive(:delete) do |args, &block|
expect(args).to eql "#{@api_resource}/1"
# Not sure how to test block here
end.and_return(mock_response)

@interface.write_interface(data, extra)
expect(@interface.instance_variable_get(:@response_queue).pop).to eq(['', {
Expand Down Expand Up @@ -216,13 +216,13 @@ module OpenC3
end

it "sets target and packet names for successful responses" do
packet = @interface.convert_data_to_packet('success', { 'HTTP_STATUS' => 200, 'HTTP_REQUEST_TARGET_NAME' => 'TARGET', 'HTTP_PACKET' => 'SUCCESS' })
packet = @interface.convert_data_to_packet('success', { 'HTTP_STATUS' => 200, 'HTTP_REQUEST' => ['', {'HTTP_REQUEST_TARGET_NAME' => 'TARGET', 'HTTP_PACKET' => 'SUCCESS' }]})
expect(packet.target_name).to eq('TARGET')
expect(packet.packet_name).to eq('SUCCESS')
end

it "sets error packet name for error responses" do
packet = @interface.convert_data_to_packet('error', { 'HTTP_STATUS' => 404, 'HTTP_REQUEST_TARGET_NAME' => 'TARGET', 'HTTP_ERROR_PACKET' => 'ERROR' })
packet = @interface.convert_data_to_packet('error', { 'HTTP_STATUS' => 404, 'HTTP_REQUEST' => ['', {'HTTP_REQUEST_TARGET_NAME' => 'TARGET', 'HTTP_ERROR_PACKET' => 'ERROR' }]})
expect(packet.target_name).to eq('TARGET')
expect(packet.packet_name).to eq('ERROR')
end
Expand All @@ -241,7 +241,6 @@ module OpenC3

expect(data).to eq( @packet_data )
expect(extra['HTTP_REQUEST_TARGET_NAME']).to eq('TARGET')
expect(extra['HTTP_REQUEST_PACKET_NAME']).to eq('PACKET')
uri_str = extra['HTTP_URI'].encode('ASCII-8BIT', 'UTF-8')
expect(uri_str).to eq("https://example.com:8080#{@api_resource}")
end
Expand All @@ -262,7 +261,6 @@ module OpenC3

expect(uri_str).to eq("https://example.com:8080#{@api_resource}")
expect(extra['HTTP_REQUEST_TARGET_NAME']).to eq('TARGET')
expect(extra['HTTP_REQUEST_PACKET_NAME']).to eq('PACKET')
end
end
end
Expand Down

0 comments on commit c4909e6

Please sign in to comment.