Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds request-direct-access-url file name option #197

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Adds request-direct-access-url file name option #197

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9de6d8
Adds request-direct-access-url file name option
canpan14 Jul 12, 2023
755de29
Add file name in response and fix expiryTime typo
canpan14 Jul 20, 2023
62a47d7
Merge branch 'Alfresco:master' into feature/2064-request-direct-acces…
canpan14 Jul 20, 2023
62f240d
Undo expiresAt fix
canpan14 Jul 22, 2023
5ad616c
Merge branch 'feature/2064-request-direct-access-url-file-name-option…
canpan14 Jul 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/main/webapp/definitions/alfresco-core.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2167,6 +2167,9 @@ paths:

If the content type is not supported for preview, then a value of **false** is ignored, and
the attachment will be returned in the response.

The **fileName** field is optional for setting the file name when downloaded. By default Alfresco
will use the `cm:name` attribute unless configured otherwise.
required: false
schema:
$ref: '#/definitions/DirectAccessUrlBodyCreate'
Expand Down Expand Up @@ -2453,6 +2456,9 @@ paths:

If the content type is not supported for preview, then a value of **false** is ignored, and
the attachment will be returned in the response.

The **fileName** field is optional for setting the file name when downloaded. By default Alfresco
will use the `cm:name` attribute unless configured otherwise.
required: false
schema:
$ref: '#/definitions/DirectAccessUrlBodyCreate'
Expand Down Expand Up @@ -3130,6 +3136,9 @@ paths:

If the content type is not supported for preview, then a value of **false** is ignored, and
the attachment will be returned in the response.

The **fileName** field is optional for setting the file name when downloaded. By default Alfresco
will use the `cm:name` attribute unless configured otherwise.
required: false
schema:
$ref: '#/definitions/DirectAccessUrlBodyCreate'
Expand Down Expand Up @@ -3464,6 +3473,9 @@ paths:

Note: It is up to the actual ContentStore implementation if it can fulfil this
request or not.

The **fileName** field is optional for setting the file name when downloaded. By default Alfresco
will use the `cm:name` attribute unless configured otherwise.
required: false
schema:
$ref: '#/definitions/DirectAccessUrlBodyCreate'
Expand Down Expand Up @@ -4157,6 +4169,9 @@ paths:

If the content type is not supported for preview, then a value of **false** is ignored, and
the attachment will be returned in the response.

The **fileName** field is optional for setting the file name when downloaded. By default Alfresco
will use the `cm:name` attribute unless configured otherwise.
required: false
schema:
$ref: '#/definitions/DirectAccessUrlBodyCreate'
Expand Down Expand Up @@ -4406,6 +4421,9 @@ paths:

If the content type is not supported for preview, then a value of **false** is ignored, and
the attachment will be returned in the response.

The **fileName** field is optional for setting the file name when downloaded. By default Alfresco
will use the `cm:name` attribute unless configured otherwise.
required: false
schema:
$ref: '#/definitions/DirectAccessUrlBodyCreate'
Expand Down Expand Up @@ -10693,6 +10711,9 @@ definitions:
attachment:
type: boolean
description: URL type (embedded/attachment).
fileName:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd usually have the items from the "Create" entity in the response too - i.e. DirectAccessUrl below. I don't think this is explicitly called out in the v1 API guidelines though.

I've added our architect team to review too, since they usually review additions to the v1 API.

Copy link
Author

@canpan14 canpan14 Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could add it to the DirectAccessUrl class in alfresco-community-repo, but it looks like the current object structure is fed in from the content store itself. Not exactly sure where that code is or how dangerous it is to alter.
I could make a DirectAccessUrlResponse class for the purposes of extending and adding on the additional field.
It's not hard to return the field, I just want to keep things in line with your expectations for the code base as a whole.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tpage-alfresco is correct, please make sure to add the file name to the response too.
In terms of implementation, you have already modified the ContentServiceImpl class, so adding an attribute to DirectAccessUrl (part of our Java public API) is not a problem.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jamalkm The issue is the call to the content store serialized directly into DirectAccessUrl. While the serialization might still work with an extra field added, I worry it might start to diverge from representing the data object actually returned from the database vs. what we are returning from the API.

DirectAccessUrl directAccessUrl = null;
if (store.isContentDirectUrlEnabled())
{
    try
    {
        // referencing this
        directAccessUrl = store.requestContentDirectUrl(contentUrl, attachment, fileName, contentMimetype, validFor);
    }
    catch (UnsupportedOperationException ex)
    {
        // expected exception
    }
}
return directAccessUrl;

I can still open a PR on it though

Copy link
Author

@canpan14 canpan14 Jul 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alfresco/alfresco-community-repo#2081
Let me know if this is what you were looking for.

type: string
description: Optional file name when downloaded
DirectAccessUrlEntry:
type: object
required:
Expand All @@ -10715,6 +10736,9 @@ definitions:
type: string
format: date-time
description: The direct access URL would become invalid when the expiry date is reached
fileName:
type: string
description: The file name when downloaded
SharedLinkBodyCreate:
type: object
required:
Expand Down