The "download" action in the "_scaffold" doesn't work quite right

[CarlosDB]

Hello,

I'm coming from web2py, and in my free time, I've been doing some research in anticipation of a future (and, I believe, distant) ☹ migration to py4web.

Right now, I'm working with upload fields and image downloads, and there's something that's not working quite right. If files are stored in `settings.UPLOAD_FOLDER`, everything works well.

However, one of the applications I have in web2py heavily uses upload fields with images. Because of this, I have a directory for each table within the "uploads" folder.

└── uploads
    ├── t_table_1
    ├── t_table_2
    └── t_table_3

Inside each directory, I store the images from the upload fields of each table.

There are several things that I believe are not quite right.

• First, there's the action defined in common.py of the "_scaffold" app for downloading files from upload fields. The comments reference specifying "field.upload_path," but it seems like there's either an error or something pending implementation. Would it be appropriate to submit a PR to change it to "field.uploadfolder"?

# #######################################################

# Define a convenience action to allow users to download

# files uploaded and reference by Field(type='upload')

# #######################################################

if settings.UPLOAD_FOLDER:

    @action('download/<filename>')                                                  

    @action.uses(db)                                                                                          

    def download(filename):

        return downloader(db, settings.UPLOAD_FOLDER, filename)

    # To take advantage of this in Form(s)

    # for every field of type upload you MUST specify:

    #

    # field.upload_path = settings.UPLOAD_FOLDER

    # field.download_url = lambda filename: URL('download/%s' % filename)

• Second, when I use uploadfolder = settings.MY_CUSTOM_UPLOAD_FOLDER in a field of a table, the proposed action in "_scaffold" for downloading files doesn't work, and the links to the files are broken (404 error).

   Is this the expected behavior, and should new actions be defined to download files with custom upload folders?

While reviewing the code, I came across the following:

def downloader(db, path, filename, download_filename=None):

    """

    Given a db, and filesystem path, and the filename of an uploaded file,

    it retrieves the file, checks permission, and returns or stream its.

    Optionally as an attachment if the URL contains attachment=true

    If the file is not in the filesystem, it gets copied into the path folder

    before being returned for speed.

    To be used as follows:

    @action('download/<filename>')

    @action.uses(db)

    def download(filename):

        return downloader(db, PATH, filename)

    PATH is the fullpath to where uploaded files are normally stored.

    """

    items = re.match(REGEX_UPLOAD_PATTERN, filename)

    if not items:

        raise HTTP(404)

    tablename = items.group("table")

    fieldname = items.group("field")

    try:

        field = db[tablename][fieldname]

# New line

        path = field.uploadfolder if field.uploadfolder else path

        # Functionality to handle uploadseparate Field declaration.

        if field.uploadseparate:

            uuidname = items.group("uuidkey")

            path = os.path.join(path, *[f"{tablename}.{fieldname}", uuidname[:2]])

    except (AttributeError, KeyError):

        raise HTTP(404)

    try:

        (original_name, stream) = field.retrieve(filename, path, nameonly=True)

        fullpath = os.path.join(path, filename)

        if not os.path.exists(fullpath) and hasattr(stream, "read"):

            with open(fullpath, "wb") as fp:

                shutil.copyfile(fp, stream)

    except NotAuthorizedException:

        raise HTTP(403)

    except NotFoundException:

        raise HTTP(404)

    except IOError:

        raise HTTP(404)

    if not request.query.get("attachment"):

        download_filename = None

    elif not download_filename:

        download_filename = original_name

    return bottle.static_file(filename, root=path, download=download_filename)

I see that with just one line

path = field.uploadfolder if field.uploadfolder else path

it's easy to override the image path when "uploadfolder" is specified in the table field definition. This way, with just one "download" action, everything would work well.

I suppose it's up to Massimo to decide if these changes would be appropriate.

Carlos.

[Jim Steil]

Here is what I have for one of my tables

db.define_table(
    "ticket_attachment",
    Field("id", "id", readable=False),
    Field("ticket", "reference ticket"),
    Field("uploaded", "datetime", requires=IS_DATETIME("")),
    Field(
        "file_name",
        length=256,
        required=True,
        requires=IS_NOT_EMPTY(),
    ),
    Field(
        "attachment",
        "upload",
        required=True,
        autodelete=True,
        uploadfolder=os.path.join(
            FILE_LOCATIONS.get("uploads"),
            "tickets",
        ),
        download_url=lambda filename: URL(f"download/tickets-{filename}"),
    ),
)

Then I have this code in common.py:

#  define convenience action to allow users to download files uploaded and reference by Field(type='upload')
if FILE_LOCATIONS.get("uploads"):

    @action("download/<filename>")
    @action.uses(db)
    def download(filename):

        folder = filename.split("-")[0]
        path = FILE_LOCATIONS.get("uploads")
        if folder:
            filename = filename.split("-")[1]
            path += f"/{folder}"
        return downloader(db, path, filename)

    #  USAGE
    #
    #    uploadfolder=f"{FILE_LOCATIONS.get('uploads')}/tickets",
    #    download_url=lambda filename: URL(f"download/tickets-{filename}"),
    #
    #  this will put the file in the tickets sub-directory of the uploads directory
    #  the second line tells the downloader to look in the tickets subdirectory of the uploads
    #  directory to download the file.

Hope that helps

-Jim

[CarlosDB]

Thanks, Jim. I understand your code, and it works well. However, I think we're losing a bit compared to web2py.

Let me explain.

With web2py, there was no need to use the trick of passing the directory name to the download function via the URL. Web2py used the value of `uploadfolder` to locate the file to download.

Now, in py4web, the `downloader` function (py4web/utils/downloader.py) doesn't take `uploadfolder` into account, and some extra work is required.

The problem is that any changes to improve this might break your code. 😞

What do you think?

Carlos.

[Jim Steil]

Carlos

I won't have the time to look at this until next week some time.  If I forget and don't get back to you, please bump this so I remember...

-Jim

[CarlosDB]

Thank you. When you have time we can continue.

[Jim Steil]

I took a quick look this morning

I think this is going to need input from Massimo.  It's possible we could change the default as it is coded in common.py in _scaffold and therefore should only affect new apps built on the _scaffold.  But I think I would be out of line if I decided to change that without his input.

-Jim

[Massimo]

I think your proposed changes are great and I will be happy to take a PR

[CarlosDB]

Done!