Mailing-List: contact dev-help@brooklyn.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@brooklyn.apache.org
From: iyovcheva <git@git.apache.org>
To: dev@brooklyn.apache.org
Reply-To: dev@brooklyn.apache.org
References: <git-pr-272-brooklyn-server@git.apache.org>
In-Reply-To: <git-pr-272-brooklyn-server@git.apache.org>
Subject: [GitHub] brooklyn-server pull request #272: orphaned-state deletion: keep PortForward...
Content-Type: text/plain
Message-Id: <20160722185952.440DEE058E@git1-us-west.apache.org>
Date: Fri, 22 Jul 2016 18:59:52 +0000 (UTC)
archived-at: Fri, 22 Jul 2016 18:59:54 -0000

Github user iyovcheva commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/272#discussion_r71928836
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/impl/DeleteOrphanedStateTransformer.java ---
    @@ -91,93 +107,219 @@ public BrooklynMementoRawData transform(BrooklynMementoRawData input) {
                     .build();
         }
     
    -    protected Set<String> findAllReferencedEnrichers(BrooklynMementoRawData input) {
    -        return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/enrichers/string");
    -    }
    -
    -    protected Set<String> findAllReferencedPolicies(BrooklynMementoRawData input) {
    -        return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/policies/string");
    -    }
    -
    -    protected Set<String> findAllReferencedFeeds(BrooklynMementoRawData input) {
    -        return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/feeds/string");
    -    }
    -
    -    protected Set<String> findAllReferencedAdjuncts(Map<String, String> items, String xpath) {
    -        Set<String> result = Sets.newLinkedHashSet();
    -
    -        for(Map.Entry<String, String> entry : items.entrySet()) {
    -            result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), xpath, XPathConstants.NODESET)));
    +    /**
    +     * Searches the state, based on using xpath to find references, starting at the roots of
    +     * reachability. 
    +     * 
    +     * For locations, we find all locations referenced by entities, enrichers, policies or feeds 
    +     * (looking at the xpath for the locations or location refs in the persisted state). We then 
    +     * search through those locations for other locations reachable by them, and so on.
    +     * 
    +     * We also keep any location that is of type {@link PortForwardManager}.
    +     */
    +    protected static class ReachabilityXpathInspector {
    +        public ReferencedState inspect(BrooklynMementoRawData input) {
    +            return new ReferencedState()
    +                    .locations(findAllReferencedLocations(input))
    +                    .enrichers(findAllReferencedEnrichers(input))
    +                    .policies(findAllReferencedPolicies(input))
    +                    .feeds(findAllReferencedFeeds(input));
             }
     
    -        return result;
    -    }
    -
    -    @VisibleForTesting
    -    public Map<String, String> findLocationsToKeep(BrooklynMementoRawData input) {
    -        Set<String> allReferencedLocations = findAllReferencedLocations(input);
    -        return copyRetainingKeys(input.getLocations(), allReferencedLocations);
    -    }
    -
    -    @VisibleForTesting
    -    public Set<String> findAllReferencedLocations(BrooklynMementoRawData input) {
    -        Set<String> result = Sets.newLinkedHashSet();
    -
    -        result.addAll(searchLocationsToKeep(input.getEntities(), "/entity"));
    -        result.addAll(searchLocationsToKeep(input.getPolicies(), "/policy"));
    -        result.addAll(searchLocationsToKeep(input.getEnrichers(), "/enricher"));
    -        result.addAll(searchLocationsToKeep(input.getFeeds(), "/feed"));
    -        result.addAll(searchLocationsToKeepInLocations(input.getLocations(), result));
    -
    -        return result;
    -    }
    -
    -    protected Set<String> searchLocationsToKeep(Map<String, String> items, String searchInTypePrefix) {
    -        String locationsXpath = searchInTypePrefix+"/locations/string";
    -        String locationProxyXpath = searchInTypePrefix+"//locationProxy";
    -
    -        Set<String> result = Sets.newLinkedHashSet();
    -
    -        for(Map.Entry<String, String> entry : items.entrySet()) {
    -            result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), locationsXpath, XPathConstants.NODESET)));
    -            result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), locationProxyXpath, XPathConstants.NODESET)));
    +        protected Set<String> findAllReferencedEnrichers(BrooklynMementoRawData input) {
    +            return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/enrichers/string");
    +        }
    +    
    +        protected Set<String> findAllReferencedPolicies(BrooklynMementoRawData input) {
    +            return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/policies/string");
    +        }
    +    
    +        protected Set<String> findAllReferencedFeeds(BrooklynMementoRawData input) {
    +            return findAllReferencedAdjuncts(input.getEntities(), "entity" + "/feeds/string");
    +        }
    +    
    +        protected Set<String> findAllReferencedAdjuncts(Map<String, String> items, String xpath) {
    +            Set<String> result = Sets.newLinkedHashSet();
    +    
    +            for(Map.Entry<String, String> entry : items.entrySet()) {
    +                result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), xpath, XPathConstants.NODESET)));
    +            }
    +    
    +            return result;
    +        }
    +    
    +        protected Set<String> findAllReferencedLocations(BrooklynMementoRawData input) {
    +            Set<String> result = Sets.newLinkedHashSet();
    +    
    +            result.addAll(searchLocationsToKeep(input.getEntities(), "/entity"));
    +            result.addAll(searchLocationsToKeep(input.getPolicies(), "/policy"));
    +            result.addAll(searchLocationsToKeep(input.getEnrichers(), "/enricher"));
    +            result.addAll(searchLocationsToKeep(input.getFeeds(), "/feed"));
    +            result.addAll(searchLocationsToKeepInLocations(input.getLocations(), result));
    +    
    +            return result;
    +        }
    +    
    +        protected Set<String> searchLocationsToKeep(Map<String, String> items, String searchInTypePrefix) {
    +            String locationsXpath = searchInTypePrefix+"/locations/string";
    +            String locationProxyXpath = searchInTypePrefix+"//locationProxy";
    +    
    +            Set<String> result = Sets.newLinkedHashSet();
    +    
    +            for(Map.Entry<String, String> entry : items.entrySet()) {
    +                result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), locationsXpath, XPathConstants.NODESET)));
    +                result.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(entry.getValue(), locationProxyXpath, XPathConstants.NODESET)));
    +            }
    +    
    +            return result;
    +        }
    +    
    +        protected Set<String> searchLocationsToKeepInLocations(Map<String, String> locations, Set<String> alreadyReferencedLocations) {
    +            Set<String> result = Sets.newLinkedHashSet();
    +    
    +            String prefix = "/location";
    +            String locationTypeXpath = prefix+"/type";
    +            String locationChildrenXpath = prefix+"/children/string";
    +            String locationParentDirectTagXpath = prefix+"/parent";
    +            String locationProxyXpath = prefix+"//locationProxy";
    +    
    +            Set<String> locsToInspect = MutableSet.copyOf(alreadyReferencedLocations);
    +    
    +            // Keep org.apache.brooklyn.core.location.access.PortForwardManager, even if not referenced.
    +            // It is found and loaded by PortForwardManagerLocationResolver.
    +            for (Map.Entry<String, String> entry : locations.entrySet()) {
    +                String locId = entry.getKey();
    +                if (!alreadyReferencedLocations.contains(locId)) {
    +                    String locType = XmlUtil.xpath(entry.getValue(), locationTypeXpath);
    +                    if (locType != null && locType.contains("PortForwardManager")) {
    +                        result.add(locId);
    +                        locsToInspect.add(locId);
    +                    }
    +                }
    +            }
    +    
    +            while (locsToInspect.size() > 0) {
    +                Set<String> referencedLocs = Sets.newLinkedHashSet();
    +                for (String id : locsToInspect) {
    +                    String xmlData = locations.get(id);
    +                    if (xmlData != null) {
    +                        referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationChildrenXpath, XPathConstants.NODESET)));
    +                        referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationParentDirectTagXpath, XPathConstants.NODESET)));
    +                        referencedLocs.addAll(getAllNodesFromXpath((org.w3c.dom.NodeList) XmlUtil.xpath(xmlData, locationProxyXpath, XPathConstants.NODESET)));
    +                    }
    +                }
    +                Set<String> newlyDiscoveredLocs = MutableSet.<String>builder()
    +                        .addAll(referencedLocs)
    +                        .removeAll(alreadyReferencedLocations)
    +                        .removeAll(result)
    +                        .build();
    +                result.addAll(newlyDiscoveredLocs);
    +                locsToInspect = newlyDiscoveredLocs;
    +            }
    +    
    +            return result;
             }
    +        
    +        protected Set<String> getAllNodesFromXpath(org.w3c.dom.NodeList nodeList) {
    +            Set<String> result = Sets.newLinkedHashSet();
     
    -        return result;
    +            for (int i = 0; i < nodeList.getLength(); i++) {
    +                Node nextNode = nodeList.item(i);
    +                if (nextNode != null) {
    +                    result.add(nextNode.getTextContent());
    +                }
    +            }
    +            
    +            return result;
    +        }
         }
    -
    -    protected Set<String> searchLocationsToKeepInLocations(Map<String, String> locations, Set<String> alreadyReferencedLocations) {
    -        Set<String> result = Sets.newLinkedHashSet();
    -
    -        String prefix = "/location";
    -        String locationChildrenXpath = prefix+"/children/string";
    -        String locationParentDirectTagXpath = prefix+"/parent";
    -        String locationProxyXpath = prefix+"//locationProxy";
    -
    -        Set<String> locsToInspect = alreadyReferencedLocations;
    +    
    +    /**
    +     * Searches the state, based on state.contains(id). We don't care about the structure or where
    +     * in the state the id was mentioned.
    +     * 
    +     * The rules of reachability (in terms of the roots used etc) are the same as for 
    +     * {@link ReachabilityXpathInspector}.
    +     */
    +    protected static class ReachabilityGrepInspector {
    +        protected ReferencedState inspect(BrooklynMementoRawData input) {
    +            Set<String> locations = Sets.newLinkedHashSet();
    +            Set<String> feeds = Sets.newLinkedHashSet();
    +            Set<String> enrichers = Sets.newLinkedHashSet();
    +            Set<String> policies = Sets.newLinkedHashSet();
    +
    +            for (String id : input.getEnrichers().keySet()) {
    +                if (isMentionedBy(id, BrooklynObjectType.ENTITY, input)) {
    +                    enrichers.add(id);
    +                }
    +            }
    +            for (String id : input.getPolicies().keySet()) {
    +                if (isMentionedBy(id, BrooklynObjectType.ENTITY, input)) {
    +                    policies.add(id);
    +                }
    +            }
    +            for (String id : input.getFeeds().keySet()) {
    +                if (isMentionedBy(id, BrooklynObjectType.ENTITY, input)) {
    +                    feeds.add(id);
    +                }
    +            }
    +            
    +            // Initial pass of locations (from the roots of reachability)
    +            for (Map.Entry<String, String> entry : input.getLocations().entrySet()) {
    +                String id = entry.getKey();
    +                String locationState = entry.getValue();
    +                if (locationState.contains("PortForwardManager")) {
    +                    locations.add(id);
    --- End diff --
    
    Will we keep all locations of that type? Can't we search by <pfm> reference?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---