Skip to content

[#841] fix wrong column names in the DataAccess model - #842

Open
rudra337-lgtm wants to merge 1 commit into
irods:mainfrom
rudra337-lgtm:fix/841-dataaccess-colnames
Open

rudra337-lgtm wants to merge 1 commit into
irods:mainfrom
rudra337-lgtm:fix/841-dataaccess-colnames

Conversation

@rudra337-lgtm

Copy link
Copy Markdown

Fixes #841

Problem

In irods/models.py, columns 701–704 of the DataAccess model carried their icat_key strings with the COL_ prefix:

type             = Column(Integer, "DATA_ACCESS_TYPE",       700)  # already without prefix
name             = Column(String,  "COL_DATA_ACCESS_NAME",   701)
token_namespace  = Column(String,  "COL_DATA_TOKEN_NAMESPACE", 702)
user_id          = Column(Integer, "COL_DATA_ACCESS_USER_ID",  703)
data_id          = Column(Integer, "COL_DATA_ACCESS_DATA_ID",  704)

The COL_ prefix belongs only to the #define/C-identifier names in the iRODS server sources, not to the genquery1 attribute strings. lib/core/include/irods/rodsGenQueryNames.h of irods/irods maps COL_DATA_ACCESS_NAME → "DATA_ACCESS_NAME", COL_DATA_TOKEN_NAMESPACE → "DATA_TOKEN_NAMESPACE", COL_DATA_ACCESS_USER_ID → "DATA_ACCESS_USER_ID", and COL_DATA_ACCESS_DATA_ID → "DATA_ACCESS_DATA_ID" — i.e. exactly the strings DataAccess.type (700) already used, and attributing strings that carry no COL_ prefix. Querying with COL_DATA_ACCESS_NAME was reported in #841 to fail server-side ("no column COL_DATA_ACCESS_NAME found") in some uses, and the mismatched names also surface on the client in criteria (Criterion('=', DataAccess.name, 'own')) and in the keys of result rows, which irods/results.py builds from ModelBase.columns()[attriInx].icat_key (the KeyError: <Column 701 COL_DATA_ACCESS_NAME> observed in the issue thread).

Fix

Strip the COL_ prefix from the four column-name strings in the DataAccess model, as agreed by the maintainers in the issue thread ("just change the internal column_names by stripping off 'COL_' for this milestone"):

  • 701: "DATA_ACCESS_NAME"
  • 702: "DATA_TOKEN_NAMESPACE"
  • 703: "DATA_ACCESS_USER_ID"
  • 704: "DATA_ACCESS_DATA_ID"

Per the same discussion, the column type (Integer→String) change for user_id (the upper(bigint) error in case-insensitive queries) is deliberately out of scope here and belongs to the 4.0 milestone.

Note: CollectionAccess (710–714, "COL_COLL_ACCESS_*") shows the same family of inconsistency (COLL_ACCESS_* on the server side); left untouched here, since this PR stays scoped to the DataAccess model named in the issue. Happy to follow the same change there if maintainers agree.

Tests

New offline regression tests in irods/test/modules/test_dataaccess_column_names__issue_841.py (no iRODS server needed; the genquery request is built client-side and result-key mapping reproduced via empty_gen_query_out + ResultSet):

  • test_dataaccess_request_columns_use_irods_attribute_names
  • test_criteria_built_from_dataaccess_columns_use_irods_attribute_names
  • test_result_keys_surface_irods_attribute_names
  • test_no_dataaccess_column_name_carries_the_col_prefix

All four fail on current main (verified: 'COL_DATA_ACCESS_NAME' == 'DATA_ACCESS_NAME' assertion errors) and pass with this change. Registered names are pinned to the values in rodsGenQueryNames.h quoted above.

Locally (Windows, no iRODS server available):

  • python -m pytest irods/test/modules -q: 4 passed (this PR) / 1 failed + 3 errors raised — the failures are test_auto_close_of_data_objects__issue_456 (needs a live server) and fixture-driven modules run by the repo's own harness, identical counts with the change stashed (pre-existing, unrelated).
  • python -m pytest irods/test/test_paths.py -q: 1 passed.
  • ruff check: 0 findings on the new test file; irods/models.py findings byte-identical before/after the change (39 pre-existing, proven by stashing).
  • mypy --ignore-missing-imports on the touched sources: identical 2 pre-existing errors in untouched import-closure files before/after (proven by stashing). No new type errors.
  • The Docker-based suite against a live iRODS server was not runnable on this host; the focused offline suite above covers the changed surface.

The icat_key strings of the DataAccess columns 701-704 carried the
"COL_" prefix, inconsistent with DataAccess.type (700: DATA_ACCESS_TYPE)
and with the iRODS genquery1 attribute strings defined in
lib/core/include/irods/rodsGenQueryNames.h of the iRODS server
(DATA_ACCESS_NAME, DATA_TOKEN_NAMESPACE, DATA_ACCESS_USER_ID,
DATA_ACCESS_DATA_ID - the COL_ prefix belongs only to the "#" define
identifiers, not to the attribute strings).  The mismatch was reported
to surface in client-side criteria and result row lookups
(Criterion('=', DataAccess.name, 'own'); row[DataAccess.name /
DataAccess.user_id]) as described in issue irods#841.

Signed-off-by: Rudra <rudramandaliya337@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

bug in DataAccess model - wrong column names

1 participant