Skip to content

Conversation

@xtophe38
Copy link
Contributor

@xtophe38 xtophe38 commented Jan 2, 2025

Base information

Question Answer
Related to a SourceForge thread / Another PR / Combodo ticket? https://support.combodo.com/pages/UI.php?operation=details&class=Bug&id=7783
Type of change? Bug fix

Symptom (bug) / Objective (enhancement)

The json collector included in itop-data-collector-base may get its source information from a json file located on the server hosting the collector. In such case, the parameter jsonfile defines the name of the file to be used by the collector together with its path to access it. The path may be absolute or relative to the directory where the collector is deployed (let's call it ).

When a relative path is used to reference the json file (for instance data/myjsonfile.json) and when the collector is not launched from the directory , the json collector cannot locate the json file, logs the following error and stops.

Error:
PHP Warning: file_get_contents(data/myjsonfile.json): Failed to open stream: No such file or directory in /my-root-dir/my-collector/core/jsoncollector.class.inc.php on line xyz
...
[2025-01-02 13:31:26] [Error] [MyJsonCollector] Failed to get JSON file:
[2025-01-02 13:31:26] [Error] MyJsonCollector::Prepare() returned false

When can live with the bug by always specifying absolute paths in the configuration file. This implies that specific params.local.xml file must be provided for phpunit tests. The PR get rid of this requirement.

Reproduction procedure (bug)

  1. With itop-data-collector-base 1.4.0
  2. With PHP 8.3.15
  3. Deploy a collector base under a working directory that we'll name
  4. Create a json collector. Don't specify any jsonurl parameter but specify a jsonfile one using a relative path like 'data/myjsonfile'.
  5. Run the collector from , everything will work fine.
  6. If you run it from another directory, the collector will break.

Cause (bug)

The json collector doesn't handle the case where the source file may be defined with a relative path. See line 153 of core/jsoncollector.class.inc.php:

Proposed solution (bug and enhancement)

Add a test after the first read of the jsonfile. If it fails, retry with the APPROOT constant prepended to it.

Checklist before requesting a review

  • I have performed a self-review of my code, and that it's compliant with Combodo's guidelines
  • I have tested all changes I made on an iTop instance
  • I have added a unit test, otherwise I have explained why I couldn't
  • I have made sure the PR is clear and detailled enough so anyone can understand the real purpose without digging in the code

Checklist of things to do before PR is ready to merge

@xtophe38 xtophe38 requested review from accognet and odain-cbd January 2, 2025 15:43
@accognet accognet added enhancement New feature or request internal Work made by Combodo labels Jan 2, 2025
@xtophe38
Copy link
Contributor Author

xtophe38 commented Jan 3, 2025

Note: as you can see, the 2 provider functions of the JsonCollectorTest class have been made static. This is to comply with the requirements of the latest phpunit versions. I'm using v 10.5.29.

@xtophe38 xtophe38 merged commit e46a02b into master Jan 6, 2025
2 checks passed
@xtophe38 xtophe38 deleted the issue/7783 branch January 6, 2025 14:04
@xtophe38 xtophe38 restored the issue/7783 branch January 6, 2025 15:19
odain-cbd pushed a commit that referenced this pull request Apr 23, 2025
…hed from its home directory (#55)

* Make sure relative paths can be used when json collector is launched from outside its root directory

* Add test for jsonfile parameter defined with relative path

* Provider function must be static for PHP 8.2+
Hipska added a commit to Super-Visions/itop-data-collector-base that referenced this pull request Apr 25, 2025
* master: (32 commits)
  Fix default date_format (Combodo#54)
  Update jsoncollector.class.inc.php
  N°7783 - Relative paths cannot beused when the collector is not launched from its home directory (Combodo#55)
  Create action.yml
  Update composer.json
  Fix N°7338 to actually use "itop_login"
  N°7506 - Add method to check if a specific module is installed in iTop (Combodo#39)
  N°7234 - Add the possibility of mapping the same CSV column twice or more (Combodo#46)
  N°7507 - Bump php/phpunit libraries (Combodo#51)
  Fix test
  Revert phpunit version
  ✅ Fix unit test
  ✅ Format provider text
  N°6115 - missing notify_contact_id_archive_flag in list of read-only fields (Combodo#32)
  N°3709 - Add the possibility to use an other date format than the default one (Combodo#45)
  N°7483 - Fix lookup table ignore error not ignored (Combodo#49)
  N°7482 - Ensure to dump source definition only when on debug (Combodo#48)
  N°7481 - Fix PHP requirement not taken into account (Combodo#36)
  N°7339 - Improve error handling when user has no permission to write in data directory (Combodo#47)
  N°7338 - Use "itop_login" as fallback value if no "synchro_user" defined in parameters (Combodo#43)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request internal Work made by Combodo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants