[WIP]: View all included files

[nitrajka]

Purpose

Hi, I would like to contribute to Syncthing. 😃
I found an enhancement issue #4729. I suppose no one is working on it because it is inactive since 2018. I tried to implement it.

Testing

I suppose, It would be nice to display not only some plain list of files (as I implemented firstly)

but some nice folder tree structure.

What do you think?

I think a nested folder structure in JSON format would be useful when displaying the folder tree view in Angularjs. I wrote a function in Go which converts a list of files (and trims the paths from the Sync base folder path) into such a nested structure. I wrote tests to test it as well. Tests are in lib/stats/folder_test.go.
But the (frontend) Angularjs part is not ready, yet.
I already started to implement this, but got stuck on creating a new directive in angular. I've never written anything in Anlgularjs, therefore I need to read the documentation first.
Are you okay if I finish this PR? What do you think about the file tree view?

Screenshots

I added a new item in folder statistics view. (Of course, I also add translations later.)

After clicking on show, a popup appears and should show a nice tree structure.
For now, I send a file tree structure in JSON format.

I hope you can bear with me one or two PRs. 😃

Authorship

Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.

[golangcibot]

Error return value of filepath.Walk is not checked (from errcheck)

[golangcibot]

S1021: should merge variable declaration with assignment on next line (from gosimple)

[AudriusButkevicius]

So I think the UI part (tree view etc) is the harder part of this ticket. Implementing the logic itself to return files should not be too hard.
We should also probably make the loading logic of the tree lazy, so that each directory has to create a request.

I can see this PR uses FolderStats stuff, but that's meant for completely different purpose, which is persistence of metrics

[nitrajka]

We should also probably make the loading logic of the tree lazy, so that each directory has to create a request.

That means to create new route in the api, right?

each directory

Do you also mean every subdirectory in our syncing directory?

I can see this PR uses FolderStats stuff, but that's meant for completely different purpose, which is persistence of metrics

I am sorry, I didn't know. I read the whole documentation (hope carefully enough), but didn't notice anything about FolderStats purpose.

[AudriusButkevicius]

I think there already exists an endpoint for what you are trying to do. We'd probably make a request for every subdirectory, as people can have millions of files, and returning everything in a simple response might be too expensive.

[nitrajka]

I think there already exists an endpoint for what you are trying to do.

You are right. I assumed GET /rest/db/browse would be suitable for this. We can even specify the depth of the tree to be returned and prefix of the path (which can point to some directory inside the sync root directory) at which we want to return the file tree. Okay, let's give it a try. Thank you 😃

[AudriusButkevicius]

We might need to modify that endpoint, as I think what it returns is really bad (maps of stuff).

I don't think anyone uses that. so it should be ok.

It should return:

[
 { name: a
 { children: [
      { name: b
      { children: [
           { name: c },
           { name: d }
       ]}
  ]}
]

so it's array based, rather than map based.

[nitrajka]

This function iterates over every file or folder in the given part of the file system and calls fn(f) on its path (which is of type string).:
https://github.com/syncthing/syncthing/blob/master/lib/db/instance.go#L281

That means, that the anonymous function defined in /lib/model/model.go:
https://github.com/syncthing/syncthing/blob/master/lib/model/model.go#L2349
may get a file's path, which already partially exists in the output structure.
For example, we may have /folder1/file1 in our root. The anonymous function would be called twice. Once with /folder1 and once with /folder1/file1.

Then we check every folder on a particular path (e.g for path '/folder1/file1' we split it and check folder1 and file1), whether those folders already exist in the output structure:
https://github.com/syncthing/syncthing/blob/master/lib/model/model.go#L2374
If we had an array-based output, this check would take O(n) time (because we would have to go through an array of a folder's children), which would be in total O(n^2) time with the outer loop. That seems inefficient to me.

Also, converting the output structure before returning it to an array-based structure seems rather unprofessional to me.
I also don't want to modify withGlobal (to return something more helpful) because it is used in other functions as well (mostly in /lib/db/set.go).
It seems, that this part of the code was designed to work exactly with map-based output to be efficient enough...

Is it really necessary to change the output's type? I understand it would look nicer with arrays, but as far as I understand the code, I don't see more efficient solution than convert the output structure before returning it. :/ Do you have a different opinion?

[AudriusButkevicius]

I don't think you need to modify withGlobal. WithGlobal iterates in a sorted fashion, so /folder/ will always be before /folder/file.

You simply need to have a stack of directories as you traverse in and out of them, and your children will always be in the top directory of the stack.