gh-2573: Replace hasTrait usages with Operation #2574

t92549 · 2022-01-18T22:17:23Z

Related Issue

Resolve Replace hasTrait usages with HasTrait Operation #2573

codecov-commenter · 2022-01-18T22:40:42Z

Codecov Report

Merging #2574 (e05e5de) into gh-2552-remove-deprecated (d081ca6) will decrease coverage by 0.00%.
The diff coverage is 65.00%.

@@                       Coverage Diff                       @@
##             gh-2552-remove-deprecated    #2574      +/-   ##
===============================================================
- Coverage                        65.30%   65.30%   -0.01%     
+ Complexity                        2431     2429       -2     
===============================================================
  Files                              872      872              
  Lines                            27186    27197      +11     
  Branches                          3165     3166       +1     
===============================================================
+ Hits                             17755    17760       +5     
- Misses                            8034     8039       +5     
- Partials                          1397     1398       +1

Impacted Files	Coverage Δ
.../src/main/java/uk/gov/gchq/gaffer/graph/Graph.java	`68.64% <ø> (-0.09%)`	⬇️
...affer/store/operation/handler/HasTraitHandler.java	`62.50% <0.00%> (-12.50%)`	⬇️
...esultcache/handler/util/GafferResultCacheUtil.java	`83.87% <62.50%> (-3.63%)`	⬇️
.../src/main/java/uk/gov/gchq/gaffer/store/Store.java	`70.30% <66.66%> (-0.14%)`	⬇️
...re/operation/FederatedOperationChainValidator.java	`94.59% <100.00%> (ø)`
...gaffer/federatedstore/util/FederatedStoreUtil.java	`94.36% <100.00%> (ø)`
.../gchq/gaffer/jsonserialisation/JSONSerialiser.java	`73.83% <0.00%> (-1.87%)`	⬇️
...ffer/serialisation/util/JsonSerialisationUtil.java	`61.34% <0.00%> (-0.85%)`	⬇️
...k/gov/gchq/gaffer/accumulostore/AccumuloStore.java	`75.90% <0.00%> (+0.60%)`	⬆️
.../gov/gchq/gaffer/store/schema/SchemaOptimiser.java	`86.02% <0.00%> (+1.07%)`	⬆️

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 d081ca6...e05e5de. Read the comment docs.

GCHQDev404 · 2022-01-20T18:49:39Z

You considered creating a static util method somewhere ?

public static boolean hasTrait(Trait trait,String prependError){
return hasTrait(Trait trait, false,String prependError){
}

public static boolean hasTrait(Trait trait, boolean current,String prependError){
boolean rtn;
                 try {
                     rtn = graph.execute(new HasTrait.Builder()
                             .trait(trait)
                             .currentTraits(current)
                             .build(), new Context());
                 } catch (final OperationException e) {
                     throw new GafferRuntimeException(String.format("%s%s","Error performing HasTrait Operation ",isNull(prependError)? "": prependError), e);
                 }               
return rtn

t92549 · 2022-01-21T12:08:05Z

@GCHQDev404 could do but maybe seems silly to replace a function with an Operation called by a function, only used 4 times anyway.

core/store/src/main/java/uk/gov/gchq/gaffer/store/Store.java

core/store/src/main/java/uk/gov/gchq/gaffer/store/operation/handler/HasTraitHandler.java

core/store/src/test/java/uk/gov/gchq/gaffer/store/StoreTest.java

.../main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java

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

...d-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorageTraitsTest.java

...rated-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedStoreGetTraitsTest.java

This reverts commit c367555.

GCHQDev404

Possible Issue with store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java

GCHQDev404

No issue with store-implementation/federated-store/src/main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java

t92549 · 2022-01-28T18:04:01Z

.../main/java/uk/gov/gchq/gaffer/federatedstore/operation/FederatedOperationChainValidator.java

@@ -78,7 +78,8 @@ private void validateAllGraphsIdViews(final Operation op, final User user, final
            if (graphIdValid) {
                currentResult = new ValidationResult();
                clonedOp.addOption(FederatedStoreConstants.KEY_OPERATION_OPTIONS_GRAPH_IDS, graphId);
-                if (!graph.hasTrait(StoreTrait.DYNAMIC_SCHEMA)) {
+                // Deprecated function still in use due to Federated GetTraits bug with DYNAMIC_SCHEMA
+                if (!graph.getStoreTraits().contains(StoreTrait.DYNAMIC_SCHEMA)) {


Note: This should be fixed by #2580

t92549 · 2022-01-28T18:04:17Z

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

@@ -130,7 +130,8 @@ public static String createOperationErrorMsg(final Operation operation, final St
                    // then clone the operation and add the new view.
                    if (validView.hasGroups()) {
                        ((OperationView) resultOp).setView(validView);
-                    } else if (!graph.hasTrait(StoreTrait.DYNAMIC_SCHEMA)) {
+                    // Deprecated function still in use due to Federated GetTraits bug with DYNAMIC_SCHEMA
+                    } else if (!graph.getStoreTraits().contains(StoreTrait.DYNAMIC_SCHEMA)) {


Note: This should be fixed by #2580

* gh-2552: Removed deprecated code except getTraits and hasTrait * gh-2552: Fix javadoc issue * gh-2571: Fixed matchedVertex behaviour and added new ITs (#2572) * gh-2571: Fixed matchedVertex behaviour and added new ITs * gh-2571: Fixed checkstyle * gh-2571: Updated test name * gh-2573: Replace hasTrait usages with Operation (#2574) * gh-2573: Replace hasTrait usages with Operation * gh-2573: Fix tests by making exception more explicit * gh-2573: Fixed exception issues * gh-2573: Simplified but still failing test * gh-2573: Changed to user * gh-2573: Changed FederatedGetTraitsHandler to return DYNAMIC_SCHEMA * gh-2573: Use old function for FederatedOperationChainValidator * gh-2573: Fixed FederatedStoreUtil * gh-2573: Added Deprecation notice * gh-2573: Cherry pick federated store tests fix from 2553 * Revert "gh-2573: Cherry pick federated store tests fix from 2553" This reverts commit c367555. * gh-2573: Manual fix of FederatedStoreGetTraitsTest cache * gh-2573: Changed HasTraitHandler to not send Federated loop option * Gh-2560 get traits operation configurable (#2579) * Update docs links in README * Revert "Update docs links in README" This reverts commit 2d87bf3. * gh-2559 added hasTrait, its handler and corresponding tests (#2561) * gh-2559 added hasTrait, its handler and corresponding tests * gh-2559 amended comment in HasTrait, added additional tests for the handler * gh-2559 null check & tidy test Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com> * Update links to Gaffer docs in all files (#2566) Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com> * gh-2560 GetTraitsHandler is initalised with traits dependednt on store type * gh-2560 reviewed comments * gh-2560 unused imports * gh-2560 review comments * checkstyle * gh-2560 fixed federated test * gh-2560 fixed GraphTest * gh-2560 fixed TestStore * gh-2560-getTraitsOperation (#2582) Removing redundant adding of GetTraitHandler from addAdditionalOperationHandlers() because it has its own method getGetTraitsHandler(). * gh-2560: Added fix to SingleUseMapStoreWithoutVisibilitySupport * gh-2560: Fixed Set copy issue in GetTraitsHandler constructor * gh-2560: Added extra protections to GetTraitsHandler Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com> Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com> Co-authored-by: GCHQDev404 <45399082+GCHQDev404@users.noreply.github.com> * gh-2552: Removed some remaining parquet properties files * gh-2552: Review comments traits test * gh-2552-remove-deprecated MultiSerialiser (#2587) * gh-2552-remove-deprecated MultiSerialiser update the MultiSerialiser wanting to emphasise the effectiveness of it during the deprecation of serialisers. 8 is the last and best. 24 is not latest regardless of natural-order. 7 is dead. * gh-2552: MultiSerialiser spelling corrections * Update core/serialisation/src/main/java/uk/gov/gchq/gaffer/serialisation/implementation/MultiSerialiser.java Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com> Co-authored-by: p3430233 <61738524+p3430233@users.noreply.github.com> Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com> Co-authored-by: GCHQDev404 <45399082+GCHQDev404@users.noreply.github.com>

…le, iterator and wrapper classes (#2583) * Transferred ChainedIterable from Koryphe, added existing constructor and added a test for new constructor * Removed the Closeable and WrappedCloseable classes and refactored the iterables/iterators to use java.io.Closeable and java.lang.Iterable instead. Also amended the tests * Renamed the CloseableIterableDeserializer to the IterableDeserializer and removed the WrappedCloseableIterable. Tests function * Renamed CloseableIterable to Iterable and removed the WrappedCloseableIterable and the CloseableIterable types. Tests function * gh-2552: Removed deprecated code except getTraits and hasTrait * gh-2552: Fix javadoc issue * Renamed CloseableIterable to Iterable and removed the WrappedCloseableIterable and the CloseableIterable types. The GraphSerialisableTest test does not function * Renamed CloseableIterable to Iterable and removed the WrappedCloseableIterable and the CloseableIterable types. Tests function * Changed to try-with-resources as both io classes are auto-closeable * The TestStore.mockStore was being set in GraphTest but reset afterwards which was causing GraphSerialisableTest to fail. Reset TestStore.mockStore for both. All Graph test now pass * Renamed CloseableIterable to Iterable * gh-2571: Fixed matchedVertex behaviour and added new ITs (#2572) * gh-2571: Fixed matchedVertex behaviour and added new ITs * gh-2571: Fixed checkstyle * gh-2571: Updated test name * Changed CloseableIterable to Iterable and removed the extends Wrapper, instead implemented Iterable. Fixed tests * Converted CloseableIterable to Iterable. Fixed AbstractElementFilter bug caused by the introduction of the Koryphe version of ChainedIterable. Fixed tests * Changed EmptyClosableIterable to EmptyIterable * Changed the OperationDetailTest name-of-class from CloseableIterable to Iterable * Changed the the instances and JSON values from CloseableIterable to Iterable. Test pass * Changed the the instances of CloseableIterable to Iterable. Suppresed test class casts. Test pass * Changed from CloseableIterable to Iterable. Deleted WrappedCloseableIterable. FederatedStoreGetTraitsTest fails * FederatedStoreGetTraitsTest did not delete the cache service before carrying out each test causing it to fail when run as part of the maven tests. In isolation it passed. Found that the FederatedGraphStorageTraitsTest was also not clearing the cache. Added the before and after cache clearance and bumped the JUnit version of FederatedStoreGetTraitsTest from 4 to 5. Tests now pass * Changed from CloseableIterable to Iterable and added finally to the try blocks with a close-if-auto-closeable check * gh-2573: Replace hasTrait usages with Operation (#2574) * gh-2573: Replace hasTrait usages with Operation * gh-2573: Fix tests by making exception more explicit * gh-2573: Fixed exception issues * gh-2573: Simplified but still failing test * gh-2573: Changed to user * gh-2573: Changed FederatedGetTraitsHandler to return DYNAMIC_SCHEMA * gh-2573: Use old function for FederatedOperationChainValidator * gh-2573: Fixed FederatedStoreUtil * gh-2573: Added Deprecation notice * gh-2573: Cherry pick federated store tests fix from 2553 * Revert "gh-2573: Cherry pick federated store tests fix from 2553" This reverts commit c367555. * gh-2573: Manual fix of FederatedStoreGetTraitsTest cache * gh-2573: Changed HasTraitHandler to not send Federated loop option * Gh-2560 get traits operation configurable (#2579) * Update docs links in README * Revert "Update docs links in README" This reverts commit 2d87bf3. * gh-2559 added hasTrait, its handler and corresponding tests (#2561) * gh-2559 added hasTrait, its handler and corresponding tests * gh-2559 amended comment in HasTrait, added additional tests for the handler * gh-2559 null check & tidy test Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com> * Update links to Gaffer docs in all files (#2566) Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com> * gh-2560 GetTraitsHandler is initalised with traits dependednt on store type * gh-2560 reviewed comments * gh-2560 unused imports * gh-2560 review comments * checkstyle * gh-2560 fixed federated test * gh-2560 fixed GraphTest * gh-2560 fixed TestStore * gh-2560-getTraitsOperation (#2582) Removing redundant adding of GetTraitHandler from addAdditionalOperationHandlers() because it has its own method getGetTraitsHandler(). * gh-2560: Added fix to SingleUseMapStoreWithoutVisibilitySupport * gh-2560: Fixed Set copy issue in GetTraitsHandler constructor * gh-2560: Added extra protections to GetTraitsHandler Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com> Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com> Co-authored-by: GCHQDev404 <45399082+GCHQDev404@users.noreply.github.com> * gh-2552: Removed some remaining parquet properties files * Added missed iterable try-with-resources block, reformated and tested * Reformated code. Updated test code. All tests pass * Reformated code. Updated test code. All tests pass * Reformatted code. Updated test code. All tests pass * Reformatted code. All tests pass * Fixed NamedViewCacheTest bugs and reformatted code. All tests pass * Reformatted code. All tests pass * Reformatted code. All tests pass * Reformatted code. All tests pass * Reformatted code again. All tests pass * Reformatted code. Changed the Operation ArrayUtils import to lang3 as accumulo did not have the other apache version. All tests pass * Reformatted code again. All tests pass * Reformatted code again. All tests pass * Reformatted code. All tests pass * Created Objects.isNull static import * Created Objects.isNull and Objects.nonNull static imports * Update the copyright to the correct dates * Removed unnecessary cast * Corrected an/a grammar error * Removed the incorrect spacing in the imports * Created Objects.isNull and Objects.nonNull static imports for JSONSerialiser * Removed deleted FederatedGraphStorageTraitsTest * Re-added incorrectly deleted empty line * Removed the unnecessary try-finally blocks from the LimitedIterableTest * Removed the unnecessary try-finally blocks * Removed the unused imports * Removed the unnecessary try-finally blocks Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com> Co-authored-by: p3430233 <61738524+p3430233@users.noreply.github.com> Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com> Co-authored-by: GCHQDev404 <45399082+GCHQDev404@users.noreply.github.com>

gh-2573: Replace hasTrait usages with Operation

71388d5

t92549 requested a review from GCHQDev404 January 18, 2022 22:17

t92549 added 2 commits January 18, 2022 22:37

gh-2573: Fix tests by making exception more explicit

5b839d4

gh-2573: Fixed exception issues

cf33701

t92549 linked an issue Jan 18, 2022 that may be closed by this pull request

Replace hasTrait usages with HasTrait Operation #2573

Closed

gh-2573: Simplified but still failing test

8796c34

t92549 and others added 6 commits January 21, 2022 17:15

gh-2573: Changed to user

e634a21

gh-2573: Changed FederatedGetTraitsHandler to return DYNAMIC_SCHEMA

a906660

gh-2573: Use old function for FederatedOperationChainValidator

2a85436

gh-2573: Fixed FederatedStoreUtil

a36a1bd

gh-2573: Added Deprecation notice

0f75cdf

gh-2573: Cherry pick federated store tests fix from 2553

c367555

GCHQDev404 suggested changes Jan 25, 2022

View reviewed changes

t92549 added 2 commits January 25, 2022 11:59

Revert "gh-2573: Cherry pick federated store tests fix from 2553"

2ab7d05

This reverts commit c367555.

gh-2573: Manual fix of FederatedStoreGetTraitsTest cache

e05e5de

GCHQDev404 mentioned this pull request Jan 26, 2022

HasTraitsHandler remove reference to FederatedStore Flag #2578

Open

gh-2573: Changed HasTraitHandler to not send Federated loop option

40bb337

GCHQDev404 approved these changes Jan 28, 2022

View reviewed changes

GCHQDev404 suggested changes Jan 28, 2022

View reviewed changes

GCHQDev404 approved these changes Jan 28, 2022

View reviewed changes

t92549 commented Jan 28, 2022

View reviewed changes

t92549 merged commit f83376d into gh-2552-remove-deprecated Jan 28, 2022

t92549 mentioned this pull request Jan 28, 2022

Replace hasTrait usages with HasTrait Operation #2573

Closed

n3101 deleted the gh-2573-use-HasTrait-operation branch March 7, 2023 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-2573: Replace hasTrait usages with Operation #2574

gh-2573: Replace hasTrait usages with Operation #2574

t92549 commented Jan 18, 2022 •

edited by github-actions bot

Loading

codecov-commenter commented Jan 18, 2022 •

edited

Loading

GCHQDev404 commented Jan 20, 2022 •

edited

Loading

t92549 commented Jan 21, 2022

GCHQDev404 left a comment

GCHQDev404 left a comment

t92549 Jan 28, 2022

t92549 Jan 28, 2022

gh-2573: Replace hasTrait usages with Operation #2574

gh-2573: Replace hasTrait usages with Operation #2574

Conversation

t92549 commented Jan 18, 2022 • edited by github-actions bot Loading

Related Issue

codecov-commenter commented Jan 18, 2022 • edited Loading

Codecov Report

GCHQDev404 commented Jan 20, 2022 • edited Loading

t92549 commented Jan 21, 2022

GCHQDev404 left a comment

Choose a reason for hiding this comment

GCHQDev404 left a comment

Choose a reason for hiding this comment

t92549 Jan 28, 2022

Choose a reason for hiding this comment

t92549 Jan 28, 2022

Choose a reason for hiding this comment

t92549 commented Jan 18, 2022 •

edited by github-actions bot

Loading

codecov-commenter commented Jan 18, 2022 •

edited

Loading

GCHQDev404 commented Jan 20, 2022 •

edited

Loading