diff --git a/NAMESPACE b/NAMESPACE index 8de0d89..6f2229b 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -78,7 +78,7 @@ exportMethods( "new", "[[", "[[<-", "$", "$<-", "show", ) -export( "P", "readProtoFiles", "readProtoFiles2", "asMessage" ) +export( "P", "readProtoFiles", "readProtoFiles2", "resetDescriptorPool", "asMessage" ) if( exists( ".DollarNames", asNamespace("utils") ) ) importFrom( utils, .DollarNames ) S3method(.DollarNames, Message ) diff --git a/R/internals.R b/R/internals.R index 0156949..ef4f063 100644 --- a/R/internals.R +++ b/R/internals.R @@ -101,6 +101,12 @@ readProtoFiles2 <- function(files, invisible(NULL) } +resetDescriptorPool <- function(){ + .Call( "resetDescriptorPool_cpp", PACKAGE = "RProtoBuf" ) + readProtoFiles2( protoPath=system.file( "proto", package="RProtoBuf", lib.loc=.RProtoBuf_libname) ) + invisible(NULL) +} + getProtobufLibVersion <- function( format = FALSE ){ version <- .Call( "get_protobuf_library_version", PACKAGE = "RProtoBuf" ) diff --git a/R/zzz.R b/R/zzz.R index 9db382f..2f7e1c7 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -3,11 +3,12 @@ ##minversion <- packageDescription(pkgname, lib.loc=libname)$MinimumLibProtoVersion ##minversion <- as.integer( gsub( "[[:space:]]+", "", minversion ) ) ##.Call( "check_libprotobuf_version", minversion, PACKAGE = "RProtoBuf" ) - readProtoFiles( package=pkgname, lib.loc=libname ) + readProtoFiles2( protoPath=system.file( "proto", package=pkgname, lib.loc=libname ) ) attachDescriptorPool( pos = length(search()) ) options("RProtoBuf.int64AsString" = FALSE) #if( exists( ".httpd.handlers.env", asNamespace( "tools" ) ) ){ # e <- tools:::.httpd.handlers.env # e[["RProtoBuf"]] <- RProtoBuf.http.handler #} + .RProtoBuf_libname <<- libname } diff --git a/inst/unitTests/data/cyclical/proj1/proto/a.proto b/inst/unitTests/data/cyclical/proj1/proto/a.proto new file mode 100644 index 0000000..34ca537 --- /dev/null +++ b/inst/unitTests/data/cyclical/proj1/proto/a.proto @@ -0,0 +1,3 @@ +syntax = "proto2"; + +message A {} diff --git a/inst/unitTests/data/cyclical/proj1/proto/c.proto b/inst/unitTests/data/cyclical/proj1/proto/c.proto new file mode 100644 index 0000000..10fa4ad --- /dev/null +++ b/inst/unitTests/data/cyclical/proj1/proto/c.proto @@ -0,0 +1,7 @@ +syntax = "proto2"; + +import "proto/b.proto"; + +message C { + optional B b = 1; +} diff --git a/inst/unitTests/data/cyclical/proj2/proto/b.proto b/inst/unitTests/data/cyclical/proj2/proto/b.proto new file mode 100644 index 0000000..db9a484 --- /dev/null +++ b/inst/unitTests/data/cyclical/proj2/proto/b.proto @@ -0,0 +1,7 @@ +syntax = "proto2"; + +import "proto/a.proto"; + +message B { + optional A a = 1; +} diff --git a/inst/unitTests/data/recursive/subdir/subdir_message.proto b/inst/unitTests/data/recursive/subdir/subdir_message.proto new file mode 100644 index 0000000..402885c --- /dev/null +++ b/inst/unitTests/data/recursive/subdir/subdir_message.proto @@ -0,0 +1,9 @@ +syntax = "proto2"; + +package protobuf_unittest_recursive_subdir; + +import "unittest_import.proto"; + +message SubdirMessage { + optional protobuf_unittest_import.ImportMessage msg = 1; +} diff --git a/inst/unitTests/data/subdir/subdir_message.proto b/inst/unitTests/data/subdir/subdir_message.proto new file mode 100644 index 0000000..f0e80ec --- /dev/null +++ b/inst/unitTests/data/subdir/subdir_message.proto @@ -0,0 +1,9 @@ +syntax = "proto2"; + +package protobuf_unittest_subdir; + +import "unittest_import.proto"; + +message SubdirMessage { + optional protobuf_unittest_import.ImportMessage msg = 1; +} diff --git a/inst/unitTests/data/unittest.proto b/inst/unitTests/data/unittest.proto index a2da0cd..3da1673 100644 --- a/inst/unitTests/data/unittest.proto +++ b/inst/unitTests/data/unittest.proto @@ -42,6 +42,9 @@ syntax = "proto2"; // the extra file from the directory in which this file is import "unittest_import.proto"; +// To test if we can import a message in another directory. +import "subdir/subdir_message.proto"; + // We don't put this in a package within proto2 because we need to make sure // that the generated code doesn't depend on being in the proto2 namespace. // In test_util.h we do "using namespace unittest = protobuf_unittest". diff --git a/inst/unitTests/runit.import.R b/inst/unitTests/runit.import.R index 85c20b6..2f00006 100644 --- a/inst/unitTests/runit.import.R +++ b/inst/unitTests/runit.import.R @@ -14,11 +14,63 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -test.import <- function() { +.setUp <- function() { + resetDescriptorPool() +} + +test.import.reset <- function() { + ## Verify that the common setup is resetting the descriptor pool. + ## Only the messages defined in the proto directory should be present. + checkTrue(all(grepl("^(rexp|tutorial|rprotobuf)\\.", objects("RProtoBuf:DescriptorPool")))) +} + +test.import.error <- function() { ## Verify that we get a graceful error rather than a segfault. checkException(readProtoFiles(system.file("DESCRIPTION", package="RProtoBuf"))) } +test.import <- function() { + unit.test.data.dir <- system.file("unitTests", "data", package="RProtoBuf") + checkTrue(!exists("protobuf_unittest_import.ImportMessage", "RProtoBuf:DescriptorPool")) + + readProtoFiles2(recursive = FALSE, protoPath=unit.test.data.dir) + checkTrue(exists("protobuf_unittest_import.ImportMessage", "RProtoBuf:DescriptorPool")) + # The following is imported by unittest.proto and hence, should have been imported implicitly. + checkTrue(exists("protobuf_unittest_subdir.SubdirMessage", "RProtoBuf:DescriptorPool")) + # The following is not imported by any of the top level files. + checkTrue(!exists("protobuf_unittest_recursive_subdir.SubdirMessage", "RProtoBuf:DescriptorPool")) +} + +test.import.subdir <- function() { + unit.test.data.dir <- system.file("unitTests", "data", package="RProtoBuf") + + # Any of these two will work. + readProtoFiles2(file="subdir/subdir_message.proto", protoPath=unit.test.data.dir) + readProtoFiles2(dir="subdir", protoPath=unit.test.data.dir) + checkTrue(exists("protobuf_unittest_subdir.SubdirMessage", "RProtoBuf:DescriptorPool")) + + readProtoFiles2(dir="recursive", recursive=TRUE, protoPath=unit.test.data.dir) + checkTrue(exists("protobuf_unittest_recursive_subdir.SubdirMessage", "RProtoBuf:DescriptorPool")) +} + +test.import.cyclical <- function() { + unit.test.cyclical.dir <- system.file("unitTests", "data", "cyclical", package="RProtoBuf") + proj1.dir <- file.path(unit.test.cyclical.dir, "proj1") + proj2.dir <- file.path(unit.test.cyclical.dir, "proj2") + + # The following files should be loaded in order as C depends on B depends on A. + + readProtoFiles2(file="proto/a.proto", protoPath=proj1.dir) + checkTrue(exists("A", "RProtoBuf:DescriptorPool")) + checkTrue(!exists("C", "RProtoBuf:DescriptorPool")) + + readProtoFiles2(file="proto/b.proto", protoPath=proj2.dir) + checkTrue(exists("B", "RProtoBuf:DescriptorPool")) + + readProtoFiles2(file="proto/c.proto", protoPath=proj1.dir) + checkTrue(exists("C", "RProtoBuf:DescriptorPool")) +} + test.assign.in.global <- function() { xx.undef <<- 3 checkEquals(xx.undef, 3) diff --git a/man/readProtoFiles.Rd b/man/readProtoFiles.Rd index cebb855..29ca61a 100644 --- a/man/readProtoFiles.Rd +++ b/man/readProtoFiles.Rd @@ -1,6 +1,7 @@ \name{readProtoFiles} \alias{readProtoFiles} \alias{readProtoFiles2} +\alias{resetDescriptorPool} \title{protocol buffer descriptor importer} \description{ Imports proto files into the descriptor pool that @@ -10,6 +11,7 @@ \usage{ readProtoFiles(files, dir, package="RProtoBuf", pattern="\\\\.proto$", lib.loc=NULL) readProtoFiles2(files, dir=".", pattern="\\\\.proto$", recursive=FALSE, protoPath=getwd()) + resetDescriptorPool() } \arguments{ \item{files}{Proto files} @@ -32,6 +34,8 @@ interpreted relative to \code{protoPath}, so that there is consistency in future imports of the same files through import statements of other proto files. + + \code{resetDescriptorPool} clears all imported proto definitions. } \value{\code{NULL}, invisibly.} \author{Romain Francois } diff --git a/src/DescriptorPoolLookup.cpp b/src/DescriptorPoolLookup.cpp index c279841..64833a7 100644 --- a/src/DescriptorPoolLookup.cpp +++ b/src/DescriptorPoolLookup.cpp @@ -34,8 +34,9 @@ SEXP DescriptorPoolLookup::getElements() { return Rcpp::wrap(elements); } std::set DescriptorPoolLookup::elements; RWarningErrorCollector DescriptorPoolLookup::error_collector; RSourceTree DescriptorPoolLookup::source_tree; -GPB::compiler::Importer DescriptorPoolLookup::importer(&source_tree, &error_collector); -GPB::DynamicMessageFactory DescriptorPoolLookup::message_factory(importer.pool()); +GPB::compiler::Importer* DescriptorPoolLookup::importer = + new GPB::compiler::Importer(&source_tree, &error_collector); +GPB::DynamicMessageFactory DescriptorPoolLookup::message_factory(importer->pool()); /** * Add descriptors from a proto file to the descriptor pool. @@ -51,7 +52,7 @@ void DescriptorPoolLookup::importProtoFiles(SEXP files, SEXP dirs) { source_tree.addDirectories(dirs); int n = LENGTH(files); for (int j = 0; j < n; j++) { - const GPB::FileDescriptor* file_desc = importer.Import(CHAR(STRING_ELT(files, j))); + const GPB::FileDescriptor* file_desc = importer->Import(CHAR(STRING_ELT(files, j))); if (!file_desc) { std::string message = std::string("Could not load proto file '") + CHAR(STRING_ELT(files, j)) + "'\n"; @@ -80,7 +81,18 @@ void DescriptorPoolLookup::importProtoFiles(SEXP files, SEXP dirs) { // source_tree.removeDirectories( dirs ) ; } -const GPB::DescriptorPool* DescriptorPoolLookup::pool() { return importer.pool(); } +/** + * Clears any persisted state for the descriptor pool. + */ +void DescriptorPoolLookup::reset() { + source_tree.removeAllDirectories(); + elements.clear(); + // TODO: Find out why deleting the old importer crashes the unit test run sometimes. + // delete importer; + importer = new GPB::compiler::Importer(&source_tree, &error_collector); +} + +const GPB::DescriptorPool* DescriptorPoolLookup::pool() { return importer->pool(); } const GPB::DynamicMessageFactory* DescriptorPoolLookup::factory() { return &message_factory; } diff --git a/src/DescriptorPoolLookup.h b/src/DescriptorPoolLookup.h index b2763b7..d16d955 100644 --- a/src/DescriptorPoolLookup.h +++ b/src/DescriptorPoolLookup.h @@ -19,6 +19,8 @@ class DescriptorPoolLookup { static void importProtoFiles(SEXP files, SEXP cwd); + static void reset(); + static const GPB::DescriptorPool* pool(); static const GPB::DynamicMessageFactory* factory(); @@ -27,7 +29,7 @@ class DescriptorPoolLookup { static std::set elements; static RWarningErrorCollector error_collector; static RSourceTree source_tree; - static GPB::compiler::Importer importer; + static GPB::compiler::Importer* importer; static GPB::DynamicMessageFactory message_factory; }; diff --git a/src/RSourceTree.cpp b/src/RSourceTree.cpp index 5666edb..755aff9 100644 --- a/src/RSourceTree.cpp +++ b/src/RSourceTree.cpp @@ -48,5 +48,6 @@ void RSourceTree::removeDirectories(SEXP dirs) { directories.erase(std::string(CHAR(STRING_ELT(dirs, i)))); } } +void RSourceTree::removeAllDirectories() { directories.clear(); } } // namespace rprotobuf diff --git a/src/RSourceTree.h b/src/RSourceTree.h index 914d674..e447a88 100644 --- a/src/RSourceTree.h +++ b/src/RSourceTree.h @@ -10,6 +10,7 @@ class RSourceTree : public GPB::compiler::SourceTree { void addDirectories(SEXP dirs); void removeDirectory(const std::string& directory); void removeDirectories(SEXP dirs); + void removeAllDirectories(); private: std::set directories; diff --git a/src/init.c b/src/init.c index 442442c..19c1544 100644 --- a/src/init.c +++ b/src/init.c @@ -146,6 +146,7 @@ extern SEXP MethodDescriptor__output_type(SEXP); extern SEXP newProtocolBufferLookup(SEXP); extern SEXP newProtoMessage(SEXP); extern SEXP readProtoFiles_cpp(SEXP, SEXP); +extern SEXP resetDescriptorPool_cpp(); extern SEXP ServiceDescriptor__as_character(SEXP); extern SEXP ServiceDescriptor__as_list(SEXP); extern SEXP ServiceDescriptor__as_Message(SEXP); @@ -301,6 +302,7 @@ static const R_CallMethodDef CallEntries[] = { {"newProtocolBufferLookup", (DL_FUNC) &newProtocolBufferLookup, 1}, {"newProtoMessage", (DL_FUNC) &newProtoMessage, 1}, {"readProtoFiles_cpp", (DL_FUNC) &readProtoFiles_cpp, 2}, + {"resetDescriptorPool_cpp", (DL_FUNC) &resetDescriptorPool_cpp, 0}, {"ServiceDescriptor__as_character", (DL_FUNC) &ServiceDescriptor__as_character, 1}, {"ServiceDescriptor__as_list", (DL_FUNC) &ServiceDescriptor__as_list, 1}, {"ServiceDescriptor__as_Message", (DL_FUNC) &ServiceDescriptor__as_Message, 1}, @@ -341,3 +343,7 @@ void R_init_RProtoBuf(DllInfo *dll) { R_registerRoutines(dll, NULL, CallEntries, NULL, NULL); R_useDynamicSymbols(dll, FALSE); } + +void R_unload_RProtoBuf(DllInfo* dll) { + resetDescriptorPool_cpp(); +} diff --git a/src/rprotobuf.cpp b/src/rprotobuf.cpp index f238b59..ba238a7 100644 --- a/src/rprotobuf.cpp +++ b/src/rprotobuf.cpp @@ -50,6 +50,16 @@ RcppExport SEXP readProtoFiles_cpp(SEXP file, SEXP dirs) { END_RCPP } +/** + * reset the descriptor pool, clearing all persisted state + */ +RcppExport SEXP resetDescriptorPool_cpp() { + BEGIN_RCPP + DescriptorPoolLookup::reset(); + return R_NilValue; + END_RCPP +} + /** * get the descriptor associated with a message type *