[Rprotobuf-commits] r511 - in pkg: . R inst/unitTests

[noreply at r-forge.r-project.org]

Author: murray
Date: 2013-08-22 08:13:45 +0200 (Thu, 22 Aug 2013)
New Revision: 511

Modified:
   pkg/ChangeLog
   pkg/R/extensions.R
   pkg/inst/unitTests/runit.extensions.R
Log:
Add checks to ensure that a valid extension FieldDescriptor is used with
getExtension and setExtension and add more tests.



Modified: pkg/ChangeLog
===================================================================
--- pkg/ChangeLog	2013-08-22 03:30:26 UTC (rev 510)
+++ pkg/ChangeLog	2013-08-22 06:13:45 UTC (rev 511)
@@ -1,10 +1,12 @@
 2013-08-21  Murray Stokely  <murray at FreeBSD.org>
 
+	* R/extensions.R: Add checks to ensure that a valid extension
+	  FieldDescriptor is passed to getExtension and setExtension.
 	* inst/unitTests/runit.int64.R (test.int64): Add a test
 	  illustrating how RProtoBuf is broken in handling 64-bit integers
 	  (commented out for now).
 	* man/P.Rd: Document behavior for extensions.
-	* inst/unitTests/runit.extensions.R (test.extension): Add another test.
+	* inst/unitTests/runit.extensions.R (test.extension): Add more tests.
 	* src/wrapper_Message.cpp (rprotobuf): Add function for returning
 	  the number of extensions set in this message, to improve show() output.
 	* src/rprotobuf.cpp (rprotobuf): Add support for looking up

Modified: pkg/R/extensions.R
===================================================================
--- pkg/R/extensions.R	2013-08-22 03:30:26 UTC (rev 510)
+++ pkg/R/extensions.R	2013-08-22 06:13:45 UTC (rev 511)
@@ -21,10 +21,14 @@
 } )
 
 setMethod( "setExtension", "Message", function( object, field, values ){
-	stopifnot(is_extension(field))
-
-        .Call( "setMessageField", object at pointer, field, values,
-              PACKAGE = "RProtoBuf" )
+	if (!inherits(field, "FieldDescriptor")) {
+		stop("setExtension requires a FieldDescriptor")
+	}
+	if (!is_extension(field)) {
+		stop(paste(name(field), "is not an extension FieldDescriptor."))
+	}
+	.Call( "setMessageField", object at pointer, field, values,
+	      PACKAGE = "RProtoBuf" )
 	invisible( object )
 } )
 
@@ -33,6 +37,12 @@
 	standardGeneric( "getExtension" )
 } )
 setMethod( "getExtension", "Message", function( object, field){
+	if (!inherits(field, "FieldDescriptor")) {
+		stop("getExtension requires a FieldDescriptor")
+	}
+	if (!is_extension(field)) {
+		stop(paste(name(field), "is not an extension FieldDescriptor."))
+	}
         .Call( "getExtension", object at pointer, field,
               PACKAGE = "RProtoBuf" )
 } )

Modified: pkg/inst/unitTests/runit.extensions.R
===================================================================
--- pkg/inst/unitTests/runit.extensions.R	2013-08-22 03:30:26 UTC (rev 510)
+++ pkg/inst/unitTests/runit.extensions.R	2013-08-22 06:13:45 UTC (rev 511)
@@ -26,6 +26,10 @@
     checkTrue(inherits(protobuf_unittest.optional_uint32_extension,
                        "FieldDescriptor"))
 
+    # Verify we can pull in other extensions with P()
+    checkTrue(inherits(P("protobuf_unittest.optional_uint32_extension"),
+                       "FieldDescriptor"))
+
     ## Test setting and getting singular extensions.
     test <- new(protobuf_unittest.TestAllExtensions)
     test$setExtension(protobuf_unittest.optional_int32_extension,
@@ -45,6 +49,8 @@
                 1:10)
 
     ## Test nested extensions.
+    checkEquals(test$getExtension(protobuf_unittest.TestNestedExtension.test),
+                NULL)
     test$setExtension(protobuf_unittest.TestNestedExtension.test, "foo")
     checkEquals(test$getExtension(protobuf_unittest.TestNestedExtension.test),
                                   "foo")
@@ -54,22 +60,19 @@
     test$setExtension(protobuf_unittest.optional_nested_enum_extension,
                       protobuf_unittest.TestAllTypes.NestedEnum$BAR)
 
-    # Test that we get an error printed to terminal (not a real stop error)
-    # not a crash for invalid enum:
-    # TODO(mstokely): Make this a stop() error.
+    # This causes an Rcpp exception, but not an R stop error as of my
+    # version of Rcpp, so we can't checkError unfortunately, but we
+    # can at least make sure it doesn't crash R.
     # TODO(edd): Commented out now
     # test$setExtension(protobuf_unittest.optional_nested_enum_extension, 9)
 
-    ## Test nested extensions
-    checkEquals(test$getExtension(protobuf_unittest.TestNestedExtension.test),
-                NULL)
-    test$setExtension(protobuf_unittest.TestNestedExtension.test, "Hello World")
-    checkEquals(test$getExtension(protobuf_unittest.TestNestedExtension.test),
-                "Hello World")
-
     ## Test nested message extensions.
     tmp <- new( protobuf_unittest.TestAllTypes.NestedMessage )
     tmp$bb <- 3
     test$setExtension(protobuf_unittest.optional_nested_message_extension, tmp)
     checkEquals(test$getExtension(protobuf_unittest.optional_nested_message_extension)$bb, 3)
+
+    ## Check that we do something sensible if invalid field descriptors are passed
+    checkException(test$getExtension(protobuf_unittest.TestAllExtensions))
+    checkException(test$setExtension(protobuf_unittest.TestAllExtensions, 3))
 }