More translate browse paths work

einarmo · Jan 28, 2019 · 14f44aa · 14f44aa
1 parent 4cf6f8e
commit 14f44aa
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 44 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -1,4 +1,5 @@
 // Place your settings in this file to overwrite default and user settings.
 {
-    "debug.allowBreakpointsEverywhere": true
+    "debug.allowBreakpointsEverywhere": true,
+    "git.ignoreLimitWarning": true
 }
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -3,11 +3,11 @@ lto = true
 
 [workspace]
 members = [
+    "console-logging",
     "types",
     "core",
     "client",
     "server",
-    "console-logging",
     "integration",
     "samples/demo-server",
     "samples/simple-client",

diff --git a/server/Cargo.toml b/server/Cargo.toml
@@ -46,3 +46,7 @@ version = "0.6.0" # OPCUARustVersion
 [dependencies.opcua-client]
 path = "../client"
 version = "0.6.0" # OPCUARustVersion
+
+[dev-dependencies.opcua-console-logging]
+path = "../console-logging"
+version = "0.6.0" # OPCUARustVersion
diff --git a/server/src/address_space/relative_path.rs b/server/src/address_space/relative_path.rs
@@ -10,37 +10,50 @@ use crate::{
 
 /// Given a `RelativePath`, find all the nodes that match against it.
 pub(crate) fn find_nodes_relative_path(address_space: &AddressSpace, node_id: &NodeId, relative_path: &RelativePath) -> Result<Vec<NodeId>, StatusCode> {
-    // TODO THIS CODE IS PROBABLY BROKEN - need test examples for TranslateBrowsePathToNodeIds
-    if address_space.find_node(node_id).is_none() {
-        Err(StatusCode::BadNodeIdUnknown)
-    } else {
-        let elements = relative_path.elements.as_ref().unwrap();
-        if elements.is_empty() {
-            Err(StatusCode::BadNothingToDo)
-        } else {
-            let mut matching_nodes = vec![node_id.clone()];
-            let mut next_matching_nodes = Vec::with_capacity(100);
+    match address_space.find_node(node_id) {
+        None => {
+            trace!("find_nodes_relative_path cannot find node {:?}", node_id);
+            Err(StatusCode::BadNodeIdUnknown)
+        }
+        Some(_) => {
+            let elements = relative_path.elements.as_ref().unwrap();
+            if elements.is_empty() {
+                warn!("find_nodes_relative_path elements are empty");
+                Err(StatusCode::BadNothingToDo)
+            } else {
+                let mut matching_nodes = vec![node_id.clone()];
+                let mut next_matching_nodes = Vec::with_capacity(100);
 
-            // Traverse the relative path elements
-            for relative_path_element in elements.iter() {
-                next_matching_nodes.clear();
+                // Traverse the relative path elements. Each time around, we will find the matching
+                // elements at that level using the next element
+                for element in elements.iter() {
+                    if element.target_name.is_null() {
+                        warn!("find_nodes_relative_path browse name is invalid (null)");
+                        return Err(StatusCode::BadBrowseNameInvalid);
+                    }
 
-                if matching_nodes.is_empty() {
-                    break;
-                }
+                    next_matching_nodes.clear();
 
-                for node_id in &matching_nodes {
-                    // Iterate current set of nodes and put the results into next
-                    if let Some(mut result) = follow_relative_path(address_space, &node_id, relative_path_element) {
-                        next_matching_nodes.append(&mut result);
+                    matching_nodes.drain(..).for_each(|node_id| {
+                        // Iterate current set of nodes and put the results into next
+                        if let Some(mut result) = follow_relative_path(address_space, &node_id, element) {
+                            next_matching_nodes.append(&mut result);
+                        }
+                    });
+                    if next_matching_nodes.is_empty() {
+                        break;
+                    } else {
+                        matching_nodes.append(&mut next_matching_nodes);
                     }
                 }
 
-                matching_nodes.clear();
-                matching_nodes.append(&mut next_matching_nodes);
+                if matching_nodes.is_empty() {
+                    warn!("find_nodes_relative_path bad no match");
+                    Err(StatusCode::BadNoMatch)
+                } else {
+                    Ok(matching_nodes)
+                }
             }
-
-            Ok(matching_nodes)
         }
     }
 }
@@ -68,4 +81,4 @@ fn follow_relative_path(address_space: &AddressSpace, node_id: &NodeId, relative
     } else {
         None
     }
-}
+}
diff --git a/server/src/services/view.rs b/server/src/services/view.rs
@@ -95,11 +95,14 @@ impl ViewService {
 
         if let Some(ref browse_paths) = request.browse_paths {
             if browse_paths.is_empty() {
+                trace!("Browse paths is empty");
                 Ok(self.service_fault(&request.request_header, StatusCode::BadNothingToDo))
             } else if max_nodes_per_operation > 0 && browse_paths.len() > max_nodes_per_operation {
+                trace!("Browse paths size {} exceeds max nodes {}", browse_paths.len(), max_nodes_per_operation);
                 Ok(self.service_fault(&request.request_header, StatusCode::BadTooManyOperations))
             } else {
-                let results = browse_paths.iter().map(|browse_path| {
+                let results = browse_paths.iter().enumerate().map(|(i, browse_path)| {
+                    trace!("Processing browse path {}", i);
                     let node_id = browse_path.starting_node.clone();
                     if browse_path.relative_path.elements.is_none() {
                         BrowsePathResult {
@@ -110,6 +113,7 @@ impl ViewService {
                         // Starting from the node_id, find paths
                         match relative_path::find_nodes_relative_path(address_space, &node_id, &browse_path.relative_path) {
                             Err(err) => {
+                                trace!("Browse path result for find nodes returned in error {}", err.name());
                                 BrowsePathResult {
                                     status_code: err,
                                     targets: None,
@@ -153,12 +157,10 @@ impl ViewService {
 
     fn browse_nodes(session: &mut Session, address_space: &AddressSpace, nodes_to_browse: &[BrowseDescription], max_references_per_node: usize) -> Vec<BrowseResult> {
         nodes_to_browse.iter().map(|node_to_browse| {
-            let browse_result = Self::browse_node(session, &address_space, 0, node_to_browse, max_references_per_node);
-            if let Ok(browse_result) = browse_result {
-                browse_result
-            } else {
-                BrowseResult {
-                    status_code: browse_result.unwrap_err(),
+            match Self::browse_node(session, &address_space, 0, node_to_browse, max_references_per_node) {
+                Ok(browse_result) => browse_result,
+                Err(status_code) => BrowseResult {
+                    status_code,
                     continuation_point: ByteString::null(),
                     references: None,
                 }

diff --git a/server/src/tests/services/view.rs b/server/src/tests/services/view.rs
@@ -1,3 +1,5 @@
+use opcua_console_logging;
+
 use crate::prelude::*;
 use crate::services::view::ViewService;
 
@@ -251,16 +253,17 @@ fn translate_browse_paths_to_node_ids() {
     let results = response.results.unwrap();
     assert_eq!(results.len(), 1);
     let r1 = &results[0];
-/* TODO
-    let targets = r1.targets.as_ref().unwrap();
-    assert_eq!(targets.len(), 1);
-    let t1 = &targets[0];
-    assert_eq!(&t1.target_id.node_id, &AddressSpace::objects_folder_id());
-    */
+    /* TODO
+        let targets = r1.targets.as_ref().unwrap();
+        assert_eq!(targets.len(), 1);
+        let t1 = &targets[0];
+        assert_eq!(&t1.target_id.node_id, &AddressSpace::objects_folder_id());
+        */
 }
 
 #[test]
 fn translate_browse_paths_to_node_ids2() {
+    opcua_console_logging::init();
 
     // Inputs and outputs taken from this testcase in Node OPCUA
     //
@@ -277,10 +280,11 @@ fn translate_browse_paths_to_node_ids2() {
         "/Objects/Server.ServerStatus.BuildInfo.",
         "/Objects.Server",
         "/Objects/2:MatrikonOPC Simulation Server (DA)",
+        // TODO tests that test inverse and no subtypes
     ].iter().map(|path|
         BrowsePath {
             starting_node: starting_node.clone(),
-            relative_path: RelativePath::from_str(path, &RelativePathElement::default_node_resolver).unwrap()
+            relative_path: RelativePath::from_str(path, &RelativePathElement::default_node_resolver).unwrap(),
         }
     ).collect::<Vec<_>>();
 
@@ -300,27 +304,31 @@ fn translate_browse_paths_to_node_ids2() {
     let results = response.results.unwrap();
     assert_eq!(results.len(), browse_paths_len);
 
+    let mut idx = 0;
+
     // results[0]
     {
-        let r = &results[0];
+        let r = &results[idx];
         assert!(r.status_code.is_good());
         let targets = r.targets.as_ref().unwrap();
         assert_eq!(targets.len(), 1);
         assert_eq!(&targets[0].target_id, &ObjectId::Server.into());
+        idx += 1;
     }
 
     // results[1]
     {
-        let r = &results[1];
+        let r = &results[idx];
         assert!(r.status_code.is_good());
         let targets = r.targets.as_ref().unwrap();
 //        assert_eq!(targets.len(), 1);
 //        assert_eq!(&targets[0].target_id, &VariableId::Server_ServerStatus.into());
+        idx += 1;
     }
 
     // results[2]
     {
-        let r = &results[2];
+        let r = &results[idx];
         assert!(r.status_code.is_good());
         let targets = r.targets.as_ref().unwrap();
 //        assert_eq!(targets.len(), 1);
@@ -329,6 +337,7 @@ fn translate_browse_paths_to_node_ids2() {
         // results[2].targets.length.should.eql(1);
         // results[2].targets[0].targetId.toString().should.eql("ns=0;i=2260");
         // results[2].targets[0].targetId.value.should.eql(opcua.VariableIds.Server_ServerStatus_BuildInfo);
+        idx += 1;
     }
 
     // results[3]
@@ -337,20 +346,24 @@ fn translate_browse_paths_to_node_ids2() {
         // results[3].targets.length.should.eql(1);
         // results[3].targets[0].targetId.toString().should.eql("ns=0;i=2261");
         // results[3].targets[0].targetId.value.should.eql(opcua.VariableIds.Server_ServerStatus_BuildInfo_ProductName);
+        idx += 1;
     }
 
     // results[4]
     {
         // results[4].statusCode.should.eql(StatusCodes.BadBrowseNameInvalid);
+        idx += 1;
     }
 
     // results[5]
     {
         // results[5].statusCode.should.eql(StatusCodes.BadNoMatch);
+        idx += 1;
     }
 
     // results[6]
     {
         // results[6].statusCode.should.eql(StatusCodes.BadNoMatch);
+        idx += 1;
     }
 }