Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
CELDEV-1006 - refactor AttachmentURLCommand to ResourceUrlService #297
base: dev
Are you sure you want to change the base?
CELDEV-1006 - refactor AttachmentURLCommand to ResourceUrlService #297
Changes from all commits
80c1f47
1280d39
80b0d1a
47750ab
588f034
150a7fc
3b0ec74
9e3352c
b1c77ba
37b6138
ab8fd7c
504b79c
b9feaa7
8933716
69aed61
0f4570c
ee3a2bd
a74d3f9
c62b5f6
09a4c02
c507cc8
6f4b916
229455d
f2fa74c
334d07e
eb05687
094d2c0
38fdb38
4c02d4f
4c2a217
2ccb773
d05cae7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the service method called
...Uri
, the SS name...Url
but both return aUriBuilder
? a few thoughts:UriBuilder#build
andURI#toURL
. So if we actually handle URLs here, the java logic should capture anyMalformedURLException
.FileUriService
actually builds URLs, so why not returnURL
objects? This would encapsulate the handling of URL malformation inside the service and make it's usage more robust. If a client wants to change the URL, it can always create aUriBuilder
from it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to 1. There is a #try statement for velocity
to 2. Nope URL must be fully qualified. UriBuilder can handle e.g. "/folder/to/file.js" which is not fully quallified and returns a correct toString()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UriBuilder#build
andURI#toURL
within velocity. the try/catch here won't preventMalformedURLException
thrown outside of the try/catch....Url...
if they don't return at minimum Url's but incomplete Uri's. The service methods are named...Uri...
. Why this inconsistency?