Prevent nodes without registered entry points from being stored

[sphuber]

Fixes #3457

Up until now it was possible to store instances of Node subclasses
that do not have a registered entry point. Imagine for example the
definition of a Data subclass called SubClass in a shell and an
instance being stored. The node type string will be:

__main__.SubClass.

When trying to load this node at a later point in time, the type string
can of course not be resolved to an importable class and the loader
would raise an exception.

The first change is that this exception is now turned into a warning and
the loader falls back onto the Data class. Note that we do not use the
Node class for this as this base class is also not storable and so the
ORM logic is potentially ill-defined for instances of the base Node.

Secondly, we now add a check to Node.store to make sure that the class
corresponds to a registered entry point. If this is not the case, the
store method will raise StoringNotAllowed.

One unit test was deleted because its implementation was incorrect and
was not actually testing what it was intending to test. Besides intended
functionality is now covered by other tests.

[greschd]

Nice, thanks! Probably @giovannipizzi should give it a quick look also, to make sure he's on board with the new behavior.

[codecov]

Codecov Report

Merging #3886 into develop will decrease coverage by 0.00%.
The diff coverage is 91.30%.

@@             Coverage Diff             @@
##           develop    #3886      +/-   ##
===========================================
- Coverage    77.18%   77.18%   -0.01%     
===========================================
  Files          457      457              
  Lines        33778    33795      +17     
===========================================
+ Hits         26072    26084      +12     
- Misses        7706     7711       +5     

Flag	Coverage Δ
#django	69.22% <91.30%> (+0.01%)	⬆️
#sqlalchemy	70.04% <91.30%> (+<0.01%)	⬆️

Impacted Files	Coverage Δ
aiida/plugins/entry_point.py	88.07% <81.81%> (+0.07%)	⬆️
aiida/orm/nodes/node.py	90.86% <100.00%> (+0.13%)	⬆️
aiida/orm/utils/node.py	92.07% <100.00%> (+1.17%)	⬆️
aiida/orm/utils/log.py	76.66% <0.00%> (-16.67%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef59281...fd15288. Read the comment docs.

[sphuber]

I have performed the following performance test on the branch of this PR and develop.
In a verdi shell:

%timeit [Int().store() for _ in range(100)

• branch: 1.72 s ± 85.7 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
• develop: 1.69 s ± 101 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

I feel this is acceptable. @giovannipizzi and @greschd this is ready for review

[greschd]

I though a bit more about how this impacts developing Data sub-classes. My conclusion is that this change is fine in that context, but I'll reproduce my thoughts here to see if you agree:

When developing the Data nodes, one might want to work in an interactive context (REPL, Jupyter). The type strings in this case will be invalid, which makes storing impossible. However, even previously these stored nodes could not be loaded back, making the storing pointless. Interactive development of the Data sub-classes can still happen on unstored nodes.

If the store / load cycle is really required in this context, one could add an escape hatch (validate_type_string flag) to the store method. But, in order to be useful, it would also require a way to load it back (like a MyDataClass.load_node(pk)). I don't think either is really necessary.