Re: [PR] SLING-12283 : write config in correct format incase of factoryPid is … [sling-org-apache-sling-installer-provider-jcr]
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]
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]
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]
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]
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]
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]
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]
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]
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
