Fix /api/v1 prefix consistency and add a root resource to the REST API - #111
Open
ArthurLuciani2 wants to merge 130 commits into
Open
Fix /api/v1 prefix consistency and add a root resource to the REST API#111ArthurLuciani2 wants to merge 130 commits into
/api/v1 prefix consistency and add a root resource to the REST API#111ArthurLuciani2 wants to merge 130 commits into
Conversation
- Import `NotAnFSDBError` in `core.py` - Import `_is_fsdb` from `plantdb.commons.fsdb.validation` in `core.py` - Add a `_is_fsdb(basedir)` check in `FSDB.__init__` and raise `NotAnFSDBError` if the directory is not a valid FSDB
- Add return type hint `-> bool` to `_is_fsdb` in `validation.py` - Expand `_is_fsdb` docstring with detailed description and usage examples - Verify the provided path is a directory before proceeding - Ensure the presence of the `MARKER_FILE_NAME` file - Introduce scan‑directory validation using new helper `_is_scan_dataset` - Log warnings for empty databases and for any bad scan directories found - Add new function `_is_scan_dataset` to validate FSDB datasets: - Checks for required `files.json` and valid JSON structure - Optionally validates filesets if `validate_json_fileset` is true - Confirms presence of required `metadata` subdirectory - Update `_is_safe_to_delete` signature to `-> bool` and improve its docstring.
- Import `_fileset_path` and `_scan_json_file` from `path_helpers` in `validation.py` - Extend `_is_fsdb` signature to accept `validate_json_fileset` and pass it to scan checks - Rename parameters to `scan_path` in `_is_scan_dataset` and update all internal path usages - Add optional `validate_json_fileset` flag documentation to both `_is_fsdb` and `_is_scan_dataset` - Implement `_is_valid_fileset` to verify fileset directories and required files listed in `files.json` - Update `_is_scan_dataset` to invoke `_is_valid_fileset` when validation is enabled - Adjust calls to `_is_scan_dataset` in `_is_fsdb` to include the new flag - Refactor variable names and path handling for clarity across the validation module
- Import `_is_scan_dataset` in `file_ops.py` (pre‑load for validation utilities). - Remove `required_fs` handling in `get_scans`; now only checks that the scan directory exists before loading filesets. - Comment out the user prompt (`yes_no_choice`) and deletion loop for bad scans, preventing interactive prompts in non‑TTY environments. - Keep existing logic for loading filesets and updating scans unchanged.
…mentation
- Update `pyproject.toml` script entry: replace `fsdb_check` with `fsdb_healthcheck`.
- Modify `validation.py` to suggest the new `fsdb_healthcheck` CLI instead of a TODO comment.
- Add new CLI module `src/commons/plantdb/commons/cli/fsdb_healthcheck.py`:
- Replace `argparse` with `click` for argument parsing.
- Introduce `--log-level`, `--fix`, `--fix-missing`, and `--fix-extra` options.
- Configure logger via `get_logger('fsdb_healthcheck', ...)`.
- Implement missing‑reference fixing logic with progress bar and backup handling.
- Stub `--fix-extra` with `NotImplementedError`.
- Remove old `fsdb_check.py` implementation.
- Log an error when the provided path is not a directory. - Log an error when the required marker file `MARKER_FILE_NAME` is missing. - Update empty‑FSDB warning to use the path string directly. - Store bad scan directories as strings and simplify the bad‑scan log output. - Add explicit error logging for missing `metadata` subdirectory. - Add error logs for missing `files.json`, JSON parse failures, and missing `filesets` entry. - Log an error when a fileset directory defined in `files.json` is absent. - Introduce `_fileset_files_exists` helper to verify all required files exist and log the count of missing files. - Update `_is_valid_fileset` to use the new helper for detailed missing‑file reporting.
…ional files‑json updates - Updated `file_ops.py` to use `Path` from `pathlib` and added `typing` imports for clearer type hints. - Replaced direct `shutil.rmtree` calls with `send2trash` for safer, reversible deletions of scans, filesets, and metadata directories. - Introduced `backup_file` usage before overwriting `files.json` when `updates_files_json` is enabled. - Extended `_load_scan` signature to `def _load_scan(db: 'FSDB', scan_id: str, updates_files_json: bool = False) -> 'Scan | None'` and added detailed docstring. - Modified `_load_scans` to return a `dict[str, 'Scan']`, accept `updates_files_json` flag, and improved handling of bad scans (no interactive prompts). - Added validation imports (`_is_valid_fileset`, `_is_scan_dataset`) and guarded type‑checking imports with `TYPE_CHECKING` to avoid circular dependencies. - Updated helper functions (`_load_scan_filesets`, `_load_fileset`, `_load_fileset_files`) to return a `(result, needs_update)` tuple, propagating the update flag. - Adjusted internal calls to reflect new return signatures and update logic. - Replaced legacy `yes_no_choice` prompt handling with commented‑out code, eliminating interactive deletion in non‑TTY environments. - Updated function signatures for `_load_dummy_fileset`, `_load_file`, `_load_measures`, `_load_scan_measures`, `_delete_file`, `_delete_fileset`, `_delete_scan`, `_make_fileset`, `_make_scan`, and `_store_scan` with explicit type hints and return annotations.
- Reformat `yes_no_choice` signature to use explicit spacing (`default: bool = True`). - Introduce `yes_no_abort_choice` in `utils.py` to allow aborting a yes/no prompt and return `None` when aborted. - Add `backup_filename` function to create a timestamped backup path for a given file. - Add `backup_file` function that copies the original file to the backup path generated by `backup_filename`.
…nnect - Drop the `required_filesets` attribute, its `__init__` parameter, and related docstring sections in `core.py` - Initialize scans without setting `self.required_filesets`; default validation now relies on the presence of a `metadata` fileset - Add an `_is_fsdb(self.basedir)` check in `FSDB.connect` to raise `NotAnFSDBError` when the directory is not a valid FSDB - Clean up import comments and unused code related to required filesets.
- Updated `plantdb/src/commons/pyproject.toml` to include ``send2trash`` in the `dependencies` list, enabling safe recycle‑bin deletions.
- Grouped CLI options with `click_option_group` into “Fix” and “Logging” sections and reordered parameters (`fsdb_path`, `fix`, `fix_missing`, `fix_extra`, `log_level`) - Added explicit FSDB marker validation using `MARKER_FILE_NAME` and raise `NotAnFSDBError` when missing - Replaced direct directory check with `Path.is_dir()` and added early error handling for non‑directory paths - Collected scan directories while ignoring hidden folders and added empty‑FSDB warning - Implemented `fix_missing_scans_reference` helper: - Validates each scan with `_is_scan_dataset` - Updates `files.json` via `_load_scan(..., updates_files_json=True)` - Tracks bad scans, prompts user with `yes_no_abort_choice`, and moves them to trash using `send2trash` - Removed old backup and progress‑bar logic; introduced new interactive deletion flow with clear warnings - Updated imports: added `Logger`, `OptionGroup`, `optgroup`, `send2trash`, and `yes_no_abort_choice`; removed unused `datetime`, `json`, `shutil`, and `tqdm` - Adjusted logger initialization comment and eliminated unnecessary `db.connect()`/`db.disconnect()` calls - Updated documentation strings to reflect new behavior and parameters in `fsdb_healthcheck.py`
- Extend `_is_fsdb` signature with `extra_dirs:list[str]=['configs']` and update docstring. - Skip verification of directories listed in `extra_dirs` during scan dataset checks. - Add `extra_dirs` parameter to `FSDB.__init__` (default `['configs']`) and store it as an instance attribute. - Pass `self.extra_dirs` to `_is_fsdb` in `FSDB.connect` to respect extra directory handling.
- Updated `src/commons/pyproject.toml` to include `click_option_group` in the `dependencies` list.
- No functional code changes; the move improves file organization and readability.
Improve FSDB validation and health‑check CLI with extra roots and safe deletions
…REST API registration - Switch imports from `plantdb.client` to `plantdb.commons` in client modules (`rest_api.py`, `plantdb_client.py`) and tests - Relocate `api_endpoints` to `plantdb.commons`, add missing `home` endpoint and update all usage examples - Rewrite server CLI (`fsdb_rest_api.py`) to import endpoints directly and replace verbose `api.add_resource` calls with a concise mapping helper - Reformat function signatures, logging calls, and type hints for consistency - Update docstrings and examples to reflect the new import path and endpoint structure
…pers - Introduce comprehensive path constants (`HOME`, `HEALTH`, `REFRESH`, `REGISTER`, `LOGIN`, `LOGOUT`, `TOKEN_REFRESH`, `TOKEN_VALIDATION`, `CREATE_API_TOKEN`, `SCANS`, `SCANS_INFO`, `SCAN`, `SCAN_MD`, `SCAN_FILESETS`, `FILESET`, `FILESET_MD`, `FILESET_FILES`, `FILE`, `FILE_MD`, `IMAGE`, `POINTCLOUD`, `MESH`, `SKELETON`, `ARCHIVE`, `FILE_PATH`) in **`src/commons/plantdb/commons/api_endpoints.py`**. - Replace all hard‑coded endpoint strings with the new constants and use `.format(...)` for dynamic segments. - Update endpoint functions: - **`pointcloud`** with size, coords, and type validation. - **`mesh`** with size and coords handling. - **`skeleton`** returning the skeleton path. - **`file_path`** endpoint to use the `FILE_PATH` constant. - Adjust docstrings and examples to reflect the new constant‑based paths and updated signatures.
- Replace all JSON error payloads from `{'message': ...}` to `{'error': ...}` across asset, image, pointcloud, mesh, and zip handling endpoints.
- Add `resource_file` helper to retrieve a `File` object with unified error handling and consistent `error` key.
- Refactor `PointCloud`, `Mesh`, `CurveSkeleton`, and `AnglesAndInternodes` resources to use `resource_file`, removing redundant `fileset_id`/`file_id` parameters and related sanitizers.
- Extend PointCloud endpoint to support a `type` query parameter (`default` or `gt`) and simplify request signature.
- Remove `PointCloudGroundTruth` resource as it is now accessible from `PointCloud`.
- Update logging and success responses to align with the new error key convention.
…ource registration - Replace bulk import of `plantdb.commons.api_endpoints` with explicit constant imports (e.g., `ARCHIVE`, `FILE`, `SCAN_MD`, `SCAN_FILESETS`, etc.) - Reorder typing imports for clarity (`Optional` and `Union` on separate lines) - Simplify `_register_resources` by removing the custom `_add` helper, using a flat `RESOURCE_MAP` with endpoint constants formatted via ``.format`` and registering each with `api.add_resource(..., resource_class_args=(db, logger))` - Update function signatures in `_configure_api`, `_setup_test_database`, and `rest_api` for consistent indentation and type hint style - Adjust import ordering and formatting throughout the file for readability
- Define constructor `def __init__(self, db, logger=None)` with detailed docstring - Store `self.db: FSDB = db` and `self.logger: logging.Logger` (fallback to `get_logger`) - Enables dependency injection of database instance and logger for the home endpoint.
- Replace all JSON error payloads from `{'message': ...}` to `{'error': ...}` across authentication endpoints (register, login, logout, token validation, refresh, and API token creation).
- Consolidate response construction using a single `response` variable with proper status codes and return it at the end of each method.
- Add success response for user registration (`User <username> successfully created`).
- Refactor logout to always return a response variable and use the `error` key for failure cases.
- Simplify token validation flow: early return on missing token and unified error handling.
- Update login to build the successful response in the `else` block and use `error` for failure cases.
- Adjust refresh token and API token creation endpoints to follow the new error-key convention and to return responses consistently.
- Remove unused imports (`requests`, `jsonify`, `make_response`).
- Change health‑check error response key from `message` to `error` and move exception handling before the success `else` block.
- Update `Refresh` resource:
- Return `{'error': ...}` on failure instead of `{'message': ...}`.
- Add explicit `else` block for successful reload response.
- Update full‑database reload endpoint:
- Use `{'error': ...}` for exception case.
- Add `else` block for successful reload message.
- Apply consistent try/except/else pattern across the modified endpoints.
- Move `resource_file`, `is_within_directory`, and `is_directory_in_archive` to new **`plantdb/server/api/utils.py`** - Clean up unused imports in `assets.py` (e.g., `os`, `pathlib`, `BytesIO`, `ZipFile`, `numpy`, `requests`) - Import `resource_file` from utils in `assets.py` - Delete the original helper implementations from `assets.py` to avoid duplication - Keep existing functionality unchanged while improving module organization.
- Replace all `{'message': ...}` responses with `{'error': ...}` in **scan**, **fileset**, **file**, and **assets** endpoints
- Update related docstrings and success response structures to reflect the new error key
- Adjust metadata, metadata‑update, and file‑upload error handling accordingly
- Minor refactor: improve logger initialization formatting and simplify `wants_base64` flag parsing in assets API.
…umentation
- Rename `FILE_PATH` constant to use `/files/{scan_id}/{file_path}`
- Minor comment formatting tweak for lower‑case JSON‑style boolean handling
- Import `ARCHIVE`, `FILE`, `HEALTH`, `IMAGE`, `LOGIN`, `SCAN`, and `SCANS` from `plantdb.commons.api_endpoints` - Replace hard‑coded URL strings with the corresponding formatted constants in all request calls - Adjust request constructions for health check, login, scans retrieval, scan metadata, file serving, image thumbnails, and archive download - Add a temporary print of the retrieved file name (debug aid)
- Updated endpoint hierarchy comment block to reflect new `/auth` and `/assets` routes - Prefixed authentication paths (`/register`, `/login`, `/logout`, token routes) with `/auth` and adjusted corresponding constants (`REGISTER`, `LOGIN`, `LOGOUT`, `TOKEN_REFRESH`, `TOKEN_VALIDATION`, `CREATE_API_TOKEN`) - Moved all static asset routes under `/assets` and updated related constants (`IMAGE`, `POINTCLOUD`, `MESH`, `SEQUENCE`, `SKELETON`, `ARCHIVE`, `FILE_PATH`) - Revised URL building functions to use the new constants, including updated example docstrings for `register`, `login`, `logout`, `token_refresh`, `token_validation`, `create_api_token`, `scans_info`, `scan`, `scan_metadata`, `scan_filesets_list`, `fileset`, `fileset_metadata`, `fileset_files_list`, `file_metadata`, `image`, `sequence`, `pointcloud`, `mesh`, `skeleton`, `archive` - Modified `file_path` helper signature to accept only `file_path` (removed `scan_id` argument) and formatted URL with the new `FILE_PATH` pattern - Adjusted all related import paths and comments to match the new namespace structure.
…t database
- Import `test_database` from `plantdb.commons.test_database` in `src/server/plantdb/server/test_rest_api.py`
- Import `API_PREFIX` from `plantdb.server.test_rest_api`
- Create a test database with `test_database(dataset=None)` before starting the API
- Construct request URLs using `f"{api.get_base_url()}/{API_PREFIX}/scans"` instead of the hard‑coded `/scans` path
- Adjust example usage comments to reflect the new imports and URL construction.
Add TOML configuration API to Scan
Refine logger naming
- Extend `_is_safe_to_delete` signature to `def _is_safe_to_delete(path, db_path) -> bool`. - Resolve both `path` and `db_path` to absolute paths and verify `db_path` is a valid FSDB. - Ensure the deletion target is a sub‑path of the FSDB and not the FSDB root itself; log errors for invalid cases. - Return `True` only after all safety validations succeed. - Update docstring to include `db_path` parameter, revised safety notes, and example usage.
… `file_ops.py` - Replace `_load_scans` example with `_load_scan` usage and add `db.disconnect()` cleanup in doctests. - Append `db.disconnect()` calls to all doctest sections for proper temporary DB teardown. - Introduce new doctest examples for: - `_load_file` - `_load_measures` and `_load_scan_measures` - `_delete_file`, `_delete_fileset`, and `_delete_scan` - `_make_fileset`, `_make_scan`, and `_store_scan` - Enhance delete functions (`_delete_file`, `_delete_fileset`, `_delete_scan`) with `_is_safe_to_delete(path, db.path())` validation to prevent out‑of‑scope deletions. - Update docstrings to incorporate the added examples and safety notes.
- Introduce new test module `src/commons/tests/test_file_ops.py` covering the public helpers in `plantdb.commons.fsdb.file_ops` - Validate loading functions with empty DB, existing scans, and various scan‑fileset‑file scenarios (`_load_scans`, `_load_scan`, `_load_scan_filesets`, `_load_fileset`, `_load_fileset_files`, `_load_file`) - Test measures handling (`_load_measures`, `_load_scan_measures`) including malformed JSON and non‑dict data cases - Ensure deletion safety when a file has no `filename` attribute (`_delete_file`) - Verify directory creation helpers (`_make_fileset`, `_make_scan`) create the expected paths - Provide fixtures `db_with_fileset`, `db_with_file`, and `db_with_fileset_and_file` for isolated dummy database setups.
- Introduce new test module `src/commons/tests/test_validation.py` - Cover `_is_valid_id` with varied inputs and error‑log checks - Test `_is_fsdb` handling of empty DB, non‑directory paths, missing marker, valid/invalid scans, and extra directories - Validate `_is_scan_dataset` for missing metadata, missing or malformed `files.json`, missing `filesets` key, and both with and without fileset validation - Verify `_is_valid_fileset` behavior for missing directory, missing files, and fully valid filesets - Add tests for `_fileset_files_exists` with empty, partially invalid, and mixed file entries - Ensure `_is_safe_to_delete` correctly rejects paths outside the DB, invalid DBs, root DB path, and accepts valid sub‑paths - Use fixtures `empty_db_path`, `db_with_scan`, `db_with_fileset`, and `db_with_file` to provide isolated dummy databases - Include logging assertions to confirm appropriate error messages are emitted.
Refine validation wording and simplify delete logs
- Detect Plant Imager v2 API when `'object'` exists and is non‑empty, extracting `species`, `environment`, and `plant_id`.
- Add fallback for Plant Imager v3 API by reading `scan_md.get('Metadata', {'object': {}})['object']`.
- Map v3 `growth_environment` field to the `environment` metadata key.
- Ensure `plant` metadata is populated from `plant_id` for both API versions.
- Updated inline comments to clarify version‑specific handling.
- Detect `ScanPath` in `scan_md` and compute workspace `x`, `y`, `z` ranges using `center_x`, `center_y` and pipeline bounding box (fallback to `-750` for `z`). - Construct `scan_info["workspace"]` as a dict with ±150 offsets around the center coordinates. - Preserve original fallback to `scan_md['workspace']` or image metadata when `ScanPath` is absent. - Add detailed comments explaining how the workspace is used for centering the plant and a FIXME note for future `position` parameter.
- In `fsdb_healthcheck.py`, set `ROMI_APP_LOGGER` using `__name__.split('.')[-1]` before creating the logger and instantiate `logger` with `os.getenv('ROMI_APP_LOGGER')`.
- Apply the same `ROMI_APP_LOGGER` naming change to `fsdb_import_folder.py`, `fsdb_import_images.py`, `fsdb_import_file.py`, and `shared_fsdb.py`.
- Ensure the environment variable is defined prior to logger creation for consistent logger names across all FSDB CLI commands.
- Set `ROMI_APP_LOGGER` using `__name__.split('.')[-1]` to capture the module’s basename.
- Ensure the logger is instantiated with the updated environment variable for consistent naming across FSDB CLI commands.
- Delete the `fsdb_healthcheck = "plantdb.comons.cli.fsdb_healthcheck:main"` line from `plantdb/src/server/pyproject.toml` to clean up unused script configuration.
Add Plant Imager v3 support to REST API
Standardize CLI logger names
A request to the bare server root (http://host:port/ or the reverse-proxy
root http://host:port/{prefix}/) previously returned 404, because all
resources are mounted under the /api/v1 prefix. Register a lightweight
route at "/" that issues a 302 redirect to the home endpoint, keeping the
deployment (reverse-proxy) prefix in the generated Location so clients
stay under the proxy path.
- fsdb_rest_api.py: add _register_root_redirect() helper that maps the
root path to home(prefix=deploy_prefix) and wire it into rest_api() so
both the fsdb_rest_api CLI and the WSGI entrypoint (wsgi.py) expose the
redirect, honoring the --api-prefix / API_PREFIX value.
- test_rest_api_server.py: add RootRedirectTests covering the no-prefix
and /plantdb-prefix cases (asserting the 302 and exact Location header)
plus a follow-redirect check that lands on the Home payload.
ArthurLuciani2
added
bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
Problem
Two related issues in the PlantDB REST API:
/api/v1prefixing. Endpoint paths and the URLs embedded in server responses were not consistently including the/api/v1mount prefix, and there was no clean separation between the API version prefix and a deployment (reverse-proxy) path prefix.http://host:port/, orhttp://host:port/{prefix}/behind a proxy) returned404, because all resources are mounted under/api/v1.Solution
Introduce two distinct prefix concepts and thread them everywhere:
API_PREFIX = "/api/v1"— always present, shared truth. The app mounts at/api/v1only.deploy_prefix, e.g./plantdb) — a runtime per-server/per-client constant for reverse-proxy deployments, used only to generate external-facing URLs in responses.Server
api_endpoints: the@api_prefixdecorator now composes<deploy_prefix>/api/v1/<endpoint>(with proper sanitization) and theHomeresource builds its endpoint map via these builders.fsdb_rest_api/wsgi: mount at/api/v1always;deploy_prefixreplaces the deprecatedapi_prefixparam; the--api-prefix/API_PREFIXvalue is honored.get_scan_info/get_scan_data) and_get_colmap_camera_modelthread the prefix into every generated URL (archive, thumbnail, filesUri, camera poses). This fixes two bugs where camera-pose URLs and the nestedget_scan_infooutput lost the prefix.302redirect at/to the home endpoint, keeping the deployment prefix in theLocationheader so clients stay under the proxy path.Client
PlantDBClientstoresself.prefixand threads it into everyapi_endpoints.*call.login_url,scan_url,scan_image_url, …) build fully-prefixed URLs;plantdb_url()now returns the server root including the deployment prefix but not/api/v1.Tests
test_api_endpoints.py— deployment-prefix composition across all endpoint builders (78 subtests).test_scan_services.py— prefix threading through scan info/data services.test_rest_api_deploy_prefix.py— server responses embed the deployment prefix (Homemap,ScansTable,Scan).test_rest_api_server.py— root-redirect tests (no-prefix and/plantdbcases, exactLocation, follow-redirect).test_rest_api.py— updated client URL assertions for/api/v1and the rootplantdb_urlbehavior.Notes
test_server_availability()now passes via the new root redirect (GET/→ 302 →/api/v1/).knowledge/, test data, caches) are local artifacts not part of this PR.