Merge pull request #508 from vng/generator

Generator fixes

generator/feature_builder.cpp

@@ -138,7 +138,7 @@ bool FeatureBuilder1::RemoveInvalidTypes()
 
   return feature::RemoveNoDrawableTypes(m_params.m_Types,
                                         m_params.GetGeomType(),
-                                        m_params.name.IsEmpty());
+                                        m_params.IsEmptyNames());
 }
 
 bool FeatureBuilder1::FormatFullAddress(string & res) const

generator/feature_generator.hpp

@@ -38,6 +38,8 @@ public:
   FeaturesCollector(string const & fName);
   virtual ~FeaturesCollector();
 
+  string const & GetFilePath() const { return m_datFile.GetName(); }
+
   virtual void operator()(FeatureBuilder1 const & f);
 };
 

generator/generator.pro

@@ -54,6 +54,7 @@ HEADERS += \
     osm_element.hpp \
     osm_id.hpp \
     osm_o5m_source.hpp \
+    osm_translator.hpp \
     osm_xml_source.hpp \
     polygonizer.hpp \
     routing_generator.hpp \

generator/generator_tests/feature_builder_test.cpp

@@ -1,24 +1,15 @@
 #include "testing/testing.hpp"
 
+#include "types_helper.hpp"
+
 #include "generator/feature_builder.hpp"
+#include "generator/osm2type.hpp"
 
-#include "indexer/feature_visibility.hpp"
 #include "indexer/classificator_loader.hpp"
-#include "indexer/classificator.hpp"
+#include "indexer/feature_visibility.hpp"
 
+using namespace tests;
 
-namespace
-{
-
-template <size_t N, size_t M> void AddTypes(FeatureParams & params, char const * (&arr)[N][M])
-{
-  Classificator const & c = classif();
-
-  for (size_t i = 0; i < N; ++i)
-    params.AddType(c.GetTypeByPath(vector<string>(arr[i], arr[i] + M)));
-}
-
-}
 
 UNIT_TEST(FBuilder_ManyTypes)
 {
@@ -188,3 +179,28 @@ UNIT_TEST(FBuilder_WithoutName)
     TEST(!fb.RemoveInvalidTypes(), ());
   }
 }
+
+UNIT_TEST(FBuilder_PointAddress)
+{
+  classificator::Load();
+
+  char const * arr[][2] = { { "addr:housenumber", "39/79" } };
+
+  OsmElement e;
+  FillXmlElement(arr, ARRAY_SIZE(arr), &e);
+
+  FeatureParams params;
+  ftype::GetNameAndType(&e, params);
+
+  TEST_EQUAL(params.m_Types.size(), 1, ());
+  TEST(params.IsTypeExist(GetType({"building", "address"})), ());
+  TEST_EQUAL(params.house.Get(), "39/79", ());
+
+  FeatureBuilder1 fb;
+  fb.SetParams(params);
+  fb.SetCenter(m2::PointD(0, 0));
+
+  TEST(fb.PreSerialize(), ());
+  TEST(fb.RemoveInvalidTypes(), ());
+  TEST(fb.CheckValid(), ());
+}

generator/generator_tests/generator_tests.pro

@@ -13,6 +13,7 @@ QT *= core
 
 HEADERS += \
     source_data.hpp \
+    types_helper.hpp \
 
 SOURCES += \
     ../../testing/testingmain.cpp \

generator/generator_tests/osm_type_test.cpp

@@ -1,5 +1,7 @@
 #include "testing/testing.hpp"
 
+#include "types_helper.hpp"
+
 #include "generator/osm_element.hpp"
 #include "generator/osm2type.hpp"
 
@@ -12,21 +14,7 @@
 #include "std/iostream.hpp"
 
 
-namespace
-{
-  void FillXmlElement(char const * arr[][2], size_t count, OsmElement * p)
-  {
-    for (size_t i = 0; i < count; ++i)
-      p->AddTag(arr[i][0], arr[i][1]);
-  }
-
-  template <size_t N> uint32_t GetType(char const * (&arr)[N])
-  {
-    vector<string> path(arr, arr + N);
-    return classif().GetTypeByPath(path);
-  }
-  uint32_t GetType(StringIL const & lst) { return classif().GetTypeByPath(lst); }
-}
+using namespace tests;
 
 UNIT_TEST(OsmType_SkipDummy)
 {

generator/generator_tests/types_helper.hpp

@@ -0,0 +1,42 @@
+#pragma once
+
+#include "generator/osm_element.hpp"
+
+#include "indexer/classificator.hpp"
+#include "indexer/feature_data.hpp"
+
+#include "std/string.hpp"
+#include "std/vector.hpp"
+
+
+namespace tests
+{
+
+template <size_t N, size_t M>
+inline void AddTypes(FeatureParams & params, char const * (&arr)[N][M])
+{
+  Classificator const & c = classif();
+
+  for (size_t i = 0; i < N; ++i)
+    params.AddType(c.GetTypeByPath(vector<string>(arr[i], arr[i] + M)));
+}
+
+inline void FillXmlElement(char const * arr[][2], size_t count, OsmElement * p)
+{
+  for (size_t i = 0; i < count; ++i)
+    p->AddTag(arr[i][0], arr[i][1]);
+}
+
+template <size_t N>
+inline uint32_t GetType(char const * (&arr)[N])
+{
+  vector<string> path(arr, arr + N);
+  return classif().GetTypeByPath(path);
+}
+
+inline uint32_t GetType(StringIL const & lst)
+{
+  return classif().GetTypeByPath(lst);
+}
+
+} // namespace tests

generator/generator_tests_support/generator_tests_support.pro

@@ -0,0 +1,17 @@
+TARGET = generator_tests_support
+TEMPLATE = lib
+CONFIG += staticlib warn_on
+
+ROOT_DIR = ../..
+DEPENDENCIES = generator map routing indexer platform geometry coding base \
+               expat tess2 protobuf tomcrypt osrm succinct
+
+include($$ROOT_DIR/common.pri)
+
+INCLUDEPATH *= $$ROOT_DIR/3party/expat/lib
+
+HEADERS += \
+    test_mwm_builder.hpp \
+
+SOURCES += \
+    test_mwm_builder.cpp \

generator/generator_tests_support/test_mwm_builder.cpp

@@ -1,4 +1,4 @@
-#include "search/search_integration_tests/test_mwm_builder.hpp"
+#include "test_mwm_builder.hpp"
 
 #include "indexer/classificator.hpp"
 #include "indexer/data_header.hpp"
@@ -12,10 +12,13 @@
 
 #include "platform/local_country_file.hpp"
 
+#include "coding/internal/file_data.hpp"
+
 #include "base/logging.hpp"
 
 #include "defines.hpp"
 
+
 TestMwmBuilder::TestMwmBuilder(platform::LocalCountryFile & file)
     : m_file(file),
       m_collector(
@@ -28,35 +31,53 @@ TestMwmBuilder::~TestMwmBuilder()
 {
   if (m_collector)
     Finish();
-  CHECK(!m_collector, ("Features weren't dumped on disk."));
 }
 
 void TestMwmBuilder::AddPOI(m2::PointD const & p, string const & name, string const & lang)
 {
-  CHECK(m_collector, ("It's not possible to add features after call to Finish()."));
   FeatureBuilder1 fb;
   fb.SetCenter(p);
   fb.SetType(m_classificator.GetTypeByPath({"railway", "station"}));
   CHECK(fb.AddName(lang, name), ("Can't set feature name:", name, "(", lang, ")"));
-  (*m_collector)(fb);
+
+  CHECK(Add(fb), (fb));
+}
+
+bool TestMwmBuilder::Add(FeatureBuilder1 & fb)
+{
+  CHECK(m_collector, ("It's not possible to add features after call to Finish()."));
+
+  if (fb.PreSerialize() && fb.RemoveInvalidTypes())
+  {
+    (*m_collector)(fb);
+    return true;
+  }
+  return false;
 }
 
 void TestMwmBuilder::Finish()
 {
+  string const tmpFilePath = m_collector->GetFilePath();
+
   CHECK(m_collector, ("Finish() already was called."));
   m_collector.reset();
+
   feature::GenerateInfo info;
   info.m_targetDir = m_file.GetDirectory();
   info.m_tmpDir = m_file.GetDirectory();
   CHECK(GenerateFinalFeatures(info, m_file.GetCountryFile().GetNameWithoutExt(),
                               feature::DataHeader::country),
         ("Can't sort features."));
-  CHECK(feature::BuildOffsetsTable(m_file.GetPath(MapOptions::Map)), ("Can't build feature offsets table."));
-  CHECK(indexer::BuildIndexFromDatFile(m_file.GetPath(MapOptions::Map),
-                                       m_file.GetPath(MapOptions::Map)),
-        ("Can't build geometry index."));
-  CHECK(indexer::BuildSearchIndexFromDatFile(m_file.GetPath(MapOptions::Map),
-                                             true /* forceRebuild */),
+
+  CHECK(my::DeleteFileX(tmpFilePath), ());
+
+  string const mapFilePath = m_file.GetPath(MapOptions::Map);
+  CHECK(feature::BuildOffsetsTable(mapFilePath), ("Can't build feature offsets table."));
+
+  CHECK(indexer::BuildIndexFromDatFile(mapFilePath, mapFilePath), ("Can't build geometry index."));
+
+  CHECK(indexer::BuildSearchIndexFromDatFile(mapFilePath, true /* forceRebuild */),
         ("Can't build search index."));
+
   m_file.SyncWithDisk();
 }

generator/generator_tests_support/test_mwm_builder.hpp

@@ -5,7 +5,9 @@
 #include "std/string.hpp"
 #include "std/unique_ptr.hpp"
 
+
 class Classificator;
+class FeatureBuilder1;
 
 namespace feature
 {
@@ -24,6 +26,7 @@ public:
   ~TestMwmBuilder();
 
   void AddPOI(m2::PointD const & p, string const & name, string const & lang);
+  bool Add(FeatureBuilder1 & fb);
 
   void Finish();
 

indexer/feature_data.cpp

@@ -146,6 +146,11 @@ string FeatureParamsBase::DebugString() const
           (!ref.empty() ? " Ref:" + ref : ""));
 }
 
+bool FeatureParamsBase::IsEmptyNames() const
+{
+  return name.IsEmpty() && house.IsEmpty() && ref.empty();
+}
+
 namespace
 {
 

indexer/feature_data.hpp

@@ -131,9 +131,11 @@ struct FeatureParamsBase
   bool operator == (FeatureParamsBase const & rhs) const;
 
   bool CheckValid() const;
-
   string DebugString() const;
 
+  /// @return true if feature doesn't have any drawable strings (names, houses, etc).
+  bool IsEmptyNames() const;
+
   template <class TSink>
   void Write(TSink & sink, uint8_t header) const
   {

omim.pro

@@ -118,10 +118,6 @@ SUBDIRS = 3party base coding geometry indexer routing
     MapDepLibs = 3party base coding geometry platform storage indexer search map \
                  routing anim render gui graphics
 
-    search_integration_tests.subdir = search/search_integration_tests
-    search_integration_tests.depends = $$MapDepLibs generator
-    SUBDIRS *= search_integration_tests
-
     map_tests.subdir = map/map_tests
     map_tests.depends = $$MapDepLibs
     SUBDIRS *= map_tests
@@ -146,8 +142,16 @@ SUBDIRS = 3party base coding geometry indexer routing
     pedestrian_routing_tests.depends = $$MapDepLibs routing
     SUBDIRS *= pedestrian_routing_tests
 
+    generator_tests_support.subdir = generator/generator_tests_support
+    generator_tests_support.depends = $$MapDepLibs generator
+    SUBDIRS *= generator_tests_support
+
+    search_integration_tests.subdir = search/search_integration_tests
+    search_integration_tests.depends = $$MapDepLibs generator generator_tests_support
+    SUBDIRS *= search_integration_tests
+
     generator_tests.subdir = generator/generator_tests
-    generator_tests.depends = $$MapDepLibs routing generator
+    generator_tests.depends = $$MapDepLibs routing generator generator_tests_support
     SUBDIRS *= generator_tests
 
     # TODO(AlexZ): Do we really need them?

search/search_integration_tests/retrieval_test.cpp

@@ -1,5 +1,7 @@
 #include "testing/testing.hpp"
 
+#include "generator/generator_tests_support/test_mwm_builder.hpp"
+
 #include "indexer/classificator_loader.hpp"
 #include "indexer/index.hpp"
 #include "indexer/mwm_set.hpp"
@@ -7,7 +9,6 @@
 #include "indexer/search_delimiters.hpp"
 #include "indexer/search_string_utils.hpp"
 
-#include "search/search_integration_tests/test_mwm_builder.hpp"
 #include "search/retrieval.hpp"
 #include "search/search_query_params.hpp"
 

search/search_integration_tests/search_integration_tests.pro

@@ -6,8 +6,8 @@ CONFIG -= app_bundle
 TEMPLATE = app
 
 ROOT_DIR = ../..
-DEPENDENCIES = generator routing search storage stats_client jansson indexer platform geometry coding base \
-               tess2 protobuf tomcrypt
+DEPENDENCIES = generator_tests_support generator routing search storage stats_client indexer \
+               platform geometry coding base tess2 protobuf tomcrypt jansson
 
 DEPENDENCIES += opening_hours \
 
@@ -22,11 +22,9 @@ SOURCES += \
     ../../testing/testingmain.cpp \
     retrieval_test.cpp \
     smoke_test.cpp \
-    test_mwm_builder.cpp \
     test_search_engine.cpp \
     test_search_request.cpp \
 
 HEADERS += \
-    test_mwm_builder.hpp \
     test_search_engine.hpp \
     test_search_request.hpp \

search/search_integration_tests/smoke_test.cpp

@@ -1,12 +1,13 @@
 #include "testing/testing.hpp"
 
+#include "generator/generator_tests_support/test_mwm_builder.hpp"
+
+#include "search/search_integration_tests/test_search_engine.hpp"
+#include "search/search_integration_tests/test_search_request.hpp"
+
 #include "indexer/classificator_loader.hpp"
 #include "indexer/scales.hpp"
 
-#include "search/search_integration_tests/test_mwm_builder.hpp"
-#include "search/search_integration_tests/test_search_engine.hpp"
-#include "search/search_integration_tests/test_search_request.hpp"
-
 #include "platform/country_defines.hpp"
 #include "platform/country_file.hpp"
 #include "platform/local_country_file.hpp"