[🚀 Feature]: List all downloaded files in a session (Grid)

[diemol]

Feature and motivation

#11443 is a proposal to implement file download on the client bindings. However, to download a file you need to know the file name, and this might be tricky given that most of the time the application under test does not let you choose the name that will be used when the file is downloaded.

We should have an endpoint that lists the downloaded files in the configured download directory in Grid. Then the user can choose the name of the one they want to download.

Usage example

Something along the lines of:

/session/:sessionId:/se/file

Which could return:

[
	{
		"name":"input.txt",
		"timestamp": "<timestamp with creation date"
	},
	{
		"name":"anotherFile.txt",
		"timestamp": "<timestamp with creation date"
	}
]

But I am not 100% that is the W3C WebDriver response payload structure, we should adapt to that one. @titusfortner do you know?

The main challenge in this issue is to decide if we need to list files that belong to a session or if it is OK to list all downloaded files.

[diemol]

It would be ideal to list only files that have been downloaded in the current session. How can that be achieved without adding brittle magic to the code that can become unmaintainable?

Let's keep in mind that:

• The Node knows that a file will be downloaded, it just sees a sendKeys or a click on a button.
• The browser is the one who decides where the file will be downloaded, so thinking about creating sub-directories for each path is tricky, it would need to be decided during session creation and the sub-directory needs to exist.
• Maybe injecting browser preferences to set a download location is an option for Firefox and Chromium based browsers. But this is not possible for Safari. Ideally, this needs to be cross browser. If we go this way, it needs to be injected during session start, and when the session starts, we do not know if a file will be downloaded.

[titusfortner]

Yeah, it's all in the Processing Model part of the spec:

Let response’s body be the UTF-8 encoded JSON serialization of a JSON Object with a key "value" set to data.

Just need to nest everything under "value" key.

Example return from GET session/b97f558144a5cda150b18e6d5c3a7d32/cookie:

{"value":[{"domain":"0.0.0.0","httpOnly":false,"name":"foo","path":"/","secure":false,"value":"bar"},"domain":"0.0.0.0","httpOnly":false,"name":"monster","path":"/set_cookie","secure":false,"value":"1"}]}

[titusfortner]

decide if we need to list files that belong to a session or if it is OK to list all downloaded files

I don't think this feature is even worth doing if we can't split out by session.

[titusfortner]

As I mentioned in #11466, I think for file downloads we should be using /session/:sessionId:/se/files instead of /session/:sessionId:/se/file

[titusfortner]

I'm saying I think we need to change the implementation to do things that way.

[titusfortner]

Let's agree if this is what we want to implement here:

GET /session/:sessionId:/se/files

return this:

{"value":
   [
	{
		"name":"input.txt",
		"timestamp": "<creation timestamp>"
	},
	{
		"name":"anotherFile.txt",
		"timestamp": "<creation timestamp>"
	}
   ]
}  

[krmahadevan]

@titusfortner - I dont have any specific opinion regarding the timestamp.
I do see it being useful when the user has no control on how to specify the file names and so the same file gets downloaded with maybe ( 1 ), ( 2 ) suffixes in file names. So the user should be able to figure out what was the latest file that was downloaded for their session by looking at the timestamp.

Incase we do agree to add timestamps (since @diemol proposed it, am assuming he agrees, and I am fine with it as well, are we looking at a buy-in from anyone else apart from you ?) I would just like the timestamp to be an epoch value instead of a string formatted timestamp (since I would like us not to deal with timezones etc when the grid is in a different timezone). That way the user can convert the epoch time into a timestamp with a timezone of their preference.

Please let me know and i will update my PR to include the timestamp changes

[titusfortner]

The more I think about it, the less I think it is useful to add timestamps. So I think we should avoid them, unless @diemol feels they are important.

[krmahadevan]

@titusfortner While @diemol gets back with his inputs.. trying to pick your brain.. What is your rationale behind not having timestamps. I mean what do you see as hindrances/pitfalls by including them.

[titusfortner]

I don't think they provide much additional value, and it's a lot easier to parse a list than a list of hashes to get the item you want to download or to iterate, etc.

[diemol]

That was just an example payload, timestamps are not a must on the return payload.

[krmahadevan]

@diemol - Since this is already built as part of #11646 Can we close this issue ?

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.