[
https://issues.apache.org/jira/browse/JCR-4355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16576874#comment-16576874
]
Alexander Klimetschek commented on JCR-4355:
--------------------------------------------
Thanks for the review. It would actually be nice to have a github workflow to
view and discuss these changes...
{quote}charset issue
{quote}
Ok to use _if the media type defines a "charset"_ in all places? (Your patch
just adapted one)
{quote}reduces the amount of @link tags having in-line prose (which I find
confusing to read)
{quote}
I used them (with just the method name w/o classname and parameters) to
increase readability in the final rendered HTML, otherwise there are repeated
very long {{JackrabbitValueFactory.somelongmethod(with, many, params)}} in
there. Wanted the docs be optimized for their html reading, not the java
sources.
Note in at least one place, your changes reformatted <ul> lists to an inline
text block (probably IDE reformatting), making them hard to follow in the
sources.
+1 for the direct link to upload algorithm.
{quote}Some storage providers also support multi-part uploads by reusing a
single URI multiple times, in which case the implementation may also return a
single URI regardless of the size of the data being uploaded
{quote}
I did not touch that text. This refers to Google Cloud Storage, which is the
only one of the major providers (AWS, Azure, Rackspace, Google) to support
[multipart via standard Content-Range headers on
PUT|https://cloud.google.com/storage/docs/json_api/v1/how-tos/resumable-upload],
without requiring separate part URLs. Which would be more standards-compliant
and simplifies the whole approach to a single URL, but, well... Anyway, we
don't support Google as DataStore right now, so there is no end-to-end
integration of that approach, nor is there a description of that in the upload
algorithm.
If we would add this, with the current API, a client could potentially detect
this case if getMaxPartSize() is smaller than the original file size AND a
single URI is provided. But I fear that might be a bit too fragile, and we
probably need a flag for that in the BinaryUpload instructions, such as
{{canUseContentRange()}}.
Maybe for now it's best to take that out of the javadoc, and only add this in
the future, when needed and it's clear how it can work.
I'll work on a new iteration of the docs.
> Javadoc fixes and improvements for new direct binary access API
> ---------------------------------------------------------------
>
> Key: JCR-4355
> URL: https://issues.apache.org/jira/browse/JCR-4355
> Project: Jackrabbit Content Repository
> Issue Type: Improvement
> Components: jackrabbit-api
> Reporter: Alexander Klimetschek
> Priority: Major
> Fix For: 2.18
>
> Attachments: JCR-4355-jr.diff, JCR-4355-v2-javadoc-html.zip,
> JCR-4355-v2.patch, JCR-4355.diff
>
>
> Here are some changes to the javadocs for the new API:
> [OAK-7569-api-javadoc-improvements.patch|https://issues.apache.org/jira/secure/attachment/12934364/12934364_OAK-7569-api-javadoc-improvements.patch]
> * more concise descriptions
> * correcting some inaccuracies (clients cannot choose whether to do single or
> multipart upload, multipart might be strictly required depending on the size)
> * most importantly the upload algorithm (standard partSize calculation was
> wrong)
> * focus on API users, separated notes to implementors
> * for BinaryDownloadOptions added note from which jcr properties a client
> would normally take these values from
> * added security considerations
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
