Gh-3059: ApplyViewToElementsFunction apply Schema validation #3060

GCHQDev404 · 2023-11-03T13:18:47Z

Related issue

Resolve ApplyViewToElementsFunction does not apply Schema validation #3059

…tion into view.

…ation into view POST Aggregation to work after a groupBy aggregation change.

…wtoelements-schema-validation' into gh-3059-federated-store-applyviewtoelements-schema-validation # Conflicts: # store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/util/ApplyViewToElementsFunction.java

…ation should only be done for the TemporaryStore of type MapStore

codecov · 2023-11-03T16:52:32Z

Codecov Report

Attention: Patch coverage is 73.33333% with 28 lines in your changes are missing coverage. Please review.

Project coverage is 66.77%. Comparing base (6213159) to head (384c308).

Files	Patch %	Lines
...ffer/federatedstore/util/MergeElementFunction.java	69.76%	18 Missing and 8 partials ⚠️
.../gchq/gaffer/data/elementdefinition/view/View.java	66.66%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop    #3060      +/-   ##
=============================================
+ Coverage      66.71%   66.77%   +0.05%     
  Complexity      2557     2557              
=============================================
  Files            907      907              
  Lines          29063    29111      +48     
  Branches        3239     3249      +10     
=============================================
+ Hits           19390    19439      +49     
+ Misses          8239     8232       -7     
- Partials        1434     1440       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

...store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreViewAggregationTest.java

t92549 · 2023-11-06T09:54:34Z

...-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/util/ApplyViewToElementsFunction.java

            this.context = Collections.unmodifiableMap(context);
        } catch (final Exception e) {
            throw new GafferCheckedException("Unable to create TemporaryResultsGraph", e);
        }

    }

+    private static void updateViewWithValidationFromSchema(final Map<String, Object> context) {


Should the function ApplyViewToElementsFunction be renamed?

To what?

It still applies view to elements. This additional new Validation suff is because of MapStore failings, this would be done for free otherwise. It should get ripped out when Mapstore is fixed.

It doesn't just apply the view anymore, it also reapplies the schema now. On this topic, does your fix apply schema aggregation too? Or just schema validation?

"does your fix apply schema aggregation too?" - I think this is not necessary as the MapStore does this.

yep correct, mapstore does aggregation. This is all due to Mapstore having issues that need validation added.

Should ApplyViewToElementsFunction be renamed then? As the View and Schema are reapplied.

With regards to the schema validation being stolen for the filter. I don't think so because this temporary.

With regards too the schema doing schema things in within a working complete store... erm...
would you choose ReApplyViewFilterAndSchemaValidationFunction getting bit a wordy.

what about FederatedElementFunction ?

Perhaps this could actually be a different function called ApplyViewAndSchemaToElementsFunction and be an alternative?

I think we landed on "MergeElementFunction"

...-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/util/ApplyViewToElementsFunction.java

tb06904 · 2023-11-06T10:31:47Z

...-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/util/ApplyViewToElementsFunction.java

+    private static void updateEntitiesViewFilterWithSchemaValidation(final Schema schema, final View view, final View.Builder updatedView) {
+        final Map<String, SchemaEntityDefinition> elements = schema.getEntities();
+        for (final Map.Entry<String, ? extends SchemaElementDefinition> schemaElement : elements.entrySet()) {
+
+            final String elementKey = schemaElement.getKey();
+            final ViewElementDefinition updatedViewElementDef = getUpdatedViewElementDef(elementKey, schemaElement.getValue(), view);
+
+            updatedView.entity(elementKey, updatedViewElementDef);
+        }
+    }
+
+    private static void updateEdgesViewFilterWithSchemaValidation(final Schema schema, final View view, final View.Builder updatedView) {
+        final Map<String, SchemaEdgeDefinition> elements = schema.getEdges();
+        for (final Map.Entry<String, ? extends SchemaElementDefinition> schemaElement : elements.entrySet()) {
+
+            final String elementKey = schemaElement.getKey();
+            final ViewElementDefinition updatedViewElementDef = getUpdatedViewElementDef(elementKey, schemaElement.getValue(), view);
+
+            updatedView.edge(elementKey, updatedViewElementDef);
+        }
+    }


As a thought this might be a good place to use inline lambdas instead you could replace both these methods with just:

schema.getEntities().entrySet().forEach(entity -> { if (entity.getValue().hasValidation()) { updatedView.entity(entity.getKey(), getUpdatedViewElementDef(entity.getValue(), view, entity.getKey())); } }); schema.getEdges().entrySet().forEach(edge -> { if (edge.getValue().hasValidation()) { updatedView.edge(edge.getKey(), getUpdatedViewElementDef(edge.getValue(), view, edge.getKey())); } });

…ts-schema-validation