Gaffer icon indicating copy to clipboard operation
Gaffer copied to clipboard

Fix FederatedStore checks for DYNAMIC_SCHEMA

Open t92549 opened this issue 3 years ago • 1 comments

Due to a partial implementation of #2008, currently FederatedOperationChains only validate the View if the graph does not have the DYNAMIC_SCHEMA trait: https://github.com/gchq/Gaffer/blob/40bb337d2ca91fea2dadcd04cddec46021c169ee/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java#L82 https://github.com/gchq/Gaffer/blob/40bb337d2ca91fea2dadcd04cddec46021c169ee/store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/util/FederatedStoreUtil.java#L134

This trait only exists in the FederatedStore as its getTraits() implementation returns ALL_TRAITS. This effectively means that only FederatedStores and ProxyStores which point to a FederatedStore will not be validated here. However, this cannot be used going forward because:

  • FederatedStore fixes introduced in the FederatedStore alpha mean it will no longer return ALL_TRAITS
  • the getTraits function is being replaced by the Operation which behaves differently, instead of returning ALL_TRAITS it will use the FederatedGetTraitsHandler which correctly returns the traits that the FederatedStore's stores have

Therefore, before the getTraits/getStoreTraits function can be completely removed, this usage has to be replaced (under this ticket). This can be done in various ways:

  • deciding on a different solution to the partial implementation of #2008: either by fully implementing it or reverting it perhaps?
  • replacing the FederatedStore check with something else smarter than this DYNAMIC_SCHEMA trait check

A solution to this must be discussed, decided on and implemented before getTraits can be completely removed and it seems that it will be best to wait until the other FederatedStore changes first to do so.

t92549 avatar Jan 28 '22 17:01 t92549

The fix for this has to be smarter than just checking if the FederatedStore is assignable from the store in question, because it can also be valid for ProxyStores pointing to FederatedStores

t92549 avatar Jan 31 '22 11:01 t92549

Needs investigation to decide if this needs to stay in alpha5 or move to post V2. Might just result in code being deleted if it's not required

lb324567 avatar Oct 26 '22 14:10 lb324567

This will link in with #2581

GCHQDev404 avatar Jan 11 '23 13:01 GCHQDev404

Closed by #2915

The solution for this was to remove the getTraits method from Gaffer and use the operation instead, combined with removing the DYNAMIC_SCHEMA trait.

GCHQDeveloper314 avatar Mar 01 '23 15:03 GCHQDeveloper314