Merge pull request #878 from anuchandy/checkgraph

DAG improvements

azure-mgmt-resources/src/main/java/com/microsoft/azure/management/resources/fluentcore/model/implementation/CreatableImpl.java

@@ -33,33 +33,7 @@ protected CreatableImpl(String name, InnerModelT innerObject) {
     }
 
     /**
-     * create this resource and creatable resources it depends on.
-     * <p>
-     * dependency resources will be created only if this is the root group otherwise
-     * it creates the main resource.
-     *
-     * @throws Exception the exception
-     */
-    protected void creatablesCreate() throws Exception {
-        if (creatableTaskGroup.isRoot()) {
-            creatableTaskGroup.prepare();
-            creatableTaskGroup.execute();
-        } else {
-            createResource();
-        }
-    }
-
-    protected ServiceCall creatablesCreateAsync(ServiceCallback<Void> callback) {
-        if (creatableTaskGroup.isRoot()) {
-            creatableTaskGroup.prepare();
-            return creatableTaskGroup.executeAsync(callback);
-        } else {
-            return createResourceAsync(callback);
-        }
-    }
-
-    /**
-     * add a creatable resource dependency for this resource.
+     * Add a creatable resource dependency for this resource.
      *
      * @param creatableResource the creatable dependency.
      */
@@ -84,12 +58,33 @@ protected Resource createdResource(String key) {
 
     /**
      * Default implementation of create().
+     *
      * @return the created resource
      * @throws Exception when anything goes wrong
      */
     @SuppressWarnings("unchecked")
     public FluentModelImplT create() throws Exception {
-        if (creatableTaskGroup.isRoot()) {
+        // This method get's called in two ways:
+        // 1. User explicitly call Creatable::create requesting creation of the resource.
+        // 2. Gets called as a part of creating dependent resources for the resource user requested to create in #1.
+        //
+        // The creatableTaskGroup of the 'Creatable' on which user called 'create' (#1) is known as the preparer.
+        // Preparer is the one responsible for preparing the underlying DAG for traversal.
+        //
+        // Initially creatableTaskGroup of all creatables as preparer, but as soon as user calls Create in one of
+        // them (say A::Create) all other creatables that A depends on will be marked as non-preparer.
+        //
+        // This achieve two goals:
+        //
+        // a. When #2 happens we know group is already prepared and all left is to create the currently requested resource.
+        // b. User can call 'Create' on any of the creatables not just the ROOT creatable. [ROOT is the one who does not
+        //    have any dependent]
+        //
+        // After the creation of each resource in the creatableTaskGroup owned by the user chosen Creatable (#1), each
+        // sub-creatableTaskGroup of the created resource will be marked back as preparer. Hence user can again call
+        // Update on any of these resources [which is nothing but equivalent to calling create again]
+        //
+        if (creatableTaskGroup.isPreparer()) {
             creatableTaskGroup.prepare();
             creatableTaskGroup.execute();
         } else {
@@ -106,7 +101,7 @@ public FluentModelImplT create() throws Exception {
      */
     @SuppressWarnings("unchecked")
     public ServiceCall createAsync(ServiceCallback<FluentModelT> callback) {
-        if (creatableTaskGroup.isRoot()) {
+        if (creatableTaskGroup.isPreparer()) {
             creatableTaskGroup.prepare();
             return creatableTaskGroup.executeAsync(Utils.toVoidCallback((FluentModelT) this, callback));
         } else {
@@ -117,9 +112,14 @@ public ServiceCall createAsync(ServiceCallback<FluentModelT> callback) {
     /**
      * Creates this resource.
      *
-     * @throws Exception the exception
+     * @throws Exception when anything goes wrong
      */
     protected abstract void createResource() throws Exception;
 
+    /**
+     * Creates this resource asynchronously.
+     *
+     * @throws Exception when anything goes wrong
+     */
     protected abstract ServiceCall createResourceAsync(ServiceCallback<Void> callback);
 }

azure-mgmt-resources/src/main/java/com/microsoft/azure/management/resources/fluentcore/model/implementation/CreatableTaskGroup.java

@@ -54,8 +54,8 @@ public CreatableTaskGroup(String key, CreatableTaskItem rootTask, RootResourceCr
     /**
      * Gets a resource created by a creatable task in this group.
      * <p>
-     * this method can null if the resource has not yet created that happens if the responsible task is not
-     * yet selected for execution or it's it progress
+     * This method can return null if the resource has not yet created that happens if the responsible task
+     * is not yet selected for execution or it's it progress
      *
      * @param key the resource id
      * @return the created resource

azure-mgmt-resources/src/main/java/com/microsoft/azure/management/resources/fluentcore/model/implementation/CreatableTaskItem.java

@@ -33,7 +33,12 @@ public Resource result() {
 
     @Override
     public void execute(TaskGroup<Resource, TaskItem<Resource>> taskGroup, DAGNode<TaskItem<Resource>> node) throws Exception {
-        this.created = this.creatable.create();
+        if (this.created == null) {
+            // execute will be called both in update and create scenarios,
+            // so run the task only if it not not executed already.
+            this.created = this.creatable.create();
+        }
+
         taskGroup.dag().reportedCompleted(node);
         taskGroup.execute();
     }

azure-samples/src/main/java/com/microsoft/azure/management/network/samples/ManageNetworkInterface.java

@@ -14,6 +14,7 @@
 import com.microsoft.azure.management.network.Network;
 import com.microsoft.azure.management.network.NetworkInterface;
 import com.microsoft.azure.management.network.NetworkInterfaces;
+import com.microsoft.azure.management.resources.ResourceGroup;
 import com.microsoft.azure.management.resources.fluentcore.arm.Region;
 import com.microsoft.azure.management.resources.fluentcore.utils.ResourceNamer;
 import com.microsoft.azure.management.samples.Utils;
@@ -58,7 +59,7 @@ public static void main(String[] args) {
 
             Azure azure = Azure
                     .configure()
-                    .withLogLevel(HttpLoggingInterceptor.Level.BASIC)
+                    .withLogLevel(HttpLoggingInterceptor.Level.BODY)
                     .authenticate(credFile)
                     .withDefaultSubscription();
 
@@ -70,17 +71,23 @@ public static void main(String[] args) {
                 //============================================================
                 // Create a virtual machine with multiple network interfaces
 
-                // Define a virtual network for the VMs in this availability set
+                // Define a resource group for holding all the resources
+                ResourceGroup.DefinitionCreatable resourceGroup = azure.resourceGroups()
+                        .define(rgName)
+                        .withRegion(Region.US_EAST);
+
+                // Define a virtual network for the NiCs
                 Network.DefinitionStages.WithCreate network = azure.networks()
                         .define(vnetName)
                         .withRegion(Region.US_EAST)
-                        .withNewGroup(rgName)
-                        .withAddressSpace("10.0.0.0/28");
+                        .withNewGroup(resourceGroup)
+                        .withAddressSpace("10.0.0.0/28")
+                        .withSubnet("subnet1", "10.0.0.0/28");
 
                 System.out.println("Creating multiple network interfaces");
                 NetworkInterface networkInterface1 = azure.networkInterfaces().define(networkInterfaceName1)
                         .withRegion(Region.US_EAST)
-                        .withExistingGroup(rgName)
+                        .withNewGroup(resourceGroup)
                         .withNewPrimaryNetwork(network)
                         .withPrimaryPrivateIpAddressDynamic()
                         .withNewPrimaryPublicIpAddress(publicIpAddressLeafDNS1)

runtimes/azure-client-runtime/src/main/java/com/microsoft/azure/DAGNode.java

@@ -8,6 +8,7 @@
 package com.microsoft.azure;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -18,6 +19,7 @@
 public class DAGNode<T> extends Node<T> {
     private List<String> dependentKeys;
     private int toBeResolved;
+    private boolean isPreparer;
 
     /**
      * Creates a DAG node.
@@ -34,11 +36,11 @@ public DAGNode(String key, T data) {
      * @return a list of keys of nodes in {@link DAGraph} those are dependents on this node
      */
     List<String> dependentKeys() {
-        return this.dependentKeys;
+        return Collections.unmodifiableList(this.dependentKeys);
     }
 
     /**
-     * mark the node identified by the given key as dependent of this node.
+     * Mark the node identified by the given key as dependent of this node.
      *
      * @param key the id of the dependent node
      */
@@ -54,7 +56,7 @@ public List<String> dependencyKeys() {
     }
 
     /**
-     * mark the node identified by the given key as this node's dependency.
+     * Mark the node identified by the given key as this node's dependency.
      *
      * @param dependencyKey the id of the dependency node
      */
@@ -70,10 +72,27 @@ public boolean hasDependencies() {
     }
 
     /**
-     * prepare the node for traversal.
+     * Mark or un-mark this node as preparer.
+     *
+     * @param isPreparer <tt>true</tt> if this node needs to be marked as preparer, <tt>false</tt> otherwise.
+     */
+    public void setPreparer(boolean isPreparer) {
+        this.isPreparer = isPreparer;
+    }
+
+    /**
+     * @return <tt>true</tt> if this node is marked as preparer
+     */
+    public boolean isPreparer() {
+        return isPreparer;
+    }
+
+    /**
+     * Initialize the node so that traversal can be performed on the parent DAG.
      */
-    public void prepare() {
+    public void initialize() {
         this.toBeResolved = this.dependencyKeys().size();
+        this.dependentKeys.clear();
     }
 
     /**

runtimes/azure-client-runtime/src/main/java/com/microsoft/azure/DAGraph.java

@@ -32,6 +32,7 @@ public class DAGraph<T, U extends DAGNode<T>> extends Graph<T, U> {
     public DAGraph(U rootNode) {
         this.rootNode = rootNode;
         this.queue = new ArrayDeque<>();
+        this.rootNode.setPreparer(true);
         this.addNode(rootNode);
     }
 
@@ -52,18 +53,25 @@ public boolean isRootNode(U node) {
         return this.rootNode == node;
     }
 
+    /**
+     * @return <tt>true</tt> if this dag is the preparer responsible for
+     * preparing the DAG for traversal.
+     */
+    public boolean isPreparer() {
+        return this.rootNode.isPreparer();
+    }
+
     /**
      * Merge this DAG with another DAG.
      * <p>
-     * this will mark this DAG as a child DAG, the dependencies of nodes in this DAG will be merged
+     * This will mark this DAG as a child DAG, the dependencies of nodes in this DAG will be merged
      * with (copied to) the parent DAG
      *
      * @param parent the parent DAG
      */
     public void merge(DAGraph<T, U> parent) {
         this.hasParent = true;
         parent.rootNode.addDependency(this.rootNode.key());
-        this.rootNode.addDependent(parent.rootNode.key());
         for (Map.Entry<String, U> entry: graph.entrySet()) {
             String key = entry.getKey();
             if (!parent.graph.containsKey(key)) {
@@ -77,23 +85,23 @@ public void merge(DAGraph<T, U> parent) {
      * in the DAG with no dependencies.
      */
     public void prepare() {
-        for (Map.Entry<String, U> entry: graph.entrySet()) {
-            entry.getValue().prepare();
-        }
-
-        initializeQueue();
-        if (queue.isEmpty()) {
-            throw new RuntimeException("Found circular dependency");
+        if (isPreparer()) {
+            for (U node : graph.values()) {
+                // Prepare each node for traversal
+                node.initialize();
+                // Mark other sub-DAGs are non-preparer
+                node.setPreparer(false);
+            }
+            initializeDependentKeys();
+            initializeQueue();
         }
     }
 
     /**
      * Gets next node in the DAG which has no dependency or all of it's dependencies are resolved and
      * ready to be consumed.
-     * <p>
-     * null will be returned when all the nodes are explored
      *
-     * @return next node
+     * @return next node or null if all the nodes have been explored
      */
     public U getNext() {
         return graph.get(queue.poll());
@@ -115,6 +123,7 @@ public T getNodeData(String key) {
      * @param completed the node ready to be consumed
      */
     public void reportedCompleted(U completed) {
+        completed.setPreparer(true);
         String dependency = completed.key();
         for (String dependentKey : graph.get(dependency).dependentKeys()) {
             DAGNode<T> dependent = graph.get(dependentKey);
@@ -126,27 +135,25 @@ public void reportedCompleted(U completed) {
     }
 
     /**
-     * populate dependents of all nodes.
+     * Initializes dependents of all nodes.
      * <p>
-     * the DAG will be explored in DFS order and all node's dependents will be identified,
+     * The DAG will be explored in DFS order and all node's dependents will be identified,
      * this prepares the DAG for traversal using getNext method, each call to getNext returns next node
      * in the DAG with no dependencies.
      */
-    public void populateDependentKeys() {
-        this.queue.clear();
+    private void initializeDependentKeys() {
         visit(new Visitor<U>() {
+            // This 'visit' will be called only once per each node.
             @Override
             public void visit(U node) {
                 if (node.dependencyKeys().isEmpty()) {
-                    queue.add(node.key());
                     return;
                 }
 
                 String dependentKey = node.key();
                 for (String dependencyKey : node.dependencyKeys()) {
                     graph.get(dependencyKey)
-                            .dependentKeys()
-                            .add(dependentKey);
+                            .addDependent(dependentKey);
                 }
             }
         });
@@ -163,5 +170,8 @@ private void initializeQueue() {
                 this.queue.add(entry.getKey());
             }
         }
+        if (queue.isEmpty()) {
+            throw new RuntimeException("Found circular dependency");
+        }
     }
 }

runtimes/azure-client-runtime/src/main/java/com/microsoft/azure/Graph.java

@@ -15,7 +15,7 @@
 /**
  * Type representing a directed graph data structure.
  * <p>
- * each node in a graph is represented by {@link Node}
+ * Each node in a graph is represented by {@link Node}
  *
  * @param <T> the type of the data stored in the graph's nodes
  * @param <U> the type of the nodes in the graph

runtimes/azure-client-runtime/src/main/java/com/microsoft/azure/Node.java

@@ -8,6 +8,7 @@
 package com.microsoft.azure;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -57,7 +58,7 @@ public boolean hasChildren() {
      * @return children (neighbours) of this node
      */
     public List<String> children() {
-        return this.children;
+        return Collections.unmodifiableList(this.children);
     }
 
     /**