feat(files): file interactions. find, findone

[jhoward1994]

What does it do?

It adds a new FilesManager class that provides methods to retrieve files uploaded to Strapi. The implementation includes:

1. A FilesManager class with two primary methods:

   • find(): Retrieves a list of files with optional filtering and sorting
   • findOne(): Retrieves a single file by its ID

2. Type definitions for file responses and query parameters:

   • FileResponse interface for individual file data
   • FileListResponse type for handling lists of files
   • FileQueryParams interface for filtering and sorting options

3. Robust validation for file queries and file IDs:

   • validateFileQueryParams(): Ensures proper format for filters and sorting
   • validateFileId(): Verifies that file IDs are valid positive integers

4. Integration with the main Strapi client:

   • Exposed via a convenient files getter property on the Strapi client

Why is it needed?

Previously, there was no standardized way to interact with files in the Strapi Media Library through the client.

How to test it?

1. Environment setup:

   • Ensure you have a Strapi instance running with the upload plugin enabled
   • Configure the client with the correct baseURL: const client = new Strapi({ baseURL: 'http://localhost:1337/api' })

2. Testing file list retrieval:

   // Retrieve all files
   const allFiles = await client.files.find();
   
   // Retrieve only image files
   const imageFiles = await client.files.find({
     filters: { mime: { $contains: 'image' } }
   });
   
   // Retrieve files sorted by name
   const sortedFiles = await client.files.find({
     sort: 'name:asc'
   });

3. Testing single file retrieval:

   // Retrieve a file by ID
   const file = await client.files.findOne(1);
   console.log(file.name, file.url, file.mime);

4. Verify error handling:

   // Should throw validation error
   try {
     await client.files.findOne(-1);
   } catch (error) {
     console.error(error); // Should show "Invalid file ID" error
   }
   
   // Should throw error for non-existent file
   try {
     await client.files.findOne(9999);
   } catch (error) {
     console.error(error); // Should show "File not found" error
   }

Related issue(s)/PR(s)

DX-1822

[innerdvations]

Just took a quick look, overall looks good, will test it out tomorrow!

[Convly]

Overall the main logic looks very good to me, some comments in the tests and error handling.

My main, biggest concern is with the terminology: When I'm using client.files.<>, I'm expecting to interact with actual files, not files' info/metadata.

Do we want to maybe rethink the methods' naming (the namespace iself seems fine if we want to add actual files method later on)? IThe thing is I'm not sure I can think of better replacements...

[jhoward1994]

@Convly I've made the changes - I think the only 2 comments left are

#63 (comment)
If you could describe how you see the file examples working?

and #63 (comment)
I matched the file manager implementation closer to e.g. src/content-types/single/manager.ts
So error handling is now the responsibility of src/http/client.ts
Is this as you expect?

[Convly]

I kinda liked the idea of handling and remapping some HTTP errors with domain logic (e.g. 404 -> file with id {id} not found) or other errors).

My comment was mainly about how it's checking for the actual errors + which one it re-throws rather than the idea itself.

I don't know if it's clear though, but basically

if (error instanceof HTTPNotFoundError) {
  throw new FileNotFoundError(fileId, error) // passing the original error to set an origin/source and be able to access the actual request/response objects
}

Or even better, cloning the HTTP client and adding an interceptor (so that it can be re-used by all the methods, it's even possible to add a getter to automatically apply everything and make it available for each method. Very simplified but smth like this

const mapFileErrors = ({ response }) => {
  if (error instanceof HTTPNotFoundError) {
    throw new FileNotFoundError(fileId, error) // passing the original error to set an origin/source and be able to access the actual request/response objects
  }

  return { response };
}

const client = this._httpClient.create();

client.interceptors.response.use(mapFileErrors);

const response = await client.get(url);
const json = await response.json();

What do you think about those options?

[Convly]

We're getting there! Final super minor changes and let's merge 🙌

Also, it seems like you forgot to update the generated types for the demo Strapi app.

diff --git a/demo/.strapi-app/types/generated/contentTypes.d.ts b/demo/.strapi-app/types/generated/contentTypes.d.ts
index 4a2f513..aa0a2e9 100644
--- a/demo/.strapi-app/types/generated/contentTypes.d.ts
+++ b/demo/.strapi-app/types/generated/contentTypes.d.ts
@@ -441,6 +441,7 @@ export interface ApiCategoryCategory extends Struct.CollectionTypeSchema {
     createdAt: Schema.Attribute.DateTime;
     createdBy: Schema.Attribute.Relation<'oneToOne', 'admin::user'> & Schema.Attribute.Private;
     description: Schema.Attribute.Text;
+    image: Schema.Attribute.Media<'images'>;
     locale: Schema.Attribute.String & Schema.Attribute.Private;
     localizations: Schema.Attribute.Relation<'oneToMany', 'api::category.category'> &
       Schema.Attribute.Private;

[Convly]

You can create this outside the interceptor to avoid too many instantiations

[Convly]

Suggested change

	({ request, response }) => {
	return { request, response };
	},
	undefined,

[Convly]

If the interceptor isn't doing anything, you can just pass undefined instead

[Convly]

Suggested change

	return null;
	return error;

[Convly]

mapping an error to a null value seems counter-intuitive upon usage, maybe we could return the original error untouched if it's not smth the file error mapper is not interested in?

This way we can always treat the result as an Error instance no matter what

What do you think? (I've included the proposed change in the usage too)

[Convly]

Suggested change

	return (error: Error): Error | null => {
	return (error: Error): Error => {

[Convly]

Suggested change

	const mappedError = mapper(error as Error);
	if (mappedError) {
	throw mappedError;
	}
	// For other errors, rethrow the original error
	throw error;
	if (error instanceof Error) {
	throw mapper(error);
	}
	// For other errors types, re-throw the original value
	throw error;

[Convly]

This will return and verify that the interceptors' manager (to add a new one) exists, but I'm not sure what else it's supposed to check? Should we instead make sure it contains the actual interceptor?

Thing is you'll need to access the private _handlers property to do that 🤔