Gh 2553 add Iterable classes from koryphe and remove closeable Iterable, iterator and wrapper classes #2583

r32575 · 2022-02-08T12:24:56Z

Closeable iterator and iterable have been removed from Koryphe for v2. This issue removes closeable iterator, iterable and wrapper from Gaffer. Other changes include:

updating the testing framework to AssertJ
fixing bugs discovered

…and added a test for new constructor

… iterables/iterators to use java.io.Closeable and java.lang.Iterable instead. Also amended the tests

… and removed the WrappedCloseableIterable. Tests function

…eIterable and the CloseableIterable types. Tests function

…eIterable and the CloseableIterable types. The GraphSerialisableTest test does not function

…eIterable and the CloseableIterable types. Tests function

…ds which was causing GraphSerialisableTest to fail. Reset TestStore.mockStore for both. All Graph test now pass

* gh-2571: Fixed matchedVertex behaviour and added new ITs * gh-2571: Fixed checkstyle * gh-2571: Updated test name

…, instead implemented Iterable. Fixed tests

…bug caused by the introduction of the Koryphe version of ChainedIterable. Fixed tests

…to Iterable

…terable. Test pass

… test class casts. Test pass

…terable. FederatedStoreGetTraitsTest fails

…arrying 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

…ry blocks with a close-if-auto-closeable check

…er builds successfully

* 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

* 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>

...va/uk/gov/gchq/gaffer/accumulostore/operation/handler/GetElementsBetweenSetsHandlerTest.java

GCHQDeveloper314

The diff for this PR contains changes to lots of files which have already been merged (removal of hbase and parquet) I think this is because git still sees this as a change for some reason. Rebasing the branch will fix that.

core/common-util/src/main/java/uk/gov/gchq/gaffer/commonutil/iterable/ChainedIterable.java

...ommon-util/src/main/java/uk/gov/gchq/gaffer/commonutil/iterable/ConsumableBlockingQueue.java

codecov-commenter · 2022-04-07T12:26:21Z

Codecov Report

❗ No coverage uploaded for pull request base (v2-alpha@b3f6e97). Click here to learn what that means.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             v2-alpha    #2583   +/-   ##
===========================================
  Coverage            ?   65.62%           
  Complexity          ?     2416           
===========================================
  Files               ?      862           
  Lines               ?    26996           
  Branches            ?     3132           
===========================================
  Hits                ?    17717           
  Misses              ?     7912           
  Partials            ?     1367

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 b3f6e97...b20767c. Read the comment docs.

t92549

Just a bunch of questions. My main comment is that, although I like most of the reformatting you have done (but not all of it), I think it makes the PR way harder to actually review when most of it is formatting, it almost hides the actual changes.

core/common-util/src/main/java/uk/gov/gchq/gaffer/commonutil/iterable/ChainedIterable.java

core/common-util/src/main/java/uk/gov/gchq/gaffer/commonutil/iterable/EmptyIterable.java

core/common-util/src/test/java/uk/gov/gchq/gaffer/commonutil/iterable/LimitedIterableTest.java

core/common-util/src/test/java/uk/gov/gchq/gaffer/commonutil/iterable/StreamIterableTest.java

core/operation/src/test/java/uk/gov/gchq/gaffer/operation/OperationChainTest.java

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

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

rest-api/core-rest/src/main/java/uk/gov/gchq/gaffer/rest/service/v1/OperationService.java

.../src/main/java/uk/gov/gchq/gaffer/accumulostore/operation/handler/GetAllElementsHandler.java

GCHQDev404

class deleted.

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

…ialiser

GCHQDev404

class deleted.

core/common-util/src/main/java/uk/gov/gchq/gaffer/commonutil/iterable/ChainedIterable.java

core/serialisation/src/main/java/uk/gov/gchq/gaffer/jsonserialisation/JSONSerialiser.java

r32575 and others added 30 commits January 6, 2022 16:15

Transferred ChainedIterable from Koryphe, added existing constructor …

73896d8

…and added a test for new constructor

Removed the Closeable and WrappedCloseable classes and refactored the…

30799eb

… iterables/iterators to use java.io.Closeable and java.lang.Iterable instead. Also amended the tests

Renamed the CloseableIterableDeserializer to the IterableDeserializer…

b17b141

… and removed the WrappedCloseableIterable. Tests function

Renamed CloseableIterable to Iterable and removed the WrappedCloseabl…

c655bc3

…eIterable and the CloseableIterable types. Tests function

gh-2552: Removed deprecated code except getTraits and hasTrait

c7fd1e3

gh-2552: Fix javadoc issue

ab0b66c

Renamed CloseableIterable to Iterable and removed the WrappedCloseabl…

ded5a98

…eIterable and the CloseableIterable types. The GraphSerialisableTest test does not function

Renamed CloseableIterable to Iterable and removed the WrappedCloseabl…

5b31a81

…eIterable and the CloseableIterable types. Tests function

Changed to try-with-resources as both io classes are auto-closeable

fc2272d

The TestStore.mockStore was being set in GraphTest but reset afterwar…

7856df1

…ds which was causing GraphSerialisableTest to fail. Reset TestStore.mockStore for both. All Graph test now pass

Renamed CloseableIterable to Iterable

74fa3fa

Merge branch 'v2-alpha' into gh-2552-remove-deprecated

4ee7645

Merge branch 'v2-alpha' into gh-2552-remove-deprecated

4c257f1

gh-2571: Fixed matchedVertex behaviour and added new ITs (#2572)

d081ca6

* 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…

5996f34

…, instead implemented Iterable. Fixed tests

Converted CloseableIterable to Iterable. Fixed AbstractElementFilter …

4bdf412

…bug caused by the introduction of the Koryphe version of ChainedIterable. Fixed tests

Changed EmptyClosableIterable to EmptyIterable

ab65521

Changed the OperationDetailTest name-of-class from CloseableIterable …

05a95a6

…to Iterable

Changed the the instances and JSON values from CloseableIterable to I…

e7dfe78

…terable. Test pass

Changed the the instances of CloseableIterable to Iterable. Suppresed…

b2cd01b

… test class casts. Test pass

Changed from CloseableIterable to Iterable. Deleted WrappedCloseableI…

c1bfb30

…terable. FederatedStoreGetTraitsTest fails

Changed from CloseableIterable to Iterable and added finally to the t…

77f5bac

…ry blocks with a close-if-auto-closeable check

Merged the gh-2552-remove-deprecated branch and fixed conflicts. Gaff…

75547c0

…er builds successfully

gh-2552: Removed some remaining parquet properties files

cf1d736

Added missed iterable try-with-resources block, reformated and tested

3651611

Reformated code. Updated test code. All tests pass

5e61442

Reformated code. Updated test code. All tests pass

c1ff4d8

r32575 removed this from the v2.0.0-alpha-0.2 milestone Feb 8, 2022

r32575 mentioned this pull request Mar 10, 2022

Gh 2562 GetElementsWithinSet not allowing for the exclusion of properties - Gaffer v1 #2597

Merged

r32575 commented Mar 10, 2022

View reviewed changes

...va/uk/gov/gchq/gaffer/accumulostore/operation/handler/GetElementsBetweenSetsHandlerTest.java Show resolved Hide resolved

t92549 mentioned this pull request Mar 24, 2022

FederatedStore traits tests are inconsistent #2616

Closed

r32575 changed the title ~~Gh 2553 add closeable classes from koryphe and remove closeable Iterable, iterator and wrapper~~ Gh 2553 add Iterable classes from koryphe and remove closeable Iterable, iterator and wrapper classes Apr 5, 2022

GCHQDeveloper314 reviewed Apr 6, 2022

View reviewed changes

core/common-util/src/main/java/uk/gov/gchq/gaffer/commonutil/iterable/ChainedIterable.java Outdated Show resolved Hide resolved

...ommon-util/src/main/java/uk/gov/gchq/gaffer/commonutil/iterable/ConsumableBlockingQueue.java Outdated Show resolved Hide resolved

r32575 added 2 commits April 6, 2022 12:26

Merged v2-alpha branch and fixed conflicts

4801a77

Merged v2-alpha branch once more and fixed conflicts

fbb9017

r32575 marked this pull request as ready for review April 6, 2022 11:56

Created Objects.isNull static import

ba2b243

r32575 added 3 commits April 7, 2022 16:33

Created Objects.isNull and Objects.nonNull static imports

88c1650

Update the copyright to the correct dates

49b75b3

Merged v2-alpha branch again and fixed conflicts

ef18a49

t92549 reviewed Apr 8, 2022

View reviewed changes

r32575 added 2 commits April 11, 2022 12:25

Removed unnecessary cast

0e9cba5

Corrected an/a grammar error

dd5ebba

GCHQDev404 suggested changes Apr 12, 2022

View reviewed changes

...d-store/src/test/java/uk/gov/gchq/gaffer/federatedstore/FederatedGraphStorageTraitsTest.java Outdated Show resolved Hide resolved

r32575 added 5 commits April 12, 2022 17:49

Removed the incorrect spacing in the imports

1466ea4

Created Objects.isNull and Objects.nonNull static imports for JSONSer…

49c2d99

…ialiser

Removed deleted FederatedGraphStorageTraitsTest

f0f20ba

Re-added incorrectly deleted empty line

2a466b8

Removed the unnecessary try-finally blocks from the LimitedIterableTest

98cf23f

GCHQDev404 reviewed Apr 12, 2022

View reviewed changes

core/common-util/src/main/java/uk/gov/gchq/gaffer/commonutil/iterable/ChainedIterable.java Show resolved Hide resolved

core/serialisation/src/main/java/uk/gov/gchq/gaffer/jsonserialisation/JSONSerialiser.java Outdated Show resolved Hide resolved

r32575 added 3 commits April 13, 2022 12:08

Removed the unnecessary try-finally blocks

5283ccf

Removed the unused imports

b20767c

Removed the unnecessary try-finally blocks

9791186

t92549 merged commit 9ab95bc into v2-alpha Apr 14, 2022

t92549 deleted the gh-2553-add-closeable-classes-from-koryphe branch April 14, 2022 14:52

t92549 mentioned this pull request Apr 14, 2022

Add Closeable classes from Koryphe #2553

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gh 2553 add Iterable classes from koryphe and remove closeable Iterable, iterator and wrapper classes #2583

Gh 2553 add Iterable classes from koryphe and remove closeable Iterable, iterator and wrapper classes #2583

r32575 commented Feb 8, 2022

GCHQDeveloper314 left a comment

codecov-commenter commented Apr 7, 2022 •

edited

Loading

t92549 left a comment

GCHQDev404 left a comment

GCHQDev404 left a comment

Gh 2553 add Iterable classes from koryphe and remove closeable Iterable, iterator and wrapper classes #2583

Gh 2553 add Iterable classes from koryphe and remove closeable Iterable, iterator and wrapper classes #2583

Conversation

r32575 commented Feb 8, 2022

GCHQDeveloper314 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 7, 2022 • edited Loading

Codecov Report

t92549 left a comment

Choose a reason for hiding this comment

GCHQDev404 left a comment

Choose a reason for hiding this comment

GCHQDev404 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 7, 2022 •

edited

Loading