You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "YegorKozlov (via GitHub)" <gi...@apache.org> on 2023/05/30 11:23:42 UTC
[GitHub] [sling-org-apache-sling-jcr-contentloader] YegorKozlov opened a new pull request, #22: SLING-11891 SlingPostServlet: Support importing binary data from json
YegorKozlov opened a new pull request, #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22
The PR adds support for importing binary data in SlingPostServlet
See https://issues.apache.org/jira/browse/SLING-11891
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] stefanseifert commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "stefanseifert (via GitHub)" <gi...@apache.org>.
stefanseifert commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1570334320
> @stefanseifert - we have some test failures on Windows, would you have time to take a look?
>
> ```
> [INFO] Results:
> [INFO]
> [ERROR] Failures:
> [ERROR] ZipReaderTest.createTempFile:224
> [ERROR] ZipReaderTest.createTempFileForceNotUnix:232->doWorkAsNotUnix:335->lambda$createTempFileForceNotUnix$9:233->createTempFile:224
> ```
-> https://issues.apache.org/jira/browse/SLING-11893
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] YegorKozlov commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1568843245
@enapps-enorman The leading colon properties in import json are not ignored and result in a "invalid property name" exception. My idea was to make sense of them instead of failing.
The Data URL style syntax is fine, but it is meant to embed small data inline. Not sure if JCR binaries will fit into it.
I'm working on a symmetrical PR to Sling GET Servlet to write the binary data, and would like to propose the leading colon syntax for a start. I'm fine to change it, but both parties need to agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] sonarcloud[bot] commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1682253113
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&metric=new_coverage&view=list) [88.2% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&metric=new_duplicated_lines_density&view=list)
 The version of Java (11.0.16.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] enapps-enorman commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1569439559
> @enapps-enorman The leading colon properties in import json are not ignored and result in a "invalid property name" exception. My idea was to make sense of them instead of failing.
>
I understand. I guess I was thinking of how the default sling POST servlet collects the content to be written to the repository.
```
SlingPostConstants.RP_PREFIX - "... This prefix must be used on all request
parameters which have significance to POST request processing. Such
parameters will not be used to denote properties to be written to the
repository."
```
> The Data URL style syntax is fine, but it is meant to embed small data inline. Not sure if JCR binaries will fit into it.
I'm not sure how this is different from your other proposal. The base64 encoded text would be the same length with either syntax.
> I'm working on a symmetrical PR to Sling GET Servlet to write the binary data, and would like to propose the leading colon syntax for a start. I'm fine to change it, but both parties need to agree.
I understand, but I would expect that including base64 encoded binaries in the json output should be optional (and disabled by default) as it could be abused.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] enapps-enorman commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1568757224
@YegorKozlov I see. Well, the leading colon seems to be already used for a specific purpose which is to be ignored during the import so I'm not sure I like changing that meaning for another purpose.
Maybe, the import parser could handle "[Data URLs](https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URLs)" style syntax for all binary property values like this: "data:[mediatype][;base64],data"
For example:
```
{
"jcr:primaryType": "nt:unstructured",
"file": {
"jcr:primaryType": "nt:file",
"jcr:content": {
"jcr:primaryType": "nt:resource",
"jcr:mimeType": "image/png",
"jcr:data": "data:image/x-png;base64,iVBORw0KGgoA"
}
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] rombert commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "rombert (via GitHub)" <gi...@apache.org>.
rombert commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1568495101
@stefanseifert - we have some test failures on Windows, would you have time to take a look?
```
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ZipReaderTest.createTempFile:224
[ERROR] ZipReaderTest.createTempFileForceNotUnix:232->doWorkAsNotUnix:335->lambda$createTempFileForceNotUnix$9:233->createTempFile:224
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] enapps-enorman commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "enapps-enorman (via GitHub)" <gi...@apache.org>.
enapps-enorman commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1568684852
@YegorKozlov I would be careful about expecting the output of the Sling GET JsonRenderer to be usable directly (without review and cleanup) as the input to the Sling POST import operation. FYI: Some of the known issues were under discussion with SLING-8375
The output from the ".tidy.json" selector only gives you a snapshot view of the current state (from the perspective of the current user), but not necessarily the JSON required to get to that state.
So, I still don't think the output from the ".tidy.json" selector can be a reliable generic solution for a round-trip payload given all the unidirectional ways that the output can be customized. For example, there may be concerns with nodetype constraints, ACLs, resource decorators that have changed items or other things that could be in play during the import of that same payload. You are potentially getting much more than just the "JCR" nodes and properties in the json output.
If you are truly only interested in a round-trip export/import scenario of only the JCR nodes and properties, then you may be better off using the xml or sysview.xml contentType for the round trip payload which would be more focused on just the JCR data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] YegorKozlov commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1568340750
The PR builds fine on JDK 11 on Mac
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-contentloader] YegorKozlov commented on pull request #22: SLING-11891 SlingPostServlet: Support importing binary data from json
Posted by "YegorKozlov (via GitHub)" <gi...@apache.org>.
YegorKozlov commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1568721729
@enapps-enorman Thanks, I read [SLING-8375](https://issues.apache.org/jira/browse/SLING-8375) before I posted this.
My goal is to have a reliable roundtrip on the cq:Page level in AEM. I agree that a universal solution is hardly possible, but cq:Page is , for sure. The use-case here is cq:Page nodes that have images with inline data (child nt:file under jcr:content), and I'd like them to be exportable/importable.
Sanitizing json before POSting is okay , I already do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-11891 SlingPostServlet: Support importing binary data from json [sling-org-apache-sling-jcr-contentloader]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #22:
URL: https://github.com/apache/sling-org-apache-sling-jcr-contentloader/pull/22#issuecomment-1761834389
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&metric=new_coverage&view=list) [88.2% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-contentloader&pullRequest=22&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org