Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NPE when the contextObject does not exist in the tree #28

Open
mathieucarbou opened this issue Jan 12, 2017 · 1 comment
Open

NPE when the contextObject does not exist in the tree #28

mathieucarbou opened this issue Jan 12, 2017 · 1 comment

Comments

@mathieucarbou
Copy link
Member

mathieucarbou commented Jan 12, 2017

This issue has been extracted from #27. See #27 for the full details

Would it be possible also to add a fix so that we do not have a NPE in the case the object of the query is not in the tree ?

I..e:

Instead of doing:

    TreeNode treeNode = ContextManager.nodeFor(contextObject);
    if(treeNode == null) {
      return;
    }

    Set<TreeNode> result = queryBuilder()
        .descendants()
        .filter(context(attributes(Matchers.<Map<String, Object>>allOf(
            hasAttribute("type", descriptor.getType()),
            hasAttribute("name", descriptor.getObserverName()),
            hasTags(descriptor.getTags())))))
        .filter(context(identifier(subclassOf(OperationStatistic.class))))
        .build().execute(Collections.singleton(treeNode));

We should be able to just do something like:

 Set<TreeNode> result = queryBuilder()
        .descendants()
        .filter(context(attributes(Matchers.<Map<String, Object>>allOf(
            hasAttribute("type", descriptor.getType()),
            hasAttribute("name", descriptor.getObserverName()),
            hasTags(descriptor.getTags())))))
        .filter(context(identifier(subclassOf(OperationStatistic.class))))
        .build().execute(Collections.singleton(ContextManager.nodeFor(contextObject)));

or:

        .build().execute(contextObject);

And the query system would just return an empty set of nodes because the queried objects are not there.

(related to Terracotta-OSS/terracotta-platform#263)

@mathieucarbou
Copy link
Member Author

In some case, yes it would make sense that we have for example something like: ContextManager.getNodeFor(contextObject) throw NoSuchElementException. In the case where you would want to make sure that the context object is in the tree.

But in my case, where I just want to query the tree to find some stats, I do not care if the object is there or not, I care about if I have results or not. In my case, I will have some tests elsewhere to verify that some context objects are well put in the tree and that I can have some stat descriptors associated to them. So if I send the wrong context object, and it is not in the tree, this "bug" would be catched at another place (I won't get stats or descriptors).

It's like we would need something like these perhaps ?

  • ContextManager.getNodeFor(contextObject) throw NoSuchElementException
  • .build().execute(contextObject); // would do .build().execute(asList(ContextManager.nodeFor(contextObject)));
  • .build().execute(asList(ContextManager.getNodeFor(contextObject)));
  • .build().execute(asList(ContextManager.nodeFor(contextObject))); // would tolerate nulls to not "search" childs of null root nodes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant