Why does DataCache.Add() not check the real backend?

[ixje]

The DataCache class has methods like Add, Delete, Find etc.
Delete(), Find, TryGet all do the following steps

1. check internal cache
2. if not found internally -> check real backend

Add() does not check the real backend. Why?

neo/neo/IO/Caching/DataCache.cs

Lines 46 to 59 in 4112df2

	public void Add(TKey key, TValue value)
	{
	lock (dictionary)
	{
	if (dictionary.TryGetValue(key, out Trackable trackable) && trackable.State != TrackState.Deleted)
	throw new ArgumentException();
	dictionary[key] = new Trackable
	{
	Key = key,
	Item = value,
	State = trackable == null ? TrackState.Added : TrackState.Changed
	};
	}
	}

Add() throws an exception (see line 51) if we try to add something that already exists in the cache. It is a design choice to not allow overwriting values, that's fine. However, if it does not find it in the cache then it happily adds it to the cache and a call to commit() will overwrite the value in the real backend. This is inconsistent behaviour. I think there should be 1 of 2 options

1. Add() always allows overriding data. This means removing the exception.
2. Add() never allows overriding data. This means adding logic to check if a value exists in the real backend before adding it to the internal cache and throwing an exception if it exists.

Now depending if TryGet() or GetAndChange() is called before an Add() it will throw an exception or not.

[erikzhang]

I think option 2 is good.

[vncoelho]

@ixje, nice findings!
I just had time to understand it now after dedicating some time to #1318.

I think that I agree with option 2. Which one do you personally prefer?

[ixje]

I PR'ed for option 2.

Personally I made everything writable in neo3-python unless explicitly setting a read_only param to True. This is because I wanted to avoid the problem I described in #1186 and it allowed me to remove the (to me personally) confusing named GetAndChange.