Skip to content

Fix : Linode File Upload#54

Merged
christyjacob4 merged 8 commits intoutopia-php:mainfrom
everly-gif:fix-trim-storage-endpoint
Dec 9, 2022
Merged

Fix : Linode File Upload#54
christyjacob4 merged 8 commits intoutopia-php:mainfrom
everly-gif:fix-trim-storage-endpoint

Conversation

@everly-gif
Copy link
Copy Markdown
Contributor

@everly-gif everly-gif commented Sep 8, 2022

Trim extra slashes before passing root.

Doing this because it creates an extra / while integrating with Appwrite.
for eg , a call as below was being made.

https://<bucket-name>.eu-central-1.linodeobjects.com**//**storage/uploads/app-630cd8d7a106aa302d27/630cd8e90d3b33e8ff5c/6/3/0/c/630cdc2875416374d993.jpg

The root within Appwrite are already prefixed with / (seen as below). Hence, we need to trim it here to avoid double slashes.

This PR implements trimming the root in the constructor.

const APP_STORAGE_UPLOADS = '/storage/uploads';
const APP_STORAGE_FUNCTIONS = '/storage/functions';
const APP_STORAGE_BUILDS = '/storage/builds';
const APP_STORAGE_CACHE = '/storage/cache';
const APP_STORAGE_CERTIFICATES = '/storage/certificates';
const APP_STORAGE_CONFIG = '/storage/config';

@everly-gif everly-gif marked this pull request as draft September 8, 2022 16:49
@stnguyen90
Copy link
Copy Markdown
Contributor

Could this be where the extra slash is being added in the beginning?

@everly-gif
Copy link
Copy Markdown
Contributor Author

Could this be where the extra slash is being added in the beginning?

yes, I believe so. I feel that's a better place to remove the slash , instead of trimming it. what do you think @lohanidamodar @eldadfux ?

@everly-gif
Copy link
Copy Markdown
Contributor Author

Also the tests pass locally , they fail in Travis CI because of the expired credentials of various providers. Need help in updating them as I do not have access to it.

@everly-gif everly-gif marked this pull request as ready for review October 18, 2022 16:40
@christyjacob4
Copy link
Copy Markdown
Contributor

Please sync your branch with main . It has fixes for the failing CI tests

Copy link
Copy Markdown
Contributor

@christyjacob4 christyjacob4 left a comment

Choose a reason for hiding this comment

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

We need to also update the local device adapter to adhere to these changes.

Copy link
Copy Markdown
Contributor

@lohanidamodar lohanidamodar left a comment

Choose a reason for hiding this comment

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

Looks good.

@christyjacob4 christyjacob4 merged commit f19a200 into utopia-php:main Dec 9, 2022
@christyjacob4
Copy link
Copy Markdown
Contributor

Awesome work @everly-gif 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants