Improve auto-publish usability (#795)

* on `--save-config`, only save configured `auto_publish` settings * alias `from_schemas` as `from_schema` * add integration testing for `auto_publish` * if integration test DB preloading fails, try to clean up the test DB * A few more info traces This change should benefit testing of the #790 cc: @Binabh
maplibre · Aug 3, 2023 · 405d4b2 · 405d4b2
1 parent a2acd17
commit 405d4b2
Show file tree

Hide file tree

Showing 10 changed files with 142 additions and 23 deletions.
diff --git a/justfile b/justfile
@@ -56,6 +56,11 @@ docker-up name:
 alias _down := stop
 alias _stop-db := stop
 
+# Restart the test database
+restart:
+    just stop
+    just start
+
 # Stop the test database
 stop:
     docker-compose down

diff --git a/martin/src/pg/config.rs b/martin/src/pg/config.rs
@@ -59,14 +59,21 @@ pub struct PgConfig {
 
 #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]
 pub struct PgCfgPublish {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    #[serde(alias = "from_schema")]
     pub from_schemas: Option<OneOrMany<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub tables: Option<BoolOrObject<PgCfgPublishType>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub functions: Option<BoolOrObject<PgCfgPublishType>>,
 }
 
 #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]
 pub struct PgCfgPublishType {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    #[serde(alias = "from_schema")]
     pub from_schemas: Option<OneOrMany<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     #[serde(alias = "id_format")]
     pub source_id_format: Option<String>,
 }

diff --git a/martin/src/pg/configurator.rs b/martin/src/pg/configurator.rs
@@ -29,21 +29,6 @@ pub struct PgBuilderAuto {
     schemas: Option<HashSet<String>>,
 }
 
-impl PgBuilderAuto {
-    pub fn new(
-        is_function: bool,
-        source_id_format: Option<&String>,
-        schemas: Option<HashSet<String>>,
-    ) -> Self {
-        Self {
-            source_id_format: source_id_format.cloned().unwrap_or_else(|| {
-                (if is_function { "{function}" } else { "{table}" }).to_string()
-            }),
-            schemas,
-        }
-    }
-}
-
 #[derive(Debug)]
 pub struct PgBuilder {
     pool: PgPool,
@@ -115,6 +100,12 @@ impl PgBuilder {
                 .as_ref()
                 .cloned()
                 .unwrap_or_else(|| db_tables_info.keys().cloned().collect());
+            info!(
+                "Auto-publishing tables in schemas [{}] as '{}' sources",
+                schemas.iter().sorted().join(", "),
+                auto_tables.source_id_format,
+            );
+
             for schema in schemas.iter().sorted() {
                 let Some(schema) = normalize_key(&db_tables_info, schema, "schema", "") else { continue };
                 let db_tables = db_tables_info.remove(&schema).unwrap();
@@ -186,7 +177,7 @@ impl PgBuilder {
             warn_on_rename(id, &id2, "Function");
             let signature = &pg_sql.signature;
             info!("Configured {dup}source {id2} from the function {signature}");
-            debug!("{}", pg_sql.query);
+            debug!("{id2} query: {}", pg_sql.query);
             info_map.insert(id2, cfg_inf.clone());
         }
 
@@ -197,6 +188,11 @@ impl PgBuilder {
                 .as_ref()
                 .cloned()
                 .unwrap_or_else(|| db_funcs_info.keys().cloned().collect());
+            info!(
+                "Auto-publishing functions in schemas [{}] as '{}' sources",
+                schemas.iter().sorted().join(", "),
+                auto_funcs.source_id_format,
+            );
 
             for schema in schemas.iter().sorted() {
                 let Some(schema) = normalize_key(&db_funcs_info, schema, "schema", "") else { continue; };
@@ -212,7 +208,7 @@ impl PgBuilder {
                     let id2 = self.resolve_id(&source_id, &db_inf);
                     self.add_func_src(&mut res, id2.clone(), &db_inf, pg_sql.clone());
                     info!("Discovered source {id2} from function {}", pg_sql.signature);
-                    debug!("{}", pg_sql.query);
+                    debug!("{id2} query: {}", pg_sql.query);
                     info_map.insert(id2, db_inf);
                 }
             }
@@ -237,17 +233,26 @@ impl PgBuilder {
 }
 
 fn new_auto_publish(config: &PgConfig, is_function: bool) -> Option<PgBuilderAuto> {
-    let default = |schemas| Some(PgBuilderAuto::new(is_function, None, schemas));
+    let default_id_fmt = |is_func| (if is_func { "{function}" } else { "{table}" }).to_string();
+    let default = |schemas| {
+        Some(PgBuilderAuto {
+            source_id_format: default_id_fmt(is_function),
+            schemas,
+        })
+    };
 
     if let Some(bo_a) = &config.auto_publish {
         match bo_a {
             BoolOrObject::Object(a) => match if is_function { &a.functions } else { &a.tables } {
                 Some(bo_i) => match bo_i {
-                    BoolOrObject::Object(item) => Some(PgBuilderAuto::new(
-                        is_function,
-                        item.source_id_format.as_ref(),
-                        merge_opt_hs(&a.from_schemas, &item.from_schemas),
-                    )),
+                    BoolOrObject::Object(item) => Some(PgBuilderAuto {
+                        source_id_format: item
+                            .source_id_format
+                            .as_ref()
+                            .cloned()
+                            .unwrap_or_else(|| default_id_fmt(is_function)),
+                        schemas: merge_opt_hs(&a.from_schemas, &item.from_schemas),
+                    }),
                     BoolOrObject::Bool(true) => default(merge_opt_hs(&a.from_schemas, &None)),
                     BoolOrObject::Bool(false) => None,
                 },

diff --git a/tests/config.yaml b/tests/config.yaml
@@ -19,6 +19,10 @@ postgres:
   # Maximum connections pool size [default: 20]
   pool_size: 20
 
+  auto_publish:
+    tables:
+      from_schemas: autodetect
+
   # Associative arrays of table sources
   tables:
     table_source:

diff --git a/tests/expected/auto/catalog_auto.json b/tests/expected/auto/catalog_auto.json
@@ -4,6 +4,11 @@
     "content_type": "application/x-protobuf",
     "description": "MixedCase.MixPoints.Geom"
   },
+  {
+    "id": "auto_table",
+    "content_type": "application/x-protobuf",
+    "description": "autodetect.auto_table.geom"
+  },
   {
     "id": "function_Mixed_Name",
     "content_type": "application/x-protobuf",

diff --git a/tests/expected/configured/catalog_cfg.json b/tests/expected/configured/catalog_cfg.json
@@ -4,6 +4,11 @@
     "content_type": "application/x-protobuf",
     "description": "MixedCase.MixPoints.Geom"
   },
+  {
+    "id": "auto_table",
+    "content_type": "application/x-protobuf",
+    "description": "autodetect.auto_table.geom"
+  },
   {
     "id": "function_zxy_query",
     "content_type": "application/x-protobuf",

diff --git a/tests/expected/generated_config.yaml b/tests/expected/generated_config.yaml
@@ -16,6 +16,18 @@ postgres:
       properties:
         Gid: int4
         TABLE: text
+    auto_table:
+      schema: autodetect
+      table: auto_table
+      srid: 4326
+      geometry_column: geom
+      extent: 4096
+      buffer: 64
+      clip_geom: true
+      geometry_type: POINT
+      properties:
+        feat_id: int4
+        gid: int4
     points1:
       schema: public
       table: points1

diff --git a/tests/expected/given_config.yaml b/tests/expected/given_config.yaml
@@ -5,6 +5,9 @@ postgres:
   default_srid: 4326
   max_feature_count: 1000
   pool_size: 20
+  auto_publish:
+    tables:
+      from_schemas: autodetect
   tables:
     MixPoints:
       schema: MixedCase
@@ -20,6 +23,23 @@ postgres:
       geometry_type: POINT
       properties:
         taBLe: text
+    auto_table:
+      schema: autodetect
+      table: auto_table
+      srid: 4326
+      geometry_column: geom
+      bounds:
+      - -166.87107126230424
+      - -53.44747249115674
+      - 168.14061220360549
+      - 84.22411861475385
+      extent: 4096
+      buffer: 64
+      clip_geom: true
+      geometry_type: POINT
+      properties:
+        feat_id: int4
+        gid: int4
     points1:
       layer_id: abc
       schema: public

diff --git a/tests/fixtures/initdb.sh b/tests/fixtures/initdb.sh
@@ -14,6 +14,9 @@ psql -P pager=off -v ON_ERROR_STOP=1 -c "DROP SCHEMA IF EXISTS tiger CASCADE;"
 psql -P pager=off -v ON_ERROR_STOP=1 -t -c "select version();"
 psql -P pager=off -v ON_ERROR_STOP=1 -t -c "select PostGIS_Full_Version();"
 
+# On error, make sure do delete all the tables we created
+# TODO: see if we can have a fail-early service test to detect errors
+trap 'echo -e "\n\n\n!!!!!!!!!!!!!!!!!!!!!!!!\n\nDELETING DB $PGDATABASE DUE TO AN ERROR!\n\n\n" && psql -c "DROP SCHEMA IF EXISTS "MixedCase" CASCADE; DROP SCHEMA IF EXISTS autodetect CASCADE;"' ERR
 
 echo -e "\n\n\n"
 echo "################################################################################################"

diff --git a/tests/fixtures/tables/autodetect.sql b/tests/fixtures/tables/autodetect.sql
@@ -0,0 +1,53 @@
+DROP SCHEMA IF EXISTS autodetect CASCADE;
+CREATE SCHEMA autodetect;
+
+CREATE TABLE autodetect.auto_table
+(
+    gid     SERIAL PRIMARY KEY,
+    feat_id INTEGER,
+    geom    GEOMETRY(POINT, 4326)
+);
+
+-- INSERT INTO autodetect.auto_table
+-- SELECT generate_series(1, 3) as id,
+--        (random() * 100000)::int as feat_id,
+--        (ST_DUMP(ST_GENERATEPOINTS(st_transform(st_tileenvelope(18, 235085, 122323), 4326), 3))).geom;
+-- INSERT INTO autodetect.auto_table
+-- SELECT generate_series(4, 30) as id,
+--        (random() * 100000)::int as feat_id,
+--        (ST_DUMP(ST_GENERATEPOINTS(st_transform(st_tileenvelope(0, 0, 0), 4326), 27))).geom;
+
+INSERT INTO autodetect.auto_table (gid, feat_id, geom)
+values (1, 71951, '0101000020E6100000211700C9E6DA6140F510E7C8F4DA2740'),
+       (2, 9437, '0101000020E61000005DC569C7E9DA6140CEC346FD7FDA2740'),
+       (3, 10709, '0101000020E6100000325F66F8E7DA61402FEEB913C5DA2740'),
+       (4, 70797, '0101000020E61000001CB4FF744BBD25C0BA329AF5570E5540'),
+       (5, 55425, '0101000020E61000001A9CADC39CB264402CC28861D96609C0'),
+       (6, 34933, '0101000020E6100000D21D13F4388C45C08A99F840FD1149C0'),
+       (7, 78891, '0101000020E61000000E5F9A5AC5364540584BA4A03CF31C40'),
+       (8, 51663, '0101000020E6100000B81C879D88DD5FC0644E7CD053D439C0'),
+       (9, 93945, '0101000020E6100000DBB399E0178A584048183AEEFCDA5340'),
+       (10, 17259, '0101000020E61000009633C32DCCFE4F40B3146ED8D7B346C0'),
+       (11, 26778, '0101000020E6100000087148D449A02BC0F95A2F60F6E841C0'),
+       (12, 921, '0101000020E6100000E04D125E8D185EC039BAF402A28F4840'),
+       (13, 62646, '0101000020E6100000B7FA72B6A32B3BC0CCF234580EAC3440'),
+       (14, 56090, '0101000020E6100000952B6EA7470654C0B4BFE4DB36213A40'),
+       (15, 14681, '0101000020E61000001C61802A806E61403212918F4FAF45C0'),
+       (16, 2042, '0101000020E6100000D81618AA67453D4077068459211A5240'),
+       (17, 71290, '0101000020E6100000EEFC29E57F046540A8838DDFA4AE0A40'),
+       (18, 86526, '0101000020E61000004EDCE036C1765C405FB54DED58CB5040'),
+       (19, 42544, '0101000020E61000005EAF676356CF49C01C71E517855131C0'),
+       (20, 81737, '0101000020E6100000B3DADB584C84514019BFD2EE3C234340'),
+       (21, 52337, '0101000020E6100000A102D7D0DFDB64C05DB051C746B94AC0'),
+       (22, 29153, '0101000020E61000008E0CF54B76E35640C2B0F17CD34C5240'),
+       (23, 70091, '0101000020E6100000C5C9BC82DBAC434018C3058D1D652CC0'),
+       (24, 3334, '0101000020E6100000B28E82F0E58F61C00BA6D4F65F695440'),
+       (25, 71750, '0101000020E610000050527E5BE5691CC06B9DB1B09C2B41C0'),
+       (26, 24924, '0101000020E610000041AA2CDA78F963C07C118FE29D084240'),
+       (27, 50823, '0101000020E6100000313E08EFA54859C051F0059F9FB95240'),
+       (28, 76138, '0101000020E6100000E72E767A1FD941C06AFA84BD7ADB13C0'),
+       (29, 63733, '0101000020E61000009A34CC4D671844C0903C5B00CF1B1340'),
+       (30, 98054, '0101000020E6100000FCB7E4474EBA6140468D5E7496BD43C0');
+
+CREATE INDEX ON autodetect.auto_table USING GIST (geom);
+CLUSTER auto_table_geom_idx ON autodetect.auto_table;