Commit 6f5ce2f5 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[3008] Further corrections based on late arriving review

comments. Again, these are mostly clean-up, nothing
substantial.
parent 0a3773e5
......@@ -191,7 +191,7 @@ public:
///
/// @param io_service is the IOService that will handle IO event processing.
///
/// @throw throws NcrListenError if the listener is already "listening" or
/// @throw NcrListenError if the listener is already "listening" or
/// in the event the open or doReceive methods fail.
void startListening(isc::asiolink::IOService& io_service);
......@@ -458,7 +458,7 @@ public:
///
/// @param io_service is the IOService that will handle IO event processing.
///
/// @throw throws NcrSenderError if the sender is already "sending" or
/// @throw NcrSenderError if the sender is already "sending" or
/// NcrSenderOpenError if the open fails.
void startSending(isc::asiolink::IOService & io_service);
......@@ -476,7 +476,7 @@ public:
///
/// @param ncr is the NameChangeRequest to send.
///
/// @throw throws NcrSenderError if the sender is not in sending state or
/// @throw NcrSenderError if the sender is not in sending state or
/// the request is empty; NcrSenderQueueFull if the send queue has reached
/// capacity.
void sendRequest(NameChangeRequestPtr& ncr);
......@@ -536,7 +536,7 @@ public:
/// This method can be used to discard all of the NCRs currently in the
/// the send queue. Note it may not be called while the sender is in
/// the sending state.
/// @throw throws NcrSenderError if called and sender is in sending state.
/// @throw NcrSenderError if called and sender is in sending state.
void clearSendQueue();
/// @brief Abstract method which opens the IO sink for transmission.
......@@ -569,7 +569,7 @@ public:
///
/// @throw If the implementation encounters an error it MUST
/// throw it as an isc::Exception or derivative.
virtual void doSend(NameChangeRequestPtr ncr) = 0;
virtual void doSend(NameChangeRequestPtr& ncr) = 0;
/// @brief Returns true if the sender is in send mode, false otherwise.
///
......
......@@ -455,5 +455,23 @@ NameChangeRequest::toText() const {
return (stream.str());
}
bool
NameChangeRequest::operator == (const NameChangeRequest& other) {
return ((change_type_ == other.change_type_) &&
(forward_change_ == other.forward_change_) &&
(reverse_change_ == other.reverse_change_) &&
(fqdn_ == other.fqdn_) &&
(ip_address_ == other.ip_address_) &&
(dhcid_ == other.dhcid_) &&
(lease_expires_on_ == other.lease_expires_on_) &&
(lease_length_ == other.lease_length_));
}
bool
NameChangeRequest::operator != (const NameChangeRequest& other) {
return (!(*this == other));
}
}; // end of isc::dhcp namespace
}; // end of isc namespace
......@@ -73,13 +73,13 @@ public:
/// a contiguous stream of digits, with no delimiters. For example a string
/// containing "14A3" converts to a byte array containing: 0x14, 0xA3.
///
/// @throw throws a NcrMessageError if the input data contains non-digits
/// @throw NcrMessageError if the input data contains non-digits
/// or there is an odd number of digits.
D2Dhcid(const std::string& data);
/// @brief Returns the DHCID value as a string of hexadecimal digits.
///
/// @return returns a string containing a contiguous stream of digits.
/// @return a string containing a contiguous stream of digits.
std::string toStr() const;
/// @brief Sets the DHCID value based on the given string.
......@@ -88,17 +88,27 @@ public:
/// a contiguous stream of digits, with no delimiters. For example a string
/// containing "14A3" converts to a byte array containing: 0x14, 0xA3.
///
/// @throw throws a NcrMessageError if the input data contains non-digits
/// @throw NcrMessageError if the input data contains non-digits
/// or there is an odd number of digits.
void fromStr(const std::string& data);
/// @brief Returns a reference to the DHCID byte vector.
///
/// @return returns a reference to the vector.
/// @return a reference to the vector.
const std::vector<uint8_t>& getBytes() {
return (bytes_);
}
/// @brief Compares two D2Dhcids for equality
bool operator==(const D2Dhcid& other) const {
return (this->bytes_ == other.bytes_);
}
/// @brief Compares two D2Dhcids for inequality
bool operator!=(const D2Dhcid other) const {
return (this->bytes_ != other.bytes_);
}
private:
/// @brief Storage for the DHCID value in unsigned bytes.
std::vector<uint8_t> bytes_;
......@@ -163,9 +173,9 @@ public:
/// @param format indicates the data format to use
/// @param buffer is the input buffer containing the marshalled request
///
/// @return returns a pointer to the new NameChangeRequest
/// @return a pointer to the new NameChangeRequest
///
/// @throw throws NcrMessageError if an error occurs creating new
/// @throw NcrMessageError if an error occurs creating new
/// request.
static NameChangeRequestPtr fromFormat(const NameChangeFormat format,
isc::util::InputBuffer& buffer);
......@@ -194,15 +204,15 @@ public:
///
/// @param json is a string containing the JSON text
///
/// @return returns a pointer to the new NameChangeRequest
/// @return a pointer to the new NameChangeRequest
///
/// @throw throws NcrMessageError if an error occurs creating new request.
/// @throw NcrMessageError if an error occurs creating new request.
static NameChangeRequestPtr fromJSON(const std::string& json);
/// @brief Instance method for marshalling the contents of the request
/// into a string of JSON text.
///
/// @return returns a string containing the JSON rendition of the request
/// @return a string containing the JSON rendition of the request
std::string toJSON() const;
/// @brief Validates the content of a populated request. This method is
......@@ -221,13 +231,13 @@ public:
/// of validation. FQDN, DHCID, and IP Address members are all currently
/// strings, these may be replaced with richer classes.
///
/// @throw throws a NcrMessageError if the request content violates any
/// @throw NcrMessageError if the request content violates any
/// of the validation rules.
void validateContent();
/// @brief Fetches the request change type.
///
/// @return returns the change type
/// @return the change type
NameChangeType getChangeType() const {
return (change_type_);
}
......@@ -241,13 +251,13 @@ public:
///
/// @param element is an integer Element containing the change type value.
///
/// @throw throws a NcrMessageError if the element is not an integer
/// @throw NcrMessageError if the element is not an integer
/// Element or contains an invalid value.
void setChangeType(isc::data::ConstElementPtr element);
/// @brief Checks forward change flag.
///
/// @return returns a true if the forward change flag is true.
/// @return a true if the forward change flag is true.
bool isForwardChange() const {
return (forward_change_);
}
......@@ -263,13 +273,13 @@ public:
/// @param element is a boolean Element containing the forward change flag
/// value.
///
/// @throw throws a NcrMessageError if the element is not a boolean
/// @throw NcrMessageError if the element is not a boolean
/// Element
void setForwardChange(isc::data::ConstElementPtr element);
/// @brief Checks reverse change flag.
///
/// @return returns a true if the reverse change flag is true.
/// @return a true if the reverse change flag is true.
bool isReverseChange() const {
return (reverse_change_);
}
......@@ -285,13 +295,13 @@ public:
/// @param element is a boolean Element containing the reverse change flag
/// value.
///
/// @throw throws a NcrMessageError if the element is not a boolean
/// @throw NcrMessageError if the element is not a boolean
/// Element
void setReverseChange(isc::data::ConstElementPtr element);
/// @brief Fetches the request FQDN
///
/// @return returns a string containing the FQDN
/// @return a string containing the FQDN
const std::string getFqdn() const {
return (fqdn_);
}
......@@ -305,13 +315,13 @@ public:
///
/// @param element is a string Element containing the FQDN
///
/// @throw throws a NcrMessageError if the element is not a string
/// @throw NcrMessageError if the element is not a string
/// Element
void setFqdn(isc::data::ConstElementPtr element);
/// @brief Fetches the request IP address.
///
/// @return returns a string containing the IP address
/// @return a string containing the IP address
const std::string& getIpAddress() const {
return (ip_address_);
}
......@@ -325,13 +335,13 @@ public:
///
/// @param element is a string Element containing the IP address
///
/// @throw throws a NcrMessageError if the element is not a string
/// @throw NcrMessageError if the element is not a string
/// Element
void setIpAddress(isc::data::ConstElementPtr element);
/// @brief Fetches the request DHCID
///
/// @return returns a reference to the request's D2Dhcid
/// @return a reference to the request's D2Dhcid
const D2Dhcid& getDhcid() const {
return (dhcid_);
}
......@@ -342,7 +352,7 @@ public:
/// a contiguous stream of digits, with no delimiters. For example a string
/// containing "14A3" converts to a byte array containing: 0x14, 0xA3.
///
/// @throw throws a NcrMessageError if the input data contains non-digits
/// @throw NcrMessageError if the input data contains non-digits
/// or there is an odd number of digits.
void setDhcid(const std::string& value);
......@@ -351,13 +361,13 @@ public:
/// @param element is a string Element containing the string of hexadecimal
/// digits. (See setDhcid(std::string&) above.)
///
/// @throw throws a NcrMessageError if the input data contains non-digits
/// @throw NcrMessageError if the input data contains non-digits
/// or there is an odd number of digits.
void setDhcid(isc::data::ConstElementPtr element);
/// @brief Fetches the request lease expiration
///
/// @return returns the lease expiration as the number of seconds since
/// @return the lease expiration as the number of seconds since
/// the (00:00:00 January 1, 1970)
uint64_t getLeaseExpiresOn() const {
return (lease_expires_on_);
......@@ -372,7 +382,7 @@ public:
/// Example: 18:54:54 June 26, 2013 would be: 20130626185455
/// NOTE This is always UTC time.
///
/// @return returns a ISO date-time string of the lease expiration.
/// @return a ISO date-time string of the lease expiration.
std::string getLeaseExpiresOnStr() const;
/// @brief Sets the lease expiration based on the given string.
......@@ -385,20 +395,20 @@ public:
/// Example: 18:54:54 June 26, 2013 would be: 20130626185455
/// NOTE This is always UTC time.
///
/// @throw throws a NcrMessageError if the ISO string is invalid.
/// @throw NcrMessageError if the ISO string is invalid.
void setLeaseExpiresOn(const std::string& value);
/// @brief Sets the lease expiration based on the given Element.
///
/// @param element is string Element containing a date-time string.
///
/// @throw throws a NcrMessageError if the element is not a string
/// @throw NcrMessageError if the element is not a string
/// Element, or if the element value is an invalid date-time string.
void setLeaseExpiresOn(isc::data::ConstElementPtr element);
/// @brief Fetches the request lease length.
///
/// @return returns an integer containing the lease length
/// @return an integer containing the lease length
uint32_t getLeaseLength() const {
return (lease_length_);
}
......@@ -412,13 +422,13 @@ public:
///
/// @param element is a integer Element containing the lease length
///
/// @throw throws a NcrMessageError if the element is not a string
/// @throw NcrMessageError if the element is not a string
/// Element
void setLeaseLength(isc::data::ConstElementPtr element);
/// @brief Fetches the request status.
///
/// @return returns the request status as a NameChangeStatus
/// @return the request status as a NameChangeStatus
NameChangeStatus getStatus() const {
return (status_);
}
......@@ -434,8 +444,8 @@ public:
/// @param name is the name of the desired element
/// @param element_map is the map of elements to search
///
/// @return returns a pointer to the element if located
/// @throw throws a NcrMessageError if the element cannot be found within
/// @return a pointer to the element if located
/// @throw NcrMessageError if the element cannot be found within
/// the map
isc::data::ConstElementPtr getElement(const std::string& name,
const ElementMap& element_map) const;
......@@ -443,9 +453,12 @@ public:
/// @brief Returns a text rendition of the contents of the request.
/// This method is primarily for logging purposes.
///
/// @return returns a string containing the text.
/// @return a string containing the text.
std::string toText() const;
bool operator == (const NameChangeRequest& b);
bool operator != (const NameChangeRequest& b);
private:
/// @brief Denotes the type of this change as either an Add or a Remove.
NameChangeType change_type_;
......
......@@ -23,9 +23,8 @@ namespace isc {
namespace d2 {
//*************************** UDPCallback ***********************
UDPCallback::UDPCallback (RawBufferPtr buffer, size_t buf_size,
UDPEndpointPtr data_source,
UDPCallback::UDPCallback (RawBufferPtr& buffer, const size_t buf_size,
UDPEndpointPtr& data_source,
const UDPCompletionHandler& handler)
: handler_(handler), data_(new Data(buffer, buf_size, data_source)) {
if (handler.empty()) {
......@@ -78,14 +77,13 @@ NameChangeUDPListener(const isc::asiolink::IOAddress& ip_address,
port_(port), format_(format), reuse_address_(reuse_address) {
// Instantiate the receive callback. This gets passed into each receive.
// Note that the callback constructor is passed an instance method
// pointer to our recv_completion_handler.
// pointer to our completion handler method, receiveCompletionHandler.
RawBufferPtr buffer(new uint8_t[RECV_BUF_MAX]);
UDPEndpointPtr data_source(new asiolink::UDPEndpoint());
recv_callback_.reset(new
UDPCallback(RawBufferPtr(new uint8_t[RECV_BUF_MAX]),
RECV_BUF_MAX,
UDPEndpointPtr(new
asiolink::UDPEndpoint()),
UDPCallback(buffer, RECV_BUF_MAX, data_source,
boost::bind(&NameChangeUDPListener::
recv_completion_handler, this, _1, _2)));
receiveCompletionHandler, this, _1, _2)));
}
NameChangeUDPListener::~NameChangeUDPListener() {
......@@ -149,8 +147,8 @@ NameChangeUDPListener::close() {
}
void
NameChangeUDPListener::recv_completion_handler(bool successful,
const UDPCallback *callback) {
NameChangeUDPListener::receiveCompletionHandler(const bool successful,
const UDPCallback *callback) {
NameChangeRequestPtr ncr;
Result result = SUCCESS;
......@@ -183,27 +181,26 @@ NameChangeUDPListener::recv_completion_handler(bool successful,
//*************************** NameChangeUDPSender ***********************
NameChangeUDPSender::NameChangeUDPSender(
const isc::asiolink::IOAddress& ip_address, const uint32_t port,
const isc::asiolink::IOAddress& server_address,
const uint32_t server_port, const NameChangeFormat format,
RequestSendHandler& ncr_send_handler, const size_t send_que_max,
const bool reuse_address)
NameChangeUDPSender::
NameChangeUDPSender(const isc::asiolink::IOAddress& ip_address,
const uint32_t port,
const isc::asiolink::IOAddress& server_address,
const uint32_t server_port, const NameChangeFormat format,
RequestSendHandler& ncr_send_handler,
const size_t send_que_max, const bool reuse_address)
: NameChangeSender(ncr_send_handler, send_que_max),
ip_address_(ip_address), port_(port), server_address_(server_address),
server_port_(server_port), format_(format),
reuse_address_(reuse_address) {
// Instantiate the send callback. This gets passed into each send.
// Note that the callback constructor is passed the an instance method
// pointer to our send_completion_handler.
send_callback_.reset(new UDPCallback(
RawBufferPtr(new uint8_t[SEND_BUF_MAX]),
SEND_BUF_MAX,
UDPEndpointPtr(new asiolink::
UDPEndpoint()),
boost::bind(&NameChangeUDPSender::
send_completion_handler, this,
_1, _2)));
// pointer to our completion handler, sendCompletionHandler.
RawBufferPtr buffer(new uint8_t[SEND_BUF_MAX]);
UDPEndpointPtr data_source(new asiolink::UDPEndpoint());
send_callback_.reset(new UDPCallback(buffer, SEND_BUF_MAX, data_source,
boost::bind(&NameChangeUDPSender::
sendCompletionHandler, this,
_1, _2)));
}
NameChangeUDPSender::~NameChangeUDPSender() {
......@@ -212,7 +209,7 @@ NameChangeUDPSender::~NameChangeUDPSender() {
}
void
NameChangeUDPSender::open(isc::asiolink::IOService & io_service) {
NameChangeUDPSender::open(isc::asiolink::IOService& io_service) {
// create our endpoint and bind the the low level socket to it.
isc::asiolink::UDPEndpoint endpoint(ip_address_.getAddress(), port_);
......@@ -264,7 +261,7 @@ NameChangeUDPSender::close() {
}
void
NameChangeUDPSender::doSend(NameChangeRequestPtr ncr) {
NameChangeUDPSender::doSend(NameChangeRequestPtr& ncr) {
// Now use the NCR to write JSON to an output buffer.
isc::util::OutputBuffer ncr_buffer(SEND_BUF_MAX);
ncr->toFormat(format_, ncr_buffer);
......@@ -280,8 +277,8 @@ NameChangeUDPSender::doSend(NameChangeRequestPtr ncr) {
}
void
NameChangeUDPSender::send_completion_handler(const bool successful,
const UDPCallback *send_callback) {
NameChangeUDPSender::sendCompletionHandler(const bool successful,
const UDPCallback *send_callback) {
Result result;
if (successful) {
result = SUCCESS;
......
......@@ -61,7 +61,7 @@
/// @endcode
///
/// Upon completion of the service, the callback instance's operator() is
/// invoked by the aiso layer. It is given both a outcome result and the
/// invoked by the asio layer. It is given both a outcome result and the
/// number of bytes either read or written, to or from the buffer supplied
/// to the service.
///
......@@ -168,7 +168,8 @@ public:
/// send.
/// @param buf_size is the capacity of the buffer
/// @param data_source storage for UDP endpoint which supplied the data
Data(RawBufferPtr buffer, size_t buf_size, UDPEndpointPtr data_source)
Data(RawBufferPtr &buffer, const size_t buf_size,
UDPEndpointPtr& data_source)
: buffer_(buffer), buf_size_(buf_size), data_source_(data_source),
put_len_(0), error_code_(), bytes_transferred_(0) {
};
......@@ -208,10 +209,10 @@ public:
/// @param handler is a method pointer to the completion handler that
/// is to be called by the operator() implementation.
///
/// @throw throws a NcrUDPError if either the handler or buffer pointers
/// @throw NcrUDPError if either the handler or buffer pointers
/// are invalid.
UDPCallback (RawBufferPtr buffer, size_t buf_size,
UDPEndpointPtr data_source,
UDPCallback (RawBufferPtr& buffer, const size_t buf_size,
UDPEndpointPtr& data_source,
const UDPCompletionHandler& handler);
/// @brief Operator that will be invoked by the asiolink layer.
......@@ -281,7 +282,7 @@ public:
/// @param src is a pointer to the data source from which to copy
/// @param len is the number of bytes to copy
///
/// @throw throws a NcrUDPError if the number of bytes to copy exceeds
/// @throw NcrUDPError if the number of bytes to copy exceeds
/// the buffer capacity or if the source pointer is invalid.
void putData(const uint8_t* src, size_t len);
......@@ -329,11 +330,11 @@ public:
/// @brief Constructor
///
/// @param ip_address is the network address on which to listen
/// @param port is the IP port on which to listen
/// @param port is the UDP port on which to listen
/// @param format is the wire format of the inbound requests. Currently
/// only JSON is supported
/// @param ncr_recv_handler the receive handler object to notify when
/// when a receive completes.
/// a receive completes.
/// @param reuse_address enables IP address sharing when true
/// It defaults to false.
///
......@@ -341,11 +342,7 @@ public:
NameChangeUDPListener(const isc::asiolink::IOAddress& ip_address,
const uint32_t port,
const NameChangeFormat format,
#if 0
const RequestReceiveHandler* ncr_recv_handler,
#else
RequestReceiveHandler& ncr_recv_handler,
#endif
const bool reuse_address = false);
/// @brief Destructor.
......@@ -358,7 +355,7 @@ public:
///
/// @param io_service the IOService which will monitor the socket.
///
/// @throw throws a NcrUDPError if the open fails.
/// @throw NcrUDPError if the open fails.
virtual void open(isc::asiolink::IOService& io_service);
/// @brief Closes the UDPSocket.
......@@ -367,7 +364,7 @@ public:
/// pending read and remove the socket callback from the IOService. It
/// then calls the socket's close method to actually close the socket.
///
/// @throw throws a NcrUDPError if the open fails.
/// @throw NcrUDPError if the open fails.
virtual void close();
/// @brief Initiates an asynchronous read on the socket.
......@@ -376,7 +373,7 @@ public:
/// recv_callback_ member's transfer buffer as the receive buffer, and
/// recv_callback_ itself as the callback object.
///
/// @throw throws a NcrUDPError if the open fails.
/// @throw NcrUDPError if the open fails.
void doReceive();
/// @brief Implements the NameChangeRequest level receive completion
......@@ -403,8 +400,8 @@ public:
/// socket receive completed without error, false otherwise.
/// @param recv_callback pointer to the callback instance which handled
/// the socket receive completion.
void recv_completion_handler(bool successful,
const UDPCallback* recv_callback);
void receiveCompletionHandler(const bool successful,
const UDPCallback* recv_callback);
private:
/// @brief IP address on which to listen for requests.
isc::asiolink::IOAddress ip_address_;
......@@ -486,7 +483,7 @@ public:
///
/// @param io_service the IOService which will monitor the socket.
///
/// @throw throws a NcrUDPError if the open fails.
/// @throw NcrUDPError if the open fails.
virtual void open(isc::asiolink::IOService & io_service);
......@@ -496,7 +493,7 @@ public:
/// pending send and remove the socket callback from the IOService. It
/// then calls the socket's close method to actually close the socket.
///
/// @throw throws a NcrUDPError if the open fails.
/// @throw NcrUDPError if the open fails.
virtual void close();
/// @brief Sends a given request asynchronously over the socket
......@@ -506,7 +503,7 @@ public:
/// asyncSend() method is called, passing in send_callback_ member's
/// transfer buffer as the send buffer and the send_callback_ itself
/// as the callback object.
virtual void doSend(NameChangeRequestPtr ncr);
virtual void doSend(NameChangeRequestPtr& ncr);
/// @brief Implements the NameChangeRequest level send completion handler.
///
......@@ -525,8 +522,8 @@ public:
/// socket send completed without error, false otherwise.
/// @param send_callback pointer to the callback instance which handled
/// the socket receive completion.
void send_completion_handler(const bool successful,
const UDPCallback* send_callback);
void sendCompletionHandler(const bool successful,
const UDPCallback* send_callback);
private:
/// @brief IP address from which to send.
......
......@@ -131,12 +131,10 @@ TEST(NameChangeUDPListenerBasicTest, basicListenTests) {
}
/// @brief Compares two NameChangeRequests for equality.
bool checkSendVsReceived(NameChangeRequestPtr sent_ncr_,
NameChangeRequestPtr received_ncr_) {
// @todo NameChangeRequest message doesn't currently have a comparison
// operator, so we will cheat and compare the text form.
return ((sent_ncr_ && received_ncr_ ) &&
((sent_ncr_->toText()) == (received_ncr_->toText())));
bool checkSendVsReceived(NameChangeRequestPtr sent_ncr,
NameChangeRequestPtr received_ncr) {
return ((sent_ncr && received_ncr) &&
(*sent_ncr == *received_ncr));
}
/// @brief Text fixture for testing NameChangeUDPListener
......@@ -147,7 +145,7 @@ public:
NameChangeListener::Result result_;
NameChangeRequestPtr sent_ncr_;
NameChangeRequestPtr received_ncr_;
NameChangeUDPListener *listener_;
NameChangeListenerPtr listener_;
isc::asiolink::IntervalTimer test_timer_;
/// @brief Constructor
......@@ -158,8 +156,8 @@ public:
: io_service_(), result_(NameChangeListener::SUCCESS),
test_timer_(io_service_) {
isc::asiolink::IOAddress addr(TEST_ADDRESS);
listener_ = new NameChangeUDPListener(addr, LISTENER_PORT,
FMT_JSON, *this, true);
listener_.reset(new NameChangeUDPListener(addr, LISTENER_PORT,
FMT_JSON, *this, true));
// Set the test timeout to break any running tasks if they hang.
test_timer_.setup(boost::bind(&NameChangeUDPListenerTest::
......@@ -167,6 +165,10 @@ public:
TEST_TIMEOUT);
}
virtual ~NameChangeUDPListenerTest(){
}
/// @brief Converts JSON string into an NCR and sends it to the listener.
///
void sendNcr(const std::string& msg) {
......@@ -221,7 +223,7 @@ public:
/// NCRs and delivery them to the "application" layer.
TEST_F(NameChangeUDPListenerTest, basicReceivetest) {
// Verify we can enter listening mode.
EXPECT_FALSE(listener_->amListening());
ASSERT_FALSE(listener_->amListening());
ASSERT_NO_THROW(listener_->startListening(io_service_));
ASSERT_TRUE(listener_->amListening());
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment