From 3d394c55822f1f99074d3360caf931e16dfae77b Mon Sep 17 00:00:00 2001
From: Wayne Tucker <wayne@tuckerlabs.com>
Date: Sun, 18 Feb 2018 16:29:39 -0500
Subject: [PATCH] Move RPCReply construction into rpc and add basic tests (#50)

* Move RPCReply construction into rpc and add basic tests
* Hide newRPCReply and get rid of RaiseErrors method
* Fixes to make Travis CI happy
---
 netconf/rpc.go      | 19 +++++++++++
 netconf/rpc_test.go | 83 +++++++++++++++++++++++++++++++++++++++++++++
 netconf/session.go  | 16 ++-------
 3 files changed, 104 insertions(+), 14 deletions(-)

diff --git a/netconf/rpc.go b/netconf/rpc.go
index 679d9b1..0e069c2 100644
--- a/netconf/rpc.go
+++ b/netconf/rpc.go
@@ -53,6 +53,25 @@ type RPCReply struct {
 	RawReply string     `xml:"-"`
 }
 
+func newRPCReply(rawXML []byte, ErrOnWarning bool) (*RPCReply, error) {
+	reply := &RPCReply{}
+	reply.RawReply = string(rawXML)
+
+	if err := xml.Unmarshal(rawXML, reply); err != nil {
+		return nil, err
+	}
+
+	if reply.Errors != nil {
+		for _, rpcErr := range reply.Errors {
+			if rpcErr.Severity == "error" || ErrOnWarning {
+				return reply, &rpcErr
+			}
+		}
+	}
+
+	return reply, nil
+}
+
 // RPCError defines an error reply to a RPC request
 type RPCError struct {
 	Type     string `xml:"error-type"`
diff --git a/netconf/rpc_test.go b/netconf/rpc_test.go
index ca3ae5b..c8fc370 100644
--- a/netconf/rpc_test.go
+++ b/netconf/rpc_test.go
@@ -89,6 +89,89 @@ func TestUUIDChar(t *testing.T) {
 	for _, v := range u {
 		if valid(int(v)) == false {
 			t.Errorf("invalid char %s", string(v))
+
+		}
+	}
+}
+
+var RPCReplytests = []struct {
+	rawXML  string
+	replyOk bool
+}{
+	{
+		`
+<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" xmlns:junos="http://xml.juniper.net/junos/15.1F4/junos">
+<commit-results>
+</commit-results>
+<ok/>
+</rpc-reply>`,
+		false,
+	},
+	{
+		`
+<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" xmlns:junos="http://xml.juniper.net/junos/15.1F4/junos">
+<commit-results>
+<rpc-error>
+<error-type>application</error-type>
+<error-tag>invalid-value</error-tag>
+<error-severity>error</error-severity>
+<error-path>[edit]</error-path>
+<error-message>mgd: Missing mandatory statement: 'root-authentication'</error-message>
+<error-info>
+<bad-element>system</bad-element>
+</error-info>
+</rpc-error>
+<rpc-error>
+<error-type>protocol</error-type>
+<error-tag>operation-failed</error-tag>
+<error-severity>error</error-severity>
+<error-message>
+configuration check-out failed: (missing mandatory statements)
+</error-message>
+</rpc-error>
+</commit-results>
+</rpc-reply>`,
+		false,
+	},
+	{
+		`
+<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" xmlns:junos="http://xml.juniper.net/junos/16.1R3/junos">
+<commit-results>
+<rpc-error>
+<error-severity>warning</error-severity>
+<error-path>[edit protocols]</error-path>
+<error-message>mgd: requires 'mpls' license</error-message>
+<error-info>
+<bad-element>mpls</bad-element>
+</error-info>
+</rpc-error>
+<rpc-error>
+<error-severity>warning</error-severity>
+<error-path>[edit protocols]</error-path>
+<error-message>mgd: requires 'bgp' license</error-message>
+<error-info>
+<bad-element>bgp</bad-element>
+</error-info>
+</rpc-error>
+<routing-engine junos:style="normal">
+<name>fpc0</name>
+<commit-check-success/>
+</routing-engine>
+</commit-results>
+<ok/>
+</rpc-reply>`,
+		false,
+	},
+}
+
+func TestNewRPCReply(t *testing.T) {
+	for _, tc := range RPCReplytests {
+		reply, err := newRPCReply([]byte(tc.rawXML), false)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if reply.RawReply != tc.rawXML {
+			t.Errorf("newRPCReply(%q) did not set RawReply to input, got %q", tc.rawXML, reply.RawReply)
 		}
 	}
 }
diff --git a/netconf/session.go b/netconf/session.go
index e8f0d69..f5509cf 100644
--- a/netconf/session.go
+++ b/netconf/session.go
@@ -39,23 +39,11 @@ func (s *Session) Exec(methods ...RPCMethod) (*RPCReply, error) {
 		return nil, err
 	}
 
-	reply := &RPCReply{}
-	reply.RawReply = string(rawXML)
-
-	if err := xml.Unmarshal(rawXML, reply); err != nil {
+	reply, err := newRPCReply(rawXML, s.ErrOnWarning)
+	if err != nil {
 		return nil, err
 	}
 
-	if reply.Errors != nil {
-		// We have errors, lets see if it's a warning or an error.
-		for _, rpcErr := range reply.Errors {
-			if rpcErr.Severity == "error" || s.ErrOnWarning {
-				return reply, &rpcErr
-			}
-		}
-
-	}
-
 	return reply, nil
 }