Fix valid location check, sync with other functions (#1777)

* This fixes the path join code in valid location check, i.e., fixes 94bcea4216cdabcd18dbe5dc4686e38d4d3800de (#1715 ).
* In the new 1-or-3 parameter interface, path to database was dropped when constructing the full path to the location.
* Creating mapsets from command line with -c now works again.
* Other validity and existence checks now use the same 1-or-3 parameter interface (path or db, loc, mapset).
* Tests added for modified functions and for couple of related ones.
* Import order fixed with isort not to have glob mixed GRASS imports as pointed out by Pylint.
* Two missing docstrings added to the is-current-x checks.

python/grass/grassdb/checks.py

@@ -9,50 +9,71 @@ for details.
 .. sectionauthor:: Vaclav Petras <wenzeslaus gmail com>
 """
 
+import datetime
+import glob
 import os
 import sys
-import datetime
 from pathlib import Path
-from grass.script import gisenv
-import grass.script as gs
-import glob
 
 import grass.grassdb.config as cfg
+import grass.script as gs
+from grass.script import gisenv
 
 
-def mapset_exists(database, location, mapset):
-    """Returns True whether mapset path exists."""
-    location_path = os.path.join(database, location)
-    mapset_path = os.path.join(location_path, mapset)
-    if os.path.exists(mapset_path):
-        return True
-    return False
+def mapset_exists(path, location=None, mapset=None):
+    """Returns True whether mapset path exists.
 
+    Either only *path* is provided or all three parameters need to be provided.
+
+    :param path: Path to a Mapset or to a GRASS GIS database directory
+    :param location: name of a Location if not part of *path*
+    :param mapset: name of a Mapset if not part of *path*
+    """
+    if location and mapset:
+        path = os.path.join(path, location, mapset)
+    elif location or mapset:
+        raise ValueError(_("Provide only path or all three parameters, not two"))
+    return os.path.exists(path)
 
-def location_exists(database, location):
-    """Returns True whether location path exists."""
-    location_path = os.path.join(database, location)
-    if os.path.exists(location_path):
-        return True
-    return False
+
+def location_exists(path, location=None):
+    """Returns True whether location path exists.
+
+    :param path: Path to a Location or to a GRASS GIS database directory
+    :param location: name of a Location if not part of *path*
+    """
+    if location:
+        path = os.path.join(path, location)
+    return os.path.exists(path)
 
 
 # TODO: distinguish between valid for getting maps and usable as current
 # https://lists.osgeo.org/pipermail/grass-dev/2016-September/082317.html
 # interface created according to the current usage
-def is_mapset_valid(mapset_path):
-    """Return True if GRASS Mapset is valid"""
+def is_mapset_valid(path, location=None, mapset=None):
+    """Return True if GRASS Mapset is valid
+
+    Either only *path* is provided or all three parameters need to be provided.
+
+    :param path: Path to a Mapset or to a GRASS GIS database directory
+    :param location: name of a Location if not part of *path*
+    :param mapset: name of a Mapset if not part of *path*
+    """
     # WIND is created from DEFAULT_WIND by `g.region -d` and functions
     # or modules which create a new mapset. Most modules will fail if
     # WIND doesn't exist (assuming that neither GRASS_REGION nor
     # WIND_OVERRIDE environmental variables are set).
-    return os.access(os.path.join(mapset_path, "WIND"), os.R_OK)
+    if location and mapset:
+        path = os.path.join(path, location, mapset)
+    elif location or mapset:
+        raise ValueError(_("Provide only path or all three parameters, not two"))
+    return os.access(os.path.join(path, "WIND"), os.R_OK)
 
 
 def is_location_valid(path, location=None):
     """Return True if GRASS Location is valid
 
-    :param database: Path to a Location or to a GRASS GIS database directory
+    :param path: Path to a Location or to a GRASS GIS database directory
     :param location: name of a Location if not part of *path*
     """
     # DEFAULT_WIND file should not be required until you do something
@@ -60,11 +81,12 @@ def is_location_valid(path, location=None):
     # containing a PERMANENT/DEFAULT_WIND file is probably a GRASS
     # location, while a directory lacking it probably isn't.
     if location:
-        path = os.path.join(location)
+        path = os.path.join(path, location)
     return os.access(os.path.join(path, "PERMANENT", "DEFAULT_WIND"), os.F_OK)
 
 
 def is_mapset_current(database, location, mapset):
+    """Return True if the given GRASS Mapset is the current mapset"""
     genv = gisenv()
     if (
         database == genv["GISDBASE"]
@@ -76,6 +98,7 @@ def is_mapset_current(database, location, mapset):
 
 
 def is_location_current(database, location):
+    """Return True if the given GRASS Location is the current location"""
     genv = gisenv()
     if database == genv["GISDBASE"] and location == genv["LOCATION_NAME"]:
         return True

python/grass/grassdb/testsuite/test_checks.py

@@ -0,0 +1,88 @@
+# MODULE:    Test of grass.grassdb.checks
+#
+# AUTHOR(S): Vaclav Petras <wenzeslaus gmail com>
+#
+# PURPOSE:   Checks of GRASS database/location/mapset structure
+#
+# COPYRIGHT: (C) 2021 Vaclav Petras, and by the GRASS Development Team
+#
+#            This program is free software under the GNU General Public
+#            License (>=v2). Read the file COPYING that comes with GRASS
+#            for details.
+
+"""Tests of grass.grassdb.checks"""
+
+import os
+from pathlib import Path
+
+from grass.grassdb.checks import (
+    dir_contains_location,
+    get_list_of_locations,
+    is_location_valid,
+    is_mapset_valid,
+    location_exists,
+    mapset_exists,
+)
+from grass.gunittest.case import TestCase
+from grass.gunittest.gmodules import call_module
+from grass.gunittest.main import test
+
+
+class TestWithCurrent(TestCase):
+    """Tests that functions return expected result for the current mapset"""
+
+    def test_valid_location(self):
+        """Test that different parameter combinations work and return true"""
+        db_path = call_module("g.gisenv", get="GISDBASE").strip()
+        loc_name = call_module("g.gisenv", get="LOCATION_NAME").strip()
+        self.assertTrue(is_location_valid(db_path, loc_name))
+        self.assertTrue(is_location_valid(os.path.join(db_path, loc_name)))
+        self.assertTrue(is_location_valid(Path(db_path), loc_name))
+        self.assertTrue(is_location_valid(Path(db_path) / loc_name))
+
+    def test_valid_mapset(self):
+        """Test that different parameter combinations work and return true"""
+        db_path = call_module("g.gisenv", get="GISDBASE").strip()
+        loc_name = call_module("g.gisenv", get="LOCATION_NAME").strip()
+        mapset_name = call_module("g.gisenv", get="MAPSET").strip()
+        self.assertTrue(is_mapset_valid(db_path, loc_name, mapset_name))
+        self.assertTrue(is_mapset_valid(os.path.join(db_path, loc_name, mapset_name)))
+        self.assertTrue(is_mapset_valid(Path(db_path), loc_name, mapset_name))
+        self.assertTrue(is_mapset_valid(Path(db_path) / loc_name / mapset_name))
+
+    def test_location_exists(self):
+        """Test that different parameter combinations work and return true"""
+        db_path = call_module("g.gisenv", get="GISDBASE").strip()
+        loc_name = call_module("g.gisenv", get="LOCATION_NAME").strip()
+        self.assertTrue(location_exists(db_path, loc_name))
+        self.assertTrue(location_exists(os.path.join(db_path, loc_name)))
+        self.assertTrue(location_exists(Path(db_path), loc_name))
+        self.assertTrue(location_exists(Path(db_path) / loc_name))
+
+    def test_mapset_exists(self):
+        """Test that different parameter combinations work and return true"""
+        db_path = call_module("g.gisenv", get="GISDBASE").strip()
+        loc_name = call_module("g.gisenv", get="LOCATION_NAME").strip()
+        mapset_name = call_module("g.gisenv", get="MAPSET").strip()
+        self.assertTrue(mapset_exists(db_path, loc_name, mapset_name))
+        self.assertTrue(mapset_exists(os.path.join(db_path, loc_name, mapset_name)))
+        self.assertTrue(mapset_exists(Path(db_path), loc_name, mapset_name))
+        self.assertTrue(mapset_exists(Path(db_path) / loc_name / mapset_name))
+
+    def test_dir_contains_location(self):
+        """Test that different parameter combinations work and return true"""
+        db_path = call_module("g.gisenv", get="GISDBASE").strip()
+        self.assertTrue(dir_contains_location(db_path))
+        self.assertTrue(dir_contains_location(Path(db_path)))
+
+    def test_get_list_of_locations(self):
+        """Test that different parameter combinations work and return true"""
+        db_path = call_module("g.gisenv", get="GISDBASE").strip()
+        current_loc_name = call_module("g.gisenv", get="LOCATION_NAME").strip()
+        list_of_locations = get_list_of_locations(db_path)
+        self.assertTrue(list_of_locations, msg="No locations in the current db found")
+        self.assertIn(current_loc_name, list_of_locations)
+
+
+if __name__ == "__main__":
+    test()