You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2021/02/09 10:27:59 UTC

[GitHub] [jackrabbit-oak] stefan-egli opened a new pull request #269: OAK-9134 : backport to 1.22 from trunk

stefan-egli opened a new pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269


    with one major difference: the default of 'oak.referenceableFrozenNode' being true in 1.22 for backwards compatibility reason - while in trunk it is false to promote the new behaviour there


----------------------------------------------------------------
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.

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



[GitHub] [jackrabbit-oak] stefan-egli commented on pull request #269: OAK-9134 : backport to 1.22 from trunk

Posted by GitBox <gi...@apache.org>.
stefan-egli commented on pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269#issuecomment-789711678


   * merged to svn 1.22 branch in rev [1887142](http://svn.apache.org/viewvc?rev=1887142&view=rev)


----------------------------------------------------------------
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.

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



[GitHub] [jackrabbit-oak] stefan-egli commented on a change in pull request #269: OAK-9134 : backport to 1.22 from trunk

Posted by GitBox <gi...@apache.org>.
stefan-egli commented on a change in pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269#discussion_r572972944



##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/write/NodeTypeRegistry.java
##########
@@ -89,9 +93,34 @@ public static void register(Root root, InputStream input, String systemId) {
 
     private void registerNodeTypes(InputStream stream, String systemId) {
         try {
-            CndImporter.registerNodeTypes(
-                    new InputStreamReader(stream, Charsets.UTF_8),
-                    systemId, ntMgr, nsReg, vf, false);
+            Reader reader = new InputStreamReader(stream, Charsets.UTF_8);
+            // OAK-9134: nt:frozenNode is not implementing mix:referenceable from JCR 2.0.
+            // This system property allows to add it back when initializing a repository.
+            final String referenceableFrozenNodeProperty = "oak.referenceableFrozenNode";
+            final boolean referenceableFrozenNode;
+            if (System.getProperty(referenceableFrozenNodeProperty) == null) {
+                // the default for referenceableFrozenNode is true in the 1.22 branch.
+                // this is in contrast to it being false in newer versions.
+                // the reason for choosing true as the default is to maintain higher
+                // backwards compatibility and minimize an otherwise high impact in this branch.
+                referenceableFrozenNode = true;
+            } else {
+                referenceableFrozenNode = Boolean.getBoolean(referenceableFrozenNodeProperty);
+            }

Review comment:
       turns out SystemPropertySupplier is in oak-store-document so not available here .. unless we move that class somewhere more generic ..




----------------------------------------------------------------
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.

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



[GitHub] [jackrabbit-oak] stefan-egli commented on a change in pull request #269: OAK-9134 : backport to 1.22 from trunk

Posted by GitBox <gi...@apache.org>.
stefan-egli commented on a change in pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269#discussion_r572928581



##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/write/NodeTypeRegistry.java
##########
@@ -89,9 +93,34 @@ public static void register(Root root, InputStream input, String systemId) {
 
     private void registerNodeTypes(InputStream stream, String systemId) {
         try {
-            CndImporter.registerNodeTypes(
-                    new InputStreamReader(stream, Charsets.UTF_8),
-                    systemId, ntMgr, nsReg, vf, false);
+            Reader reader = new InputStreamReader(stream, Charsets.UTF_8);
+            // OAK-9134: nt:frozenNode is not implementing mix:referenceable from JCR 2.0.
+            // This system property allows to add it back when initializing a repository.
+            final String referenceableFrozenNodeProperty = "oak.referenceableFrozenNode";
+            final boolean referenceableFrozenNode;
+            if (System.getProperty(referenceableFrozenNodeProperty) == null) {
+                // the default for referenceableFrozenNode is true in the 1.22 branch.
+                // this is in contrast to it being false in newer versions.
+                // the reason for choosing true as the default is to maintain higher
+                // backwards compatibility and minimize an otherwise high impact in this branch.
+                referenceableFrozenNode = true;
+            } else {
+                referenceableFrozenNode = Boolean.getBoolean(referenceableFrozenNodeProperty);
+            }

Review comment:
       +1, good point on reducing divergence




----------------------------------------------------------------
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.

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



[GitHub] [jackrabbit-oak] mreutegg commented on pull request #269: OAK-9134 : backport to 1.22 from trunk

Posted by GitBox <gi...@apache.org>.
mreutegg commented on pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269#issuecomment-775983361


   Other than above, looks good to me.


----------------------------------------------------------------
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.

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



[GitHub] [jackrabbit-oak] mreutegg commented on a change in pull request #269: OAK-9134 : backport to 1.22 from trunk

Posted by GitBox <gi...@apache.org>.
mreutegg commented on a change in pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269#discussion_r572925950



##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/write/NodeTypeRegistry.java
##########
@@ -89,9 +93,34 @@ public static void register(Root root, InputStream input, String systemId) {
 
     private void registerNodeTypes(InputStream stream, String systemId) {
         try {
-            CndImporter.registerNodeTypes(
-                    new InputStreamReader(stream, Charsets.UTF_8),
-                    systemId, ntMgr, nsReg, vf, false);
+            Reader reader = new InputStreamReader(stream, Charsets.UTF_8);
+            // OAK-9134: nt:frozenNode is not implementing mix:referenceable from JCR 2.0.
+            // This system property allows to add it back when initializing a repository.
+            final String referenceableFrozenNodeProperty = "oak.referenceableFrozenNode";
+            final boolean referenceableFrozenNode;
+            if (System.getProperty(referenceableFrozenNodeProperty) == null) {
+                // the default for referenceableFrozenNode is true in the 1.22 branch.
+                // this is in contrast to it being false in newer versions.
+                // the reason for choosing true as the default is to maintain higher
+                // backwards compatibility and minimize an otherwise high impact in this branch.
+                referenceableFrozenNode = true;
+            } else {
+                referenceableFrozenNode = Boolean.getBoolean(referenceableFrozenNodeProperty);
+            }

Review comment:
       To reduce divergence, what do you think about backporting OAK-9010 and then use a `SystemPropertySupplier`? The only difference then would be the default value when calling the factory method.




----------------------------------------------------------------
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.

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



[GitHub] [jackrabbit-oak] stefan-egli closed pull request #269: OAK-9134 : backport to 1.22 from trunk

Posted by GitBox <gi...@apache.org>.
stefan-egli closed pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269


   


----------------------------------------------------------------
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.

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



[GitHub] [jackrabbit-oak] stefan-egli commented on a change in pull request #269: OAK-9134 : backport to 1.22 from trunk

Posted by GitBox <gi...@apache.org>.
stefan-egli commented on a change in pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269#discussion_r573966047



##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/write/NodeTypeRegistry.java
##########
@@ -89,9 +93,34 @@ public static void register(Root root, InputStream input, String systemId) {
 
     private void registerNodeTypes(InputStream stream, String systemId) {
         try {
-            CndImporter.registerNodeTypes(
-                    new InputStreamReader(stream, Charsets.UTF_8),
-                    systemId, ntMgr, nsReg, vf, false);
+            Reader reader = new InputStreamReader(stream, Charsets.UTF_8);
+            // OAK-9134: nt:frozenNode is not implementing mix:referenceable from JCR 2.0.
+            // This system property allows to add it back when initializing a repository.
+            final String referenceableFrozenNodeProperty = "oak.referenceableFrozenNode";
+            final boolean referenceableFrozenNode;
+            if (System.getProperty(referenceableFrozenNodeProperty) == null) {
+                // the default for referenceableFrozenNode is true in the 1.22 branch.
+                // this is in contrast to it being false in newer versions.
+                // the reason for choosing true as the default is to maintain higher
+                // backwards compatibility and minimize an otherwise high impact in this branch.
+                referenceableFrozenNode = true;
+            } else {
+                referenceableFrozenNode = Boolean.getBoolean(referenceableFrozenNodeProperty);
+            }

Review comment:
       Created a PR suggesting such a move in https://issues.apache.org/jira/browse/OAK-9352




----------------------------------------------------------------
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.

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



[GitHub] [jackrabbit-oak] stefan-egli commented on pull request #269: OAK-9134 : backport to 1.22 from trunk

Posted by GitBox <gi...@apache.org>.
stefan-egli commented on pull request #269:
URL: https://github.com/apache/jackrabbit-oak/pull/269#issuecomment-781393852


   Adjusted to use the SystemPropertySupplier - see corresponding trunk PR https://github.com/apache/jackrabbit-oak/pull/278
   Currently that fails however, with
   ```
   caused by: Unable to resolve org.apache.jackrabbit.oak-core [32](R 32.0): 
     missing requirement [org.apache.jackrabbit.oak-core [32](R 32.0)] 
     osgi.wiring.package; (osgi.wiring.package=org.apache.jackrabbit.oak.commons.properties)
   ```


----------------------------------------------------------------
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.

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