Skip to content
This repository was archived by the owner on May 7, 2024. It is now read-only.
Merged
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
Prev Previous commit
Next Next commit
Updated tests with more scenarios, added doc strings to document API …
…of json rpc lib, added skeleton folder structure for future cli tools, added test setup, added dev set up files for installing dependencies and running tests
  • Loading branch information
Ron Quan committed Feb 24, 2017
commit 22f4e1c9134f45fd6bad908f4e59e60af5f22315
30 changes: 30 additions & 0 deletions doc/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
1. Install the dependencies via pip with the script below.
```Shell
python scripts/dev_setup.py

2. Add `<clone root>\src` to your PYTHONPATH environment variable:

#####Windows
```BatchFile
set PYTHONPATH=<clone root>\src;%PYTHONPATH%
```
#####OSX/Ubuntu (bash)
```Shell
export PYTHONPATH=<clone root>/src:${PYTHONPATH}

##Running Tests:
####Command line
#####Windows:
Provided your PYTHONPATH was set correctly, you can run the tests from your `<root clone>` directory.

To test the common modules of the CLI:
```BatchFile
python -m unittest discover -s src/common/tests
```

To test the scripter module of the CLI:
```BatchFile
python -m unittest discover -s src/mssqlscripter/mssql/scripter/tests
```

Additionally, you can run tests for all CLI tools and common modules using the `Run_All_Tests.bat` script.
3 changes: 3 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
enum34==1.1.6
pip==9.0.1
setuptools==30.4.0
Copy link
Member

@pensivebrian pensivebrian Feb 24, 2017

Choose a reason for hiding this comment

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

What is 'setuptools' used for? #ByDesign

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setuptools builds the package so we can use setup.py


In reply to: 103035860 [](ancestors = 103035860)

Empty file added src/common/HISTORY.rst
Empty file.
Empty file added src/common/MANIFEST.ini
Empty file.
2 changes: 2 additions & 0 deletions src/common/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Microsoft XPlat Cli common module
Copy link
Member

@pensivebrian pensivebrian Feb 24, 2017

Choose a reason for hiding this comment

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

nit: CLI #Resolved

=================================
4 changes: 4 additions & 0 deletions src/common/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# --------------------------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------
108 changes: 78 additions & 30 deletions src/json-rpc/json/rpc/json_rpc.py → src/common/json_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,18 @@
# --------------------------------------------------------------------------------------------
Copy link
Member

@pensivebrian pensivebrian Feb 24, 2017

Choose a reason for hiding this comment

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

nit: can use use single quotes in this file instead on double quotes - seems to be the Python convention. #Resolved


from io import BytesIO
from enum import Enum
import json

class JSON_RPC_Writer(object):
class Read_State(Enum):
Header = 1
Content = 2

class Json_Rpc_Writer(object):
"""
Writes to the supplied stream through the JSON RPC Protocol where a request is formatted through a method
name and the necessary parameters.
"""
HEADER = "Content-Length: {0}\r\n\r\n"

def __init__(self, stream, encoding = None):
Expand All @@ -15,7 +24,10 @@ def __init__(self, stream, encoding = None):
if encoding is None:
self.encoding = 'UTF-8'

def send_request(self, method, params, id):
def send_request(self, method, params, id = None):
"""
Forms and writes a JSON RPC protocol compliant request a method and it's parameters to the stream.
"""
# Perhaps move to a different def to add some validation
content_body = {
"jsonrpc": "2.0",
Expand All @@ -27,10 +39,16 @@ def send_request(self, method, params, id):
json_content = json.dumps(content_body)
header = self.HEADER.format(str(len(json_content)))

self.stream.write(header.encode("ascii"))
self.stream.write(header.encode('ascii'))
self.stream.write(json_content.encode(self.encoding))

class JSON_RPC_Reader(object):
self.stream.flush()

class Json_Rpc_Reader(object):
"""
Reads from the supplied stream through the JSON RPC Protocol. A Content-length header is required in the format
of "Content-Length: <number of bytes>".
Various exceptions may occur during the read process and are documented in each method.
"""
# \r\n
CR = 13
Copy link
Member

@pensivebrian pensivebrian Feb 22, 2017

Choose a reason for hiding this comment

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

I think a '\r' is more clear - can we use that instead? #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would require us to slice the byte array to decode in a contiguous array since each byte is a int and doesn't support decode;i.e buffer[scanoffset:scanoffset+3].decode('ascii') == '\r\n\r\n' which I think could introduce unecessary checks like if the offset + 3 exceeds the boundaries. Checking byte by it's numeric value looks cleaner IMO. Let me know what you think.


In reply to: 102364905 [](ancestors = 102364905)

LF = 10
Copy link
Member

@pensivebrian pensivebrian Feb 22, 2017

Choose a reason for hiding this comment

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

Same thing, I think a '\n' is more clear than the numeric constant. #ByDesign

Expand All @@ -50,31 +68,46 @@ def __init__(self, stream, encoding = None):
self.read_offset = 0
self.expected_content_length = 0
self.headers = {}
#TODO: Create enum
self.read_state = "Header"
self.read_state = Read_State.Header

def read_response(self):
"""
Reads the response from the supplied stream by chunks into a buffer until all headers and body content are read.

Returns the response body content in JSON
Exceptions raised:
ValueError
if the body-content can not be serialized to a JSON object
"""
# Using a mutable list to hold the value since a immutable string passed by reference won't change the value
content = [""]
while (self.read_next_chunk()):
# If we can't read a header, read the next chunk
if (self.read_state == "Header" and not self.try_read_headers()):
if (self.read_state is Read_State.Header and not self.try_read_headers()):
continue
# If we read the header, try the content. If that fails, read the next chunk
if (self.read_state == "Content" and not self.try_read_content(content)):
if (self.read_state is Read_State.Content and not self.try_read_content(content)):
continue
# We have the content
break

# Resize buffer and remove bytes we have read
self.shift_buffer_bytes_and_reset(self.read_offset)
self.trim_buffer_and_resize(self.read_offset)
try:
return json.loads(content[0])
except ValueError as error:
# response has invalid json object, throw Exception TODO: log message
except ValueError:
# response has invalid json object, throw Exception TODO: log message to telemetry
raise

def read_next_chunk(self):
"""
Reads a chunk of the stream into the byte array. Buffer size is doubled if less than 25% of buffer space is available.abs
Exceptions raised:
EOFError
Stream was empty or Stream did not contain a valid header or content-body
IOError
Stream was closed externally
"""
# Check if we need to resize
current_buffer_size = len(self.buffer)
if ((current_buffer_size - float(self.buffer_end_offset)) / current_buffer_size) < self.BUFFER_RESIZE_TRIGGER:
Expand All @@ -92,17 +125,23 @@ def read_next_chunk(self):

if (length_read == 0):
# Nothing was read, could be due to the server process shutting down while leaving stream open
# close stream and return false and/or throw exception?
# for now throwing exception
raise EOFError("End of stream reached with no valid header or content-body")

return True

except Exception:
#TODO: Add more granular exception message
except IOError as error:
# TODO: add to telemetry
raise

def try_read_headers(self):
"""
Attempts to read the Header information from the internal buffer expending the last header contain "\r\n\r\n.

Returns false if the header was not found.
Exceptions:
LookupError The content-length header was not found
ValueError The content-length contained a invalid literal for int
"""
# Scan the buffer up until right before the CRLFCRLF
scan_offset = self.read_offset
while(scan_offset + 3 < self.buffer_end_offset and
Expand All @@ -123,42 +162,51 @@ def try_read_headers(self):
colon_index = header.find(':')

if (colon_index == -1):
raise KeyError("Colon missing from Header")
raise KeyError("Colon missing from Header: {0}.".format(header))

header_key = header[:colon_index]
# Making all headers lowercase to support case insensitivity
header_key = header[:colon_index].lower()
header_value = header[colon_index + 1:]

self.headers[header_key] = header_value

#Find content body in the list of headers and parse the Value
if (self.headers["Content-Length"] is None):
raise LookupError("Content Length was not found in headers received")
if (not ("content-length" in self.headers)):
raise LookupError("Content-Length was not found in headers received.")

self.expected_content_length = int(self.headers["Content-Length"])

except Exception:
# Trash the buffer we read and shift past read content
self.shift_buffer_bytes_and_reset(self.scan_offset + 4)
self.expected_content_length = int(self.headers["content-length"])
except ValueError:
# Content-length contained invalid literal for int
self.trim_buffer_and_resize(scan_offset + 4)
raise

# Pushing read pointer past the newline characters
self.read_offset = scan_offset + 4
# TODO: Create enum for this
self.read_state = "Content"
self.read_state = Read_State.Content

return True

def try_read_content(self, content):
"""
Attempts to read the content from the internal buffer.

Returns false if buffer does not contain the entire content.
"""
# if we buffered less than the expected content length, return false
if (self.buffer_end_offset - self.read_offset < self.expected_content_length):
return False

content[0] = self.buffer[self.read_offset:self.read_offset + self.expected_content_length].decode(self.encoding)
self.read_offset += self.expected_content_length
#TODO: Create a enum for this
self.read_state = "Header"

self.read_state = Read_State.Header
return True

def shift_buffer_bytes_and_reset(self, bytes_to_remove):
def trim_buffer_and_resize(self, bytes_to_remove):
"""
Trims the buffer by the passed in bytes_to_remove by creating a new buffer that is at a minimum the default max size.
"""
current_buffer_size = len(self.buffer)
# Create a new buffer with either minumum size or leftover size
new_buffer = bytearray(max(current_buffer_size - bytes_to_remove, self.DEFAULT_BUFFER_SIZE))
Expand Down
Loading