7160252: (prefs) NodeAddedEvent was not delivered when new node add when new Node

Summary: Change native code to convey to Java code whether a new node was added
Reviewed-by: alanb, chegar

src/macosx/classes/java/util/prefs/MacOSXPreferences.java

@@ -35,16 +35,16 @@ class MacOSXPreferences extends AbstractPreferences {
     private static final String defaultAppName = "com.apple.java.util.prefs";
 
     // true if this node is a child of userRoot or is userRoot
-    private boolean isUser;
+    private final boolean isUser;
 
     // true if this node is userRoot or systemRoot
-    private boolean isRoot;
+    private final boolean isRoot;
 
     // CF's storage location for this node and its keys
-    private MacOSXPreferencesFile file;
+    private final MacOSXPreferencesFile file;
 
     // absolutePath() + "/"
-    private String path;
+    private final String path;
 
     // User root and system root nodes
     private static MacOSXPreferences userRoot = null;
@@ -73,36 +73,40 @@ class MacOSXPreferences extends AbstractPreferences {
 
     // Create a new root node. Called by getUserRoot() and getSystemRoot()
     // Synchronization is provided by the caller.
-    private MacOSXPreferences(boolean newIsUser)
-    {
-        super(null, "");
-        isUser = newIsUser;
-        isRoot = true;
-
-        initFields();
+    private MacOSXPreferences(boolean newIsUser) {
+        this(null, "", false, true, newIsUser);
     }
 
 
     // Create a new non-root node with the given parent.
     // Called by childSpi().
-    private MacOSXPreferences(MacOSXPreferences parent, String name)
-    {
-        super(parent, name);
-        isUser = isUserNode();
-        isRoot = false;
-
-        initFields();
+    private MacOSXPreferences(MacOSXPreferences parent, String name) {
+        this(parent, name, false, false, false);
     }
 
+    private MacOSXPreferences(MacOSXPreferences parent, String name,
+                              boolean isNew)
+    {
+        this(parent, name, isNew, false, false);
+    }
 
-    private void initFields()
+    private MacOSXPreferences(MacOSXPreferences parent, String name,
+                              boolean isNew, boolean isRoot, boolean isUser)
     {
+        super(parent, name);
+        this.isRoot = isRoot;
+        if (isRoot)
+            this.isUser = isUser;
+        else
+            this.isUser = isUserNode();
         path = isRoot ? absolutePath() : absolutePath() + "/";
         file = cfFileForNode(isUser);
-        newNode = file.addNode(path);
+        if (isNew)
+            newNode = isNew;
+        else
+            newNode = file.addNode(path);
     }
 
-
     // Create and return the MacOSXPreferencesFile for this node.
     // Does not write anything to the file.
     private MacOSXPreferencesFile cfFileForNode(boolean isUser)
@@ -160,7 +164,7 @@ class MacOSXPreferences extends AbstractPreferences {
     // AbstractPreferences implementation
     @Override
     protected void removeNodeSpi()
-        throws BackingStoreException
+    throws BackingStoreException
     {
         // Disallow flush or sync between these two operations
         // (they may be manipulating two different files)
@@ -180,7 +184,7 @@ class MacOSXPreferences extends AbstractPreferences {
     // AbstractPreferences implementation
     @Override
     protected String[] childrenNamesSpi()
-        throws BackingStoreException
+    throws BackingStoreException
     {
         String[] result = file.getChildrenForNode(path);
         if (result == null) throw new BackingStoreException("Couldn't get list of children for node '" + path + "'");
@@ -190,7 +194,7 @@ class MacOSXPreferences extends AbstractPreferences {
     // AbstractPreferences implementation
     @Override
     protected String[] keysSpi()
-        throws BackingStoreException
+    throws BackingStoreException
     {
         String[] result = file.getKeysForNode(path);
         if (result == null) throw new BackingStoreException("Couldn't get list of keys for node '" + path + "'");
@@ -204,15 +208,15 @@ class MacOSXPreferences extends AbstractPreferences {
         // Add to parent's child list here and disallow sync
         // because parent and child might be in different files.
         synchronized(MacOSXPreferencesFile.class) {
-            file.addChildToNode(path, name);
-            return new MacOSXPreferences(this, name);
+            boolean isNew = file.addChildToNode(path, name);
+            return new MacOSXPreferences(this, name, isNew);
         }
     }
 
     // AbstractPreferences override
     @Override
     public void flush()
-        throws BackingStoreException
+    throws BackingStoreException
     {
         // Flush should *not* check for removal, unlike sync, but should
         // prevent simultaneous removal.
@@ -227,7 +231,7 @@ class MacOSXPreferences extends AbstractPreferences {
     // AbstractPreferences implementation
     @Override
     protected void flushSpi()
-        throws BackingStoreException
+    throws BackingStoreException
     {
         // nothing here - overridden flush() doesn't call this
     }
@@ -235,7 +239,7 @@ class MacOSXPreferences extends AbstractPreferences {
     // AbstractPreferences override
     @Override
     public void sync()
-        throws BackingStoreException
+    throws BackingStoreException
     {
         synchronized(lock) {
             if (isRemoved())
@@ -256,7 +260,7 @@ class MacOSXPreferences extends AbstractPreferences {
     // AbstractPreferences implementation
     @Override
     protected void syncSpi()
-        throws BackingStoreException
+    throws BackingStoreException
     {
         // nothing here - overridden sync() doesn't call this
     }

src/macosx/classes/java/util/prefs/MacOSXPreferencesFile.java

@@ -360,11 +360,11 @@ class MacOSXPreferencesFile {
         }
     }
 
-    void addChildToNode(String path, String child)
+    boolean addChildToNode(String path, String child)
     {
         synchronized(MacOSXPreferencesFile.class) {
             markChanged();
-            addChildToNode(path, child+"/", appName, user, host);
+            return addChildToNode(path, child+"/", appName, user, host);
         }
     }
 
@@ -433,7 +433,7 @@ class MacOSXPreferencesFile {
         addNode(String path, String name, long user, long host);
     private static final native void
         removeNode(String path, String name, long user, long host);
-    private static final native void
+    private static final native boolean
         addChildToNode(String path, String child,
                        String name, long user, long host);
     private static final native void

src/macosx/native/java/util/MacOSXPreferencesFile.m

@@ -641,7 +641,7 @@ Java_java_util_prefs_MacOSXPreferencesFile_removeNode
 
 
 // child must end with '/'
-JNIEXPORT void JNICALL
+JNIEXPORT Boolean JNICALL
 Java_java_util_prefs_MacOSXPreferencesFile_addChildToNode
 (JNIEnv *env, jobject klass, jobject jpath, jobject jchild,
  jobject jname, jlong juser, jlong jhost)
@@ -656,6 +656,7 @@ Java_java_util_prefs_MacOSXPreferencesFile_addChildToNode
     CFDictionaryRef node;
     CFStringRef topKey;
     CFMutableDictionaryRef topValue;
+    Boolean beforeAdd = false;
 
     if (!path  ||  !child  ||  !name) goto badparams;
 
@@ -665,9 +666,12 @@ Java_java_util_prefs_MacOSXPreferencesFile_addChildToNode
     // copyMutableNode creates the node if necessary
     parent = copyMutableNode(path, name, user, host, &topKey, &topValue);
     throwIfNull(parent, "copyMutableNode failed");
-
+    beforeAdd = CFDictionaryContainsKey(parent, child);
     CFDictionaryAddValue(parent, child, node);
-
+    if (!beforeAdd)
+        beforeAdd = CFDictionaryContainsKey(parent, child);
+    else
+        beforeAdd = false;
     CFPreferencesSetValue(topKey, topValue, name, user, host);
 
     CFRelease(parent);
@@ -680,6 +684,7 @@ Java_java_util_prefs_MacOSXPreferencesFile_addChildToNode
     if (path) CFRelease(path);
     if (child) CFRelease(child);
     if (name) CFRelease(name);
+    return beforeAdd;
 }
 
 

test/java/util/prefs/AddNodeChangeListener.java

+/*
+ * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+ /* @test
+  * @bug  7160252
+  * @summary Checks if events are delivered to a listener
+  *          when a child node is added or removed
+  */
+
+import java.util.prefs.*;
+
+ public class AddNodeChangeListener {
+
+     private static boolean failed = false;
+     private static Preferences userRoot, N2;
+     private static NodeChangeListenerAdd ncla;
+
+     public static void main(String[] args)
+         throws BackingStoreException, InterruptedException
+     {
+        userRoot = Preferences.userRoot();
+        ncla = new NodeChangeListenerAdd();
+        userRoot.addNodeChangeListener(ncla);
+        //Should initiate a node added event
+        addNode();
+        // Should not initiate a node added event
+        addNode();
+        //Should initate a child removed event
+        removeNode();
+
+        if (failed)
+            throw new RuntimeException("Failed");
+    }
+
+    private static void addNode()
+        throws BackingStoreException, InterruptedException
+    {
+        N2 = userRoot.node("N2");
+        userRoot.flush();
+        Thread.sleep(3000);
+        if (ncla.getAddNumber() != 1)
+            failed = true;
+    }
+
+    private static void removeNode()
+        throws BackingStoreException, InterruptedException
+    {
+        N2.removeNode();
+        userRoot.flush();
+        Thread.sleep(3000);
+        if (ncla.getAddNumber() != 0)
+            failed = true;
+    }
+
+    private static class NodeChangeListenerAdd implements NodeChangeListener {
+        private int totalNode = 0;
+
+        @Override
+        public void childAdded(NodeChangeEvent evt) {
+            totalNode++;
+        }
+
+        @Override
+        public void childRemoved(NodeChangeEvent evt) {
+            totalNode--;
+        }
+
+        public int getAddNumber(){
+            return totalNode;
+        }
+    }
+ }