Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-24 Thread via GitHub


rishabhdaim commented on PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#issuecomment-2129190028

   > @rishabhdaim I think once you remove the original.pid part of the code, we 
could merge this PR.
   
   Done in 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9/commits/69597f866d6a5646ba334cbefb3aade499a97e1d


-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-23 Thread via GitHub


jsedding commented on code in PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1611777912


##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java:
##
@@ -707,6 +716,7 @@ private UpdateResult handleUpdate(final String resourceType,
 dataNode.setProperty(PROP_MODIFIED, Calendar.getInstance());
 dataNode.setProperty(PROP_ENC, ENCODING);
 dataNode.setProperty(PROP_MIME, MIME_TXT);
+dataNode.setProperty(ORIGINAL_PID, id);

Review Comment:
   I suggest we drop the original.pid. Instead we go with your approach of 
checking for the legacy pid 
(https://github.com/apache/sling-org-apache-sling-installer-factory-configuration/pull/13/files#diff-ddf6029cf7d9aa5ed2720412a7fe8de77d7f5df784c32864c6abc54393e78321R95-R97).
   
   That isn't quite as generic as I had originally intended, but it should be 
good enough. Unless I am missing something.



-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-23 Thread via GitHub


rishabhdaim commented on code in PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1611358227


##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrUtil.java:
##
@@ -73,4 +75,21 @@ public static Node createPath(final Session session,
 }
 return parentNode.getNode(relativePath);
 }
+
+/**
+ * Get the PID for a configuration event by using the R7 format before 
saving it.
+ *
+ * @param factoryPid factory PID of the configuration
+ * @param pid PID of the configuration
+ * @return The PID in R7 format
+ */
+public static String getPid(final String factoryPid, final String pid) {
+// if factory pid is separated from pid by a period (.), we need 
replace it with a ~ so that this can be installed
+// and grouped as a factory configuration
+if (pid.startsWith(factoryPid + '.')) {
+String id = pid.substring(factoryPid.length() + 1);
+return factoryPid + "~" + id;
+}
+return pid;

Review Comment:
   Addressed in 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9/commits/49da45e49774d6f9b8597bbc6376c527ef5970f3



-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-22 Thread via GitHub


jsedding commented on code in PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1609457830


##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrUtil.java:
##
@@ -73,4 +75,21 @@ public static Node createPath(final Session session,
 }
 return parentNode.getNode(relativePath);
 }
+
+/**
+ * Get the PID for a configuration event by using the R7 format before 
saving it.
+ *
+ * @param factoryPid factory PID of the configuration
+ * @param pid PID of the configuration
+ * @return The PID in R7 format
+ */
+public static String getPid(final String factoryPid, final String pid) {
+// if factory pid is separated from pid by a period (.), we need 
replace it with a ~ so that this can be installed
+// and grouped as a factory configuration
+if (pid.startsWith(factoryPid + '.')) {
+String id = pid.substring(factoryPid.length() + 1);
+return factoryPid + "~" + id;
+}
+return pid;

Review Comment:
   I was thinking to keep this slightly more generic (note: untested code 
below!):
   
   ```suggestion
   final String name;
   if (pid.matches("^" + factoryPid + "[^a-zA-Z0-9\\s]")) {
   name = pid.substring(factoryPid.length() + 1);
   } else {
   name = pid;
   }
   return factoryPid + "~" + name;
   ```
   
   WDYT?



-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-21 Thread via GitHub


rishabhdaim commented on code in PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608439188


##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java:
##
@@ -707,6 +716,7 @@ private UpdateResult handleUpdate(final String resourceType,
 dataNode.setProperty(PROP_MODIFIED, Calendar.getInstance());
 dataNode.setProperty(PROP_ENC, ENCODING);
 dataNode.setProperty(PROP_MIME, MIME_TXT);
+dataNode.setProperty(ORIGINAL_PID, id);

Review Comment:
   I didn't get any exceptions while creating a config on AEM 1, and it was 
successfully replicated on AEM 2. I set up both instances using Mongo document 
store.



-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-21 Thread via GitHub


rishabhdaim commented on code in PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608455987


##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrUtil.java:
##
@@ -73,4 +75,21 @@ public static Node createPath(final Session session,
 }
 return parentNode.getNode(relativePath);
 }
+
+/**
+ * Get the PID for a configuration event by using the R7 format before 
saving it.
+ *
+ * @param factoryPid factory PID of the configuration
+ * @param pid PID of the configuration
+ * @return The PID in R7 format
+ */
+public static String getPid(final String factoryPid, final String pid) {
+// if factory pid is separated from pid by a period (.), we need 
replace it with a ~ so that this can be installed
+// and grouped as a factory configuration
+if (pid.startsWith(factoryPid + '.')) {
+String id = pid.substring(factoryPid.length() + 1);
+return factoryPid + "~" + id;
+}
+return pid;

Review Comment:
   Addressed in 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9/commits/6b9cdd225224636759cd64a96328c9b573316aff



-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-21 Thread via GitHub


rishabhdaim commented on code in PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608439188


##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java:
##
@@ -707,6 +716,7 @@ private UpdateResult handleUpdate(final String resourceType,
 dataNode.setProperty(PROP_MODIFIED, Calendar.getInstance());
 dataNode.setProperty(PROP_ENC, ENCODING);
 dataNode.setProperty(PROP_MIME, MIME_TXT);
+dataNode.setProperty(ORIGINAL_PID, id);

Review Comment:
   I didn't get any exceptions while creating a config on AEM 1 and it gets 
replicated on AEM 2 successfully.



-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-21 Thread via GitHub


jsedding commented on code in PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608329623


##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrUtil.java:
##
@@ -73,4 +75,21 @@ public static Node createPath(final Session session,
 }
 return parentNode.getNode(relativePath);
 }
+
+/**
+ * Get the PID for a configuration event by using the R7 format before 
saving it.
+ *
+ * @param factoryPid factory PID of the configuration
+ * @param pid PID of the configuration
+ * @return The PID in R7 format
+ */
+public static String getPid(final String factoryPid, final String pid) {
+// if factory pid is separated from pid by a period (.), we need 
replace it with a ~ so that this can be installed
+// and grouped as a factory configuration
+if (pid.startsWith(factoryPid + '.')) {
+String id = pid.substring(factoryPid.length() + 1);
+return factoryPid + "~" + id;
+}
+return pid;

Review Comment:
   I would flip the logic. I.e. let a pid in the correct format through. If the 
format is incorrect, normalize the pid to a name by removing a prefix of 
`factoryPid` if present. Then concat the factoryPid, ~ and normalized pid.



-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]

2024-05-21 Thread via GitHub


jsedding commented on code in PR #9:
URL: 
https://github.com/apache/sling-org-apache-sling-installer-provider-jcr/pull/9#discussion_r1608319420


##
src/main/java/org/apache/sling/installer/provider/jcr/impl/JcrInstaller.java:
##
@@ -707,6 +716,7 @@ private UpdateResult handleUpdate(final String resourceType,
 dataNode.setProperty(PROP_MODIFIED, Calendar.getInstance());
 dataNode.setProperty(PROP_ENC, ENCODING);
 dataNode.setProperty(PROP_MIME, MIME_TXT);
+dataNode.setProperty(ORIGINAL_PID, id);

Review Comment:
   Does this work, seeting a property on the `jcr:content` node? I think the 
`nt:resource` node type is quite restrictive. IIRC Oak uses a different 
node-type for a file's `jcr:content` node. Maybe that one is more lenient. 
Alternatively, a mixin nodetype could be added to allow for the extra property.



-- 
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-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org