Ericsson / CodeCompass

CodeCompass is a software comprehension tool for large scale software written in C/C++ and Java
https://codecompass.net
GNU General Public License v3.0
520 stars 102 forks source link

New frontend remaining issues #615

Closed mdeme01 closed 1 year ago

mdeme01 commented 1 year ago

This PR aims to fix the remaning issues related to the new frontend. @mcserep @intjftw

Issues fixed:

mcserep commented 1 year ago

@mdeme01 Thanks for the quick work, we will review it! For the remaining issues, please create a PR / issue, so the review process is simplified. (The changeset is smaller and it is more clear what was changed and for what reason.)

intjftw commented 1 year ago
mdeme01 commented 1 year ago
  • Metrics are not showing now at all, I get this error in the dev console if I click on metrics for any directory

This problem occurs because you did not specify an 'src' label that is the path to the source directory in the codebase of the project. Because all of the projects had this label on the demo website, I thought this label was mandatory for every project, apparently, it is not.

I used the 'src' label for the Metrics, because if you click the root directory, or any directory above the source folder, it still generates the Metrics for the source folder, as seen on the demo website, and I took examples from there during development.

The problem is that generating metrics when clicking a sub-folder (represented by a rectangle in the treemap) is dependant on the path of the folder, because it works by appending/removing the folder name to the path. And if you generate the metrics for anything outside the source folder, the result will still be the metrics for the source folder, and the path will be out of sync.

For example, generating it for the root folder /, will result in the metrics of /projects/CodeCompass, and the path will still be /. Now if I click on a sub-folder, e.g. webgui, the input for the metrics will be /webgui, but it should be /projects/CodeCompass/webgui.

So this is why, when clicking a directory outside the source folder, I appended the source folder path to the current path (stored in the 'src' label), because I did not know how else to solve this. (And I could not figure out how this works in the old frontend.)

mcserep commented 1 year ago

This problem occurs because you did not specify an 'src' label that is the path to the source directory in the codebase of the project. Because all of the projects had this label on the demo website, I thought this label was mandatory for every project, apparently, it is not.

Unfortunately the existence of the src label is not mandatory, as discussed in #613 previously.

I used the 'src' label for the Metrics, because if you click the root directory, or any directory above the source folder, it still generates the Metrics for the source folder, as seen on the demo website, and I took examples from there during development.

The problem is that generating metrics when clicking a sub-folder (represented by a rectangle in the treemap) is dependant on the path of the folder, because it works by appending/removing the folder name to the path. And if you generate the metrics for anything outside the source folder, the result will still be the metrics for the source folder, and the path will be out of sync.

For example, generating it for the root folder /, will result in the metrics of /projects/CodeCompass, and the path will still be /. Now if I click on a sub-folder, e.g. webgui, the input for the metrics will be /webgui, but it should be /projects/CodeCompass/webgui.

So this is why, when clicking a directory outside the source folder, I appended the source folder path to the current path (stored in the 'src' label), because I did not know how else to solve this. (And I could not figure out how this works in the old frontend.)

To generate the metrics for a file or a directory:

  1. First you should fetch the fileID based on the path. This could be done with the getFileInfoByPath() endpoint in the projectservice.
  2. Then the getMetrics() in metricsservice will expect a file ID as a parameter.

This is how it is done in the current UI.

mcserep commented 1 year ago

613 (Make labels dynamic on New Frontend): the labels are recognized and displayed correctly on the new UI, but clicking them raises an error in the console log:

Invalid path: /home/mate/cc/sources/tinyxml2/

The path is valid and correct on my dev environment, the label also works on the old UI correctly.


UPDATE: I got the same error when checking the metrics, as you have mentioned it depends on it.

mdeme01 commented 1 year ago

To generate the metrics for a file or a directory:

First you should fetch the fileID based on the path. This could be done with the getFileInfoByPath() endpoint in the projectservice. Then the getMetrics() in metricsservice will expect a file ID as a parameter. This is how it is done in the current UI.

Yes, I know that. I explained the problem above. I'm sorry if it wasn't clear, I will try to explain it again.

If you click a directory for metrics, you need its ID, this much is correct and clear, and it works that way correctly. However, after the metrics has been generated, you need to be able to generate metrics for other directories, displayed in the treemap. The treemap is generated from the metrics result, which only contains the names of the directories, not their path. You should be able to generate metrics for a directory by clicking on it inside the treemap.

This operation requires a new path, for a new metrics, but you can only work with the name of the clicked directory, you don't know its path. But you have the path of the directory you initially generated the metrics for, so you can just append the name of the directory to this path.

For example: you generated metrics for the CodeCompass folder, its path is /projects/CodeCompass, which is a fully valid path. The treemap will contain the webgui directory, represented by a rectangle. Now you want to generate metrics for this directory, and you can do this by appending webgui to the current path (/projects/CodeCompass), and the new path will be /projects/CodeCompass/webgui, which is valid, thus the new metrics can be generated correctly.

The problem comes in when you generate metrics for anything outside the source folder, like the / folder, or, /projects. Appending the names of the directories will not work here, because the paths will be invalid. Using the same example, you want to generate metrics for webgui, the current path is /, and appending will result in /webgui, which is an invalid path, and the metrics generation will fail.

For some reason getMetrics returns the result of the source directory for directories outside the source folder. And that is why, the current path and the result will be out of sync. So I needed to handle this by using the src label: I used the source path for directories outside the source folder. What I do not know, is how this is handled in the old frontend, without using the src label.

mdeme01 commented 1 year ago

The labels are recognized and displayed correctly on the new UI, but clicking them raises an error in the console log

I checked it, for me it works correctly. I used the cJSON project and specified 3 labels.

I used the following command: (In my case, the project is located in /home/deme/CodeCompass/projects)

./CodeCompass_parser \
-d "sqlite:database=~/CodeCompass/database/cjson.sqlite" \
-w ~/CodeCompass/test_ws/ \
-n cJSON/ \
-i ~/CodeCompass/projects/parsed/cjson/compile_commands.json \
-i ~/CodeCompass/projects/cjson/ \
-j 4 \
--label src=/home/deme/CodeCompass/projects/cjson \
--label tests=/home/deme/CodeCompass/projects/cjson/tests \
--label fuzzing_inputs=/home/deme/CodeCompass/projects/cjson/fuzzing/inputs

All 3 labels are displayed and clicking them navigates to the correct directory. Are you sure the path you specified is correct for the that project?

mcserep commented 1 year ago

Are you sure the path you specified is correct for the that project?

Yes, it works on the old UI as well. I will try to debug this later.

mcserep commented 1 year ago

Yes, it works on the old UI as well. I will try to debug this later.

@mdeme01 I have debugged this and the source of the error is that in the DB the path is: /home/mate/cc/sources/tinyxml2 while the configured label is: /home/mate/cc/sources/tinyxml2/

Note the trailing slash. Therefore ProjectServiceHandler::getFileInfoByPath returns that the path was not found. In the old UI it works regardless of the trailing slash, so it could be checked how it is done there.

mcserep commented 1 year ago

Yes, I know that. I explained the problem above. I'm sorry if it wasn't clear, I will try to explain it again.

If you click a directory for metrics, you need its ID, this much is correct and clear, and it works that way correctly. However, after the metrics has been generated, you need to be able to generate metrics for other directories, displayed in the treemap. The treemap is generated from the metrics result, which only contains the names of the directories, not their path. You should be able to generate metrics for a directory by clicking on it inside the treemap.

This operation requires a new path, for a new metrics, but you can only work with the name of the clicked directory, you don't know its path. But you have the path of the directory you initially generated the metrics for, so you can just append the name of the directory to this path.

For example: you generated metrics for the CodeCompass folder, its path is /projects/CodeCompass, which is a fully valid path. The treemap will contain the webgui directory, represented by a rectangle. Now you want to generate metrics for this directory, and you can do this by appending webgui to the current path (/projects/CodeCompass), and the new path will be /projects/CodeCompass/webgui, which is valid, thus the new metrics can be generated correctly.

The problem comes in when you generate metrics for anything outside the source folder, like the / folder, or, /projects. Appending the names of the directories will not work here, because the paths will be invalid. Using the same example, you want to generate metrics for webgui, the current path is /, and appending will result in /webgui, which is an invalid path, and the metrics generation will fail.

For some reason getMetrics returns the result of the source directory for directories outside the source folder. And that is why, the current path and the result will be out of sync. So I needed to handle this by using the src label: I used the source path for directories outside the source folder. What I do not know, is how this is handled in the old frontend, without using the src label.

The getMetrics returns the metrics for the files in the specified directory. The result contains the absolute path for the files (as nested JSON objects), therefore I do not really get why we would append webgui to / in your example.

The old UI simply does a smart move and if the metrics for / folder was requested, but all returned metrics are for files in the /projects/CodeCompass folder, then in the visualized treemap the /projects/CodeCompass becomes the top folder.

mdeme01 commented 1 year ago

The result contains the absolute path for the files

Okay, I misunderstood it, and I didn't realize it when I first implemented it. Thanks for clarifying. Now I think I was able to fix it, at least it is working for me, so it does not require the src label anymore.

mcserep commented 1 year ago

Okay, I misunderstood it, and I didn't realize it when I first implemented it. Thanks for clarifying. Now I think I was able to fix it, at least it is working for me, so it does not require the src label anymore.

Thanks for the update and the fix, it works now for me as well similarly like in the old UI.

@mdeme01 I have debugged this and the source of the error is that in the DB the path is: /home/mate/cc/sources/tinyxml2 while the configured label is: /home/mate/cc/sources/tinyxml2/

Note the trailing slash. Therefore ProjectServiceHandler::getFileInfoByPath returns that the path was not found. In the old UI it works regardless of the trailing slash, so it could be checked how it is done there.

This is the only remaining issue I see before merging this PR.

mdeme01 commented 1 year ago

Note the trailing slash. Therefore ProjectServiceHandler::getFileInfoByPath returns that the path was not found. In the old UI it works regardless of the trailing slash, so it could be checked how it is done there.

I didn't explicitly find out how this is handled in the old UI. However, I added a check for the trailing slash, and if the path contains one, it passes the path without it to the request, and this seems to be working.

mcserep commented 1 year ago

I didn't explicitly find out how this is handled in the old UI. However, I added a check for the trailing slash, and if the path contains one, it passes the path without it to the request, and this seems to be working.

The old UI processes the path by elements, splitting it by the directory separators. See in fileManager.js:

      var path = shortcut.split('/');
      path[0] = '/';

      path.forEach(function (directory) {
        if (!directory.length) // <-- this ignores the last element if empty
          return;

        // ... 
      });

Since it is not required in the new UI, a simple check as you have implemented should be fine.