Terracotta-OSS / statistics

A statistics framework used inside Ehcache and the Terracotta products
Apache License 2.0
5 stars 18 forks source link

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

Open mathieucarbou opened 7 years ago

mathieucarbou commented 7 years ago

This issue has been extracted from https://github.com/Terracotta-OSS/statistics/pull/27. See https://github.com/Terracotta-OSS/statistics/pull/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 https://github.com/Terracotta-OSS/terracotta-platform/pull/263)

mathieucarbou commented 7 years ago

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 ?