coldbox-modules / cbfs Goto Github PK

When working with CBFS in it’s current form, you are constantly referencing the storage object and passing around a file path to all the methods.

For example:

var storage = cbfs().get( "local" );
var file = storage.create( "path/to/file.txt", "some contents" );

storage.append( "path/to/file.txt", "more contents" );
storage.copy( "path/to/file.txt", "some_other_file.txt" );

Introducing a File object would clean up the API calls considerably and wouldn’t require much to refactor if we simply proxy the calls to the disk.

// create() would return a File Object
var storage = cbfs().get( "local" );
var file = storage
  .create( "path/to/file.txt", "contents" )
  .append( "more contents" )
  .copy( "some_other_file.txt" );
  
// Other methods become easier also
file.size();
file.getAsBinary();
file.delete();

This should be directoryPath and not filePath to make this test have sense. Or am I missing something?

When calling .info() on the Local or RAM provider, you are returned a 'type' which indicates if the path is a file or a directory.

var info = disk.info( "some_file.txt" );
expect( info.type ).toBe( "file" );

On the S3 provider though this fails because it is instead returning the Content-Type of the file, which in this case would be 'text/plain'.

Calling .info() should return consistent values for all providers.

I'm trying to install this trough commandbox, but always got this message:

 Aww man,  forgebox isn't feeling well.
 Error getting ForgeBox entry [cbfs]  The entry slug sent is invalid or do
 es not exist

Is this published or not?

The S3Provider has a method called url, whereas that same method is uri in the LocalProvider. Laravel uses url as the method name - because it can return either a relative URI path ( local ) or a URL if the resource is remote. I would suggest changing LocalProvider to use url.

In addition, the url method in S3Provider is always delivering a presigned URL back. If the bucket object is public, then the URL should not be pre-signed so that browser caching of the object can be leveraged.

Suggest adding a caching layer for testing if the object is public. This way, if an object is requested frequently, the results to check S3 permissions on the object will not have to be made on each request to the URL

Edit: Looking again now the most recent changes up on Github ( the build is failing ) are showing both a url and uri method in S3Provider. So my concern in the now closed #18 is still relevant. The uri method should return a resolvable URL or uri and url methods should be available in both the Local and S3 providers.

Currently the s3 provider delivers only the path back in the uri method. This method should provide the same functionality as the Local provider in that it deliver a browser-resolvable url or uri back. For publicly available bucket objects, this should be the URL. For private bucket objects, this should be the temporary, presigned URL.

In addition, the url method is also delivering a presigned URL back. If the bucket object is public, then the URL should not be pre-signed so that browser caching of the object can be leveraged.

Suggest adding a caching layer for testing if the object is public. This way, if an object is requested frequently, the results to check S3 permissions on the object will not have to be made on each request to the URL

On a side note, this is why Laravel uses url as the method name - because it can return either a relative URI path ( local ) or a URL if the resource is remote.

Add custom interception points and announcements in all providers for the following:

cbfsOnDiskStart
cfsOnDiskShutdown
cbfsOnFileCreate
cbfsOnFileMove
cbfsOnFileCopy
cbfsOnFileDelete
cbfsOnFileInfoRequest
cbfsOnFileMove
cbfsOnDirectoryMove
cbfsOnDirectoryCreate
cbfsOnDirectoryCopy
cbfsOnDirectoryDelete

With S3 and Local providers, files often have the concept of inheritable permissions from the containing folder or bucket. Allow for the setting of disk visibility of inherit, which will bypass all attempts to set permissions on files and folders, and allow the object to inherit the settings of the parent.

A common configuration for S3 buckets is to create a Cloudfront CDN distribution in front of the bucket which handles SSL termination and caching. Another example is using Cloudflare, which sets a CNAME record to point to the AWS bucket domain. An example of this is downloads.ortussolutions.com

Add support for a public domain property on the S3Provider that will return the public URL.

The docs don't show the configuration for S3 drives. Are buckets considered directories in this context?

https://cbfs.ortusbooks.com/getting-started/configuration

Let's say you have a simple JSON file named supermario.json.

{
    "game": "Super Mario Bros"
}

Create a file object and get the contents

var result = disk.file( "supermario.json" ).get();

The result returned will be binary when it should be just the JSON. I believe this has something to do with the AbstractDiskProvider.

	/**
	 * Find out the mime type of a path
	 *
	 * @path The file path to check
	 */
	function getMimeType( required path ){
		return variables.javaUrlConnection.guessContentTypeFromName( arguments.path );
	}

What are the steps to reproduce this issue?

1. … run the ITB site locally and use a Windows machine.
2. … Go to the admin site and open the media manager option.
3. …

What happens?

The issue is that the files are not found.
…

What were you expecting to happen?

Load all media resources
…

Any logs, error output, etc?

…

Any other comments?

Jon Clausen could fix it by changing the path value, on the configure function line 19, with expandPath( "/modules_app/contentbox-custom/_content" )
…

What versions are you using?

Operating System: …
Windows 10

Package Version: …

Add a createFromFile method so that file creation doesn't always require the reading in of file contents but can use a source file to copy or move.
The interface signature will be:

/**
 * Create a file in the disk from a file path
 *
 * @source       The file path to use for storage
 * @directory    The target directory
 * @name 		 The destination file name. If not provided it defaults to the file name from the source
 * @visibility   The storage visibility of the file, available options are `public, private, readonly` or a custom data type the implemented driver can interpret
 * @overwrite    Flag to overwrite the file at the destination, if it exists. Defaults to true.
 * @deleteSource Flag to remove the source file upon creation in the disk.  Defaults to false.
 *
 * @return cbfs.models.IDisk
 *
 * @throws cbfs.FileOverrideException - When a file exists and no override has been provided
 */
function createFromFile(
	required source,
	required directory,
	string name,
	string visibility,
	boolean overwrite = true
	boolean deleteSource = false
);

The file object was created after we added announced interceptions for various file operations.

For example, consider the cbfsOnFileCreate interception. The listener signature looks like this.

function cbfsOnFileCreate( path, contents, visibility, metadata, overwrite, mode, disk ) {

}

This can be greated simplified now that we have a file object that pass around.

function cbfsOnFileCreate( file ) {

}

Add a download method which will deliver files directly to the browser. The interface signature will be:

/**
 * Download a file to the browser
 *
 * @path       The file path to download
 *
 * @throws cbfs.FileNotFoundException
 */
string function download( required path );

Good point. The other verb I tend to use besides orFail is guard. I do like that exists and existsOrFail is something that cborm and Quick have.

If we all agree on existsOrFail I can change that in this PR.

I would appreciate keeping all of these in the same place and then breaking them out to separate modules on initial release.

There are two other storage drivers I'd like to implement, off the top of my head : SQL Server Filestream, and MongoDB Grid FS. Once we get those implemented, this would be a viable solution for just about any media storage needs

CBFS was not implemented with the idea of a file as a separate object, only the disk performs operations and the path is always passed in to any function to the disk.

Currently, to upload a file, it requires a separate non-cbfs fileUpload and a diskName.create( ... ) call to move the file ( which is currently non-functional with binary files #20 ). Add an upload method to the providers, which can be used for a one-step method to upload a file to a target location.

example:

cbfs().get( myDiskName ).upload( field="fileInput", directory="my/drive/path", overwrite=true )

Laravel example: https://laravel.com/docs/9.x/filesystem#file-uploads

Calling files() should return an array of files and assume the disk's root path as the directory since no directory is provided.

When calling .files(), it's currently throwing this error:

The parameter [directory] to function [files] is required but was not passed in.

When calling .files( "" ), it's throwing this error:

Directory [] not found.

Currently, you can only copy or move files on the same disk.

Fee free to close this if you don’t like the name. I just thought cbfs had a nice ring to it like it was a filesystem format. 🙃

The usage of java.nio.file.Files in LocalProvider.cfc requires a byte array passed in to the write method. Binary objects do not have this member function.

An attempt to create a file by passing in a content argument of binary data currently fails.