Browse Source

[2378] It would throw at load(), not constructor

There's no way to know the master file would be broken in the
constructor. So update the documentation to say it'd throw from the
load() and loadIncremental().
Michal 'vorner' Vaner 12 years ago
parent
commit
807b5a13c2
2 changed files with 15 additions and 9 deletions
  1. 11 7
      src/lib/datasrc/tests/zone_loader_unittest.cc
  2. 4 2
      src/lib/datasrc/zone_loader.h

+ 11 - 7
src/lib/datasrc/tests/zone_loader_unittest.cc

@@ -32,8 +32,11 @@
 
 
 using isc::dns::RRClass;
 using isc::dns::RRClass;
 using isc::dns::Name;
 using isc::dns::Name;
+using isc::dns::RRType;
+using isc::dns::ConstRRsetPtr;
 using std::string;
 using std::string;
 using std::vector;
 using std::vector;
+using boost::shared_ptr;
 using namespace isc::datasrc;
 using namespace isc::datasrc;
 
 
 namespace {
 namespace {
@@ -168,7 +171,7 @@ private:
 
 
     // FIXME: We should be destroying it by ZoneTableSegment::destroy.
     // FIXME: We should be destroying it by ZoneTableSegment::destroy.
     // But the shared pointer won't let us, will it?
     // But the shared pointer won't let us, will it?
-    boost::shared_ptr<memory::ZoneTableSegment> ztable_segment_;
+    shared_ptr<memory::ZoneTableSegment> ztable_segment_;
 protected:
 protected:
     memory::InMemoryClient source_client_;
     memory::InMemoryClient source_client_;
     // This one is mocked. It will help us see what is happening inside.
     // This one is mocked. It will help us see what is happening inside.
@@ -380,12 +383,13 @@ TEST_F(ZoneLoaderTest, loadNoSuchFile) {
 
 
 // And it also throws when there's a syntax error in the master file
 // And it also throws when there's a syntax error in the master file
 TEST_F(ZoneLoaderTest, loadSyntaxError) {
 TEST_F(ZoneLoaderTest, loadSyntaxError) {
-    EXPECT_THROW(ZoneLoader(destination_client_, Name::ROOT_NAME(),
+    ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
-                            // This is not a master file for sure
+                      // This is not a master file for sure
-                            // (misusing a file that happens to be there
+                      // (misusing a file that happens to be there
-                            // already).
+                      // already).
-                            TEST_DATA_DIR "/example.org.sqlite3"),
+                      TEST_DATA_DIR "/example.org.sqlite3");
-                 MasterFileError);
+    EXPECT_THROW(loader.load(), MasterFileError);
+    EXPECT_FALSE(destination_client_.commit_called_);
 }
 }
 
 
 }
 }

+ 4 - 2
src/lib/datasrc/zone_loader.h

@@ -69,8 +69,6 @@ public:
     ///     beforehead.
     ///     beforehead.
     /// \throw DataSourceError in case of other (possibly low-level) errors,
     /// \throw DataSourceError in case of other (possibly low-level) errors,
     ///     such as read-only data source or database error.
     ///     such as read-only data source or database error.
-    /// \throw MasterFileError when the master_file is badly formatted or some
-    ///     similar problem is found when loading the master file.
     ZoneLoader(DataSourceClient& destination, const isc::dns::Name& zone_name,
     ZoneLoader(DataSourceClient& destination, const isc::dns::Name& zone_name,
                const char* master_file);
                const char* master_file);
 
 
@@ -104,6 +102,8 @@ public:
     /// \throw InvalidOperation in case the loading was already completed
     /// \throw InvalidOperation in case the loading was already completed
     ///     before this call.
     ///     before this call.
     /// \throw DataSourceError in case some error (possibly low-level) happens.
     /// \throw DataSourceError in case some error (possibly low-level) happens.
+    /// \throw MasterFileError when the master_file is badly formatted or some
+    ///     similar problem is found when loading the master file.
     void load() {
     void load() {
         while (!loadIncremental(1000)) { // 1000 is arbitrary largish number
         while (!loadIncremental(1000)) { // 1000 is arbitrary largish number
             // Body intentionally left blank.
             // Body intentionally left blank.
@@ -128,6 +128,8 @@ public:
     ///     before this call (by load() or by a loadIncremental that returned
     ///     before this call (by load() or by a loadIncremental that returned
     ///     true).
     ///     true).
     /// \throw DataSourceError in case some error (possibly low-level) happens.
     /// \throw DataSourceError in case some error (possibly low-level) happens.
+    /// \throw MasterFileError when the master_file is badly formatted or some
+    ///     similar problem is found when loading the master file.
     bool loadIncremental(size_t limit);
     bool loadIncremental(size_t limit);
 private:
 private:
     /// \brief The iterator used as source of data in case of the copy mode.
     /// \brief The iterator used as source of data in case of the copy mode.