[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15417521#comment-15417521
 ] 

Ian Boston commented on SLING-5948:
---

{{X-sling-uploadmode}} yes, no problem. 

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on Servlet 3 to get the Part API, 
> requiring some patches to the existing test classes.
> To support both methods a standard Servlet to handle streamed 

[jira] [Commented] (SLING-5948) Support Streaming uploads.

[Bertrand Delacretaz (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15417510#comment-15417510
 ] 

Bertrand Delacretaz commented on SLING-5948:


I just had a quick look and the performance improvements are impressive, thanks 
for this!

A nitpick: could the header be named {{X-sling-uploadmode}} to be more specific?

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on 

Streaming upload.

[Ian Boston]

Hi,
The streaming upload patch in [1] is functionally complete for performing
streaming uploads. It shows a 5x improvement in upload speed with a quick
5.6GB upload test on my box over the current upload multipart mechanism
against the same repository configuration. Results are shared in the issue.

Since this adds to the engine bundle and the servlet bundle could someone
review before I commit ?

Best Regards
Ian

1 https://issues.apache.org/jira/browse/SLING-5948

[jira] [Updated] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

 [ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ian Boston updated SLING-5948:
--
Attachment: TarMKDSNotStreamed.png
TarMKStreamed.png
TarMKDSStreamed.png

Tested with a 6GB file for both TarMK and TarMK+FDS with a 5.6GB zip file.
AEM running on a SSD. Source file on a External HDD over USB3. No network 
involved.

Streamed upload into TarMK 4m1.138s elapsed. (23MB/s)
Streamed upload into TarMK with FDS 2m44.614s (34MB/s)
Non Streamed upload into TarMK with FDS 14m33.138s (6.56MB/s)
Direct OS level copy (cp) between disks 2m15.823s (42MB/s)


The streaming upload shows a 5x improvement in upload performance in this test.

TarMK with FDS streams directly to a tmp file in the FS DS location which is 
moved into the DS when complete with an OS level move. CPU usage is greater 
perhaps due to the way Oak performs the stream transfer (ie not using native 
channels)

TarMK without FDS must put blobs into the segment store, hence the greater cost 
in terms of time and GC activity. CPU is lower, perhaps because it takes longer.

TarMK with DS non streaming shows 2x the IO overhead and higher CPU usage. Not 
certain why the high CPU usage.




> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch, 
> TarMKDSNotStreamed.png, TarMKDSStreamed.png, TarMKStreamed.png
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken 

[jira] [Updated] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

 [ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ian Boston updated SLING-5948:
--
Attachment: SLING-5948-Proposal2v3.patch

Added unit test coverage and tested successfully in in Sling launchpad.

Apply the patch, 

{code}
cd bundles/engine
mvn clean install
cd ../servlets/post
mvn clean install
cd ../../launchpad/builder
mvn clean install
java -jar target/org.apache.sling.launchpad-9-SNAPSHOT.jar &
tail -f sling/logs/error.log
{code}

Then Test normal upload 
{code}
curl -v -F file=@P1060839.jpg http://admin:admin@localhost:8080/content/test
{code}

Then test streamed upload
{code}
curl -v -F P1060839.jpg=@P1060839.jpg 
http://admin:admin@localhost:8080/content/test?uploadmode=stream
{code}

or 
{code}
curl -v -H "X-uploadmode: stream" -F P1060839.jpg=@P1060839.jpg 
http://admin:admin@localhost:8080/content/test
{code}



> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch, SLING-5948-Proposal2v3.patch
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too 

[jira] [Updated] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

 [ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ian Boston updated SLING-5948:
--
Affects Version/s: Servlets Post 2.3.12

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on Servlet 3 to get the Part API, 
> requiring some patches to the existing test classes.
> To support both methods a standard Servlet to handle streamed uploads would 
> be needed, connecting the file request stream to the Resource output stream. 
> In some cases (Oak S3 DS Async Uploads, 

[jira] [Updated] (SLING-5948) Support Streaming uploads.

[Ian Boston (JIRA)]

 [ 
https://issues.apache.org/jira/browse/SLING-5948?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ian Boston updated SLING-5948:
--
Component/s: Servlets

> Support Streaming uploads.
> --
>
> Key: SLING-5948
> URL: https://issues.apache.org/jira/browse/SLING-5948
> Project: Sling
>  Issue Type: Bug
>  Components: Engine, Servlets
>Affects Versions: Servlets Post 2.3.12, Engine 2.5.0
>Reporter: Ian Boston
>Assignee: Ian Boston
> Attachments: SLING-5948-Proposal1-illustration.patch, 
> SLING-5948-Proposal2v2.patch
>
>
> Currently multipart POST request made to sling use the commons file upload 
> component that parses the request fully before processing. If uploads are 
> small they are stored in byte[], over a configurable limit they are sent to 
> disk. This creates additional IO overhead, increases heap usage and increases 
> upload time.
> Having searched the SLing jira, and sling-dev I have failed to find an issue 
> relating to this area, although it has been discussed in the past.
> I have 2 proposals.
> The SlingMain Servlet processes all requests, identifying the request type 
> and parsing the request body. If the body is multipart the Commons File 
> Upload library is used to process the request body in full when the 
> SlingServletRequest is created or the first parameter is requested. To enable 
> streaming of a request this behaviour needs to be modified. Unfortunately, 
> processing a streamed request requires that the ultimate processor requests 
> multipar parts in a the correct order to avoid non streaming, so a streaming 
> behaviour will not be suitable for most POST requests and can only be used if 
> the ultimate Servlet has been written to process a stream rather than a map 
> of parameters.
> Both proposals need to identify requests that should be processed as a 
> stream. This identification must happen in the headers or URI as any 
> identification later than the headers may be too late. Something like a 
> custom header (x-uploadmode: stream) or a query string (?uploadmode=stream) 
> or possibly a selector (/path/to/target.stream) would work and each have 
> advantages and disadvantages.
> h1. Proposal 1
> When a POST request is identified as multipart and streaming, create a 
> LazyParameterMap that uses the Commons File Upload Streaming API 
> (https://commons.apache.org/proper/commons-fileupload/streaming.html) to 
> process the request on demand as parameters are requested. If parameters are 
> requested out of sequence, do something sensible attempting to maintain 
> streaming behaviour, but if the code really breaks streaming, throw an 
> exception to alert servlet developer early.
> h2. Pros
> * Follows a similar pattern to currently using the Servlet API.
> h2. Cons
> * [] params will be hard to support when the [] is out of order, and almost 
> impossible if the [] is an upload body.
> * May not work when a request is routed incorrectly as getParameter requests 
> will be out of streaming sequence.
> h2. Proposal 2
> When a POST request is identified as multipart and streaming, create a 
> NullParameterMap that returns null for all parameter get operations. In 
> addition set a request Attribute containing a Iterator that allows 
> access to the request stream in a similar way to the Commons File Upload 
> Streaming API.  Servlets that process uploads streams will use the 
> Iterator object retrieved from the request. Part is the Servlet 3 Part 
> https://tomcat.apache.org/tomcat-7.0-doc/servletapi/javax/servlet/http/Part.html.
>  IIUC This API is already used in the Sling Engine and exported by a bundle.
> h2. Pros
> * Won't get broken by existing getParameter calls, which all return null and 
> do no harm to the stream.
> * Far simpler implementation as the Servlet implementation has to get the 
> request data in streaming order.
> h2. Cons
> * Needs a custom Sling Upload Operation that understand how to process the 
> Iterator
> * Can't use the adaptTo mechanism on the request, as 
> request.adaptTo(Iterator.class) doesn't make sense being too generic. Would 
> need a new API to make this work. request.adaptTo(PartsIterator.class), which 
> PartsIterator extends Iterator.
> * Supporting the full breadth of the Sling Operation protocol in the Sling 
> Upload Operation will require wide scale duplication of code from the 
> ModifyOperation implementation as the ModifyOperation expects RequestProperty 
> maps and wont work with a streamed part.
> * Forces the Sling Post bundle to depend on Servlet 3 to get the Part API, 
> requiring some patches to the existing test classes.
> To support both methods a standard Servlet to handle streamed uploads would 
> be needed, connecting the file request stream to the Resource output stream. 
> In some cases (Oak S3 DS Async Uploads, Mongo DS) this wont 

Re: [VOTE] Release Sling Hypermedia API client-side tools 1.0.0

[Andrei Dulvac]

Anyone, please?

On Wed, Aug 10, 2016 at 4:56 PM Andrei Dulvac  wrote:

> Hi,
> We solved 3 issues for this initial 
> release:https://issues.apache.org/jira/browse/SLING/fixforversion/12337959
> Staging repository:
>
> https://repository.apache.org/content/repositories/orgapachesling-1499/
>
>
> You can use this UNIX script to download the release and verify the 
> signatures:http://svn.apache.org/repos/asf/sling/trunk/check_staged_release.sh
> Usage:
>
> sh check_staged_release.sh 1499 /tmp/sling-staging
>
>
> Please vote to approve this release:
>
>   [ ] +1 Approve the release
>   [ ]  0 Don't care
>   [ ] -1 Don't release, because ...
> This majority vote is open for at least 72 hours.
>
>
>
> On Wed, Aug 10, 2016 at 3:01 PM Oliver Lietz 
> wrote:
>
>> On Wednesday 10 August 2016 14:04:00 Carsten Ziegeler wrote:
>> [...]
>> > Not exporting or making it private should be the same. I looked at your
>> > change and tbh I have no idea why it doesn't work. I think the best
>> > would be to simply rename the package to *.impl - this way it is obvious
>> > that its not a public/exported package. And it will be automatically
>> > handled as such.
>>
>> Or internal instead of impl. IMHO internal illustrates the intent more
>> obvious
>> than impl as we also export packages with classes which implement
>> functions.
>> We also have impl packages in Sling with interfaces in it.
>>
>> Regards,
>> O.
>>
>> > > I will drop this release, sort out these issues and continue from
>> there.
>> >
>> > Thanks, really appreciated. And again, sorry for looking so late at it
>> >
>> > Regards
>> > Carsten
>>
>>

[jira] [Commented] (SLING-5637) Resource.delete is not called if resource is adaptable to Node

[indra kumar gurjar (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15417228#comment-15417228
 ] 

indra kumar gurjar commented on SLING-5637:
---

The patch has broken chunk delete functionality.
when trying to delete chunks it deletes asset also.

> Resource.delete is not called if resource is adaptable to Node
> --
>
> Key: SLING-5637
> URL: https://issues.apache.org/jira/browse/SLING-5637
> Project: Sling
>  Issue Type: Bug
>  Components: Servlets
>Affects Versions: Servlets Post 2.3.8
>Reporter: Carsten Ziegeler
>Assignee: Carsten Ziegeler
> Fix For: Servlets Post 2.3.10
>
>
> If a resource is adaptable to Node, the node is directly removed.
> ResourceProvider.delete is never invoked.
> While this works with the JcrResourceProvider it does not work with any other 
> resource provider providing resources which adapt to a node
> Proposed solution:
> {code}
> ### Eclipse Workspace Patch 1.0
> #P org.apache.sling.servlets.post
> Index: 
> src/main/java/org/apache/sling/servlets/post/impl/operations/DeleteOperation.java
> ===
> --- 
> src/main/java/org/apache/sling/servlets/post/impl/operations/DeleteOperation.java
>  (Revision 1737673)
> +++ 
> src/main/java/org/apache/sling/servlets/post/impl/operations/DeleteOperation.java
>  (Arbeitskopie)
> @@ -58,15 +58,15 @@
>  // SLING-3203: selectors, extension and suffix make no sense here and
>  // might lead to deleting other resources than the one the user 
> means.
>  final RequestPathInfo rpi = request.getRequestPathInfo();
> -if( (rpi.getSelectors() != null && rpi.getSelectors().length > 0) 
> +if( (rpi.getSelectors() != null && rpi.getSelectors().length > 0)
>  || (rpi.getExtension() != null && 
> rpi.getExtension().length() > 0)
>  || (rpi.getSuffix() != null && rpi.getSuffix().length() > 
> 0)) {
>  response.setStatus(
> -HttpServletResponse.SC_FORBIDDEN, 
> +HttpServletResponse.SC_FORBIDDEN,
>  "DeleteOperation request cannot include any selectors, 
> extension or suffix");
>  return;
>  }
> -
> +
>  final VersioningConfiguration versioningConfiguration = 
> getVersioningConfiguration(request);
>  final boolean deleteChunks = isDeleteChunkRequest(request);
>  final Iterator res = getApplyToResources(request);
> @@ -100,19 +100,17 @@
>  } else {
>  checkoutIfNecessary(node.getParent(), changes,
>  versioningConfiguration);
> -node.remove();
>  }
> -
> -} else {
> -try {
> -resource.getResourceResolver().delete(resource);
> -} catch (final PersistenceException pe) {
> -if (pe.getCause() instanceof RepositoryException) {
> -throw (RepositoryException) pe.getCause();
> -}
> -throw new RepositoryException(pe);
> +}
> +try {
> +resource.getResourceResolver().delete(resource);
> +} catch (final PersistenceException pe) {
> +if (pe.getCause() instanceof RepositoryException) {
> +throw (RepositoryException) pe.getCause();
>  }
> +throw new RepositoryException(pe);
>  }
> +
>  changes.add(Modification.onDeleted(resource.getPath()));
>  }
>  {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Comment Edited] (SLING-5637) Resource.delete is not called if resource is adaptable to Node

[indra kumar gurjar (JIRA)]

[ 
https://issues.apache.org/jira/browse/SLING-5637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15417228#comment-15417228
 ] 

indra kumar gurjar edited comment on SLING-5637 at 8/11/16 1:29 PM:


[~cziegeler] The patch has broken chunk delete functionality.
when trying to delete chunks it deletes asset also.


was (Author: igurjar):
The patch has broken chunk delete functionality.
when trying to delete chunks it deletes asset also.

> Resource.delete is not called if resource is adaptable to Node
> --
>
> Key: SLING-5637
> URL: https://issues.apache.org/jira/browse/SLING-5637
> Project: Sling
>  Issue Type: Bug
>  Components: Servlets
>Affects Versions: Servlets Post 2.3.8
>Reporter: Carsten Ziegeler
>Assignee: Carsten Ziegeler
> Fix For: Servlets Post 2.3.10
>
>
> If a resource is adaptable to Node, the node is directly removed.
> ResourceProvider.delete is never invoked.
> While this works with the JcrResourceProvider it does not work with any other 
> resource provider providing resources which adapt to a node
> Proposed solution:
> {code}
> ### Eclipse Workspace Patch 1.0
> #P org.apache.sling.servlets.post
> Index: 
> src/main/java/org/apache/sling/servlets/post/impl/operations/DeleteOperation.java
> ===
> --- 
> src/main/java/org/apache/sling/servlets/post/impl/operations/DeleteOperation.java
>  (Revision 1737673)
> +++ 
> src/main/java/org/apache/sling/servlets/post/impl/operations/DeleteOperation.java
>  (Arbeitskopie)
> @@ -58,15 +58,15 @@
>  // SLING-3203: selectors, extension and suffix make no sense here and
>  // might lead to deleting other resources than the one the user 
> means.
>  final RequestPathInfo rpi = request.getRequestPathInfo();
> -if( (rpi.getSelectors() != null && rpi.getSelectors().length > 0) 
> +if( (rpi.getSelectors() != null && rpi.getSelectors().length > 0)
>  || (rpi.getExtension() != null && 
> rpi.getExtension().length() > 0)
>  || (rpi.getSuffix() != null && rpi.getSuffix().length() > 
> 0)) {
>  response.setStatus(
> -HttpServletResponse.SC_FORBIDDEN, 
> +HttpServletResponse.SC_FORBIDDEN,
>  "DeleteOperation request cannot include any selectors, 
> extension or suffix");
>  return;
>  }
> -
> +
>  final VersioningConfiguration versioningConfiguration = 
> getVersioningConfiguration(request);
>  final boolean deleteChunks = isDeleteChunkRequest(request);
>  final Iterator res = getApplyToResources(request);
> @@ -100,19 +100,17 @@
>  } else {
>  checkoutIfNecessary(node.getParent(), changes,
>  versioningConfiguration);
> -node.remove();
>  }
> -
> -} else {
> -try {
> -resource.getResourceResolver().delete(resource);
> -} catch (final PersistenceException pe) {
> -if (pe.getCause() instanceof RepositoryException) {
> -throw (RepositoryException) pe.getCause();
> -}
> -throw new RepositoryException(pe);
> +}
> +try {
> +resource.getResourceResolver().delete(resource);
> +} catch (final PersistenceException pe) {
> +if (pe.getCause() instanceof RepositoryException) {
> +throw (RepositoryException) pe.getCause();
>  }
> +throw new RepositoryException(pe);
>  }
> +
>  changes.add(Modification.onDeleted(resource.getPath()));
>  }
>  {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: significant changes. Question?

[Bertrand Delacretaz]

Hi,

On Wed, Aug 10, 2016 at 12:57 PM, Ian Boston  wrote:
> ...What is the process in Sling for making significant changes to a bundle ?
> By significant, I mean anything that isn't a simple bug fix or 3 line
> commit

My take is to make sure we have strong test coverage of the affected
parts before the change (which might need work in some areas) so that
the incoming changes are well covered by tests as well.

Asking for a review is welcome when working on a module that we're
unfamiliar with.

-Bertrand

Re: significant changes. Question?

[Ian Boston]

Hi
Thank you for the clarification.
In this instance, I will ask for a review before committing.
I am not expecting to make any API changes.
Best Regards
Ian

On 10 August 2016 at 13:22, Carsten Ziegeler  wrote:

> Hi,
>
> there is no formal process and it more or less depends on the one who
> does the change and what he decides is the best way to move forward.
> If you want someone to review it before you commit, then you can ask for
> it, but it might happen that no one does it nevertheless :) (Due to
> different reasons). I think at least some of us often do a review after
> it has been committed, so if there is someone interested in reviewing
> it, it will happen. Therefore it is usually good to give it some days,
> so people have time to review it. But again, it's up to the one who
> really cares to decide when to do the release.
>
> I think for API changes this is slightly different as it makes more
> sense to discuss api changes before they are done. But even if not,
> things can be reverted/changed after they are committed.
>
> Regards
> Carsten
>
>
> > Hi,
> > What is the process in Sling for making significant changes to a bundle ?
> > By significant, I mean anything that isn't a simple bug fix or 3 line
> > commit.
> >
> > I am asking because I have some patches to bundles/engine and
> > bundles/servlet/post. They have no impact (I hope) to the normal
> operation
> > of either bundles, however since these bundles are core, I feel I should
> > probably should not just do a CTR, as there have been no comments on
> > SLING-5948... or is that Ok?
> >
> > I have not made any API changes (no package version numbers have been
> > changed), but I have updated the parent pom of bundles/servlet/post
> >  (26->27) so it uses Servlet 3 to get access to the Part interface. The
> > changes that I have require to a request to explicitly trigger for the
> code
> > to be exercised. The patches are still WIP.
> >
> > Once committed, what should happen ?
> > Should the bundles be released or should the changes sit there till
> someone
> > else decides to release ?
> >
> > There is no urgency I know of to releasing these bundles. SLING-5948
> > scratches an itch.
> >
> > I've not seen discussion of this subject on this list (for many years),
> > hence the question.
> > Best Regards
> > Ian
> >
>
>
>
>
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> cziege...@apache.org
>
>