Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace getTrait usages with Operation #2581

Closed
t92549 opened this issue Jan 28, 2022 · 5 comments · Fixed by #2901
Closed

Replace getTrait usages with Operation #2581

t92549 opened this issue Jan 28, 2022 · 5 comments · Fixed by #2901
Assignees
@GCHQDeveloper314
Labels
enhancement Improvement to existing functionality/feature tech-debt Relates to Technical Debt
Milestone
v2.0.0-alpha-0.5

Comments

@t92549
Copy link
Contributor

t92549 commented Jan 28, 2022

#2552 removed deprecated code, however, the getTraits function is still used (although deprecated).
Similar to #2573, the function version of getTraits should be replaced by executing the GetTraits Operation.

This, however, cannot be done as part of alpha 1 and has to be done later. This is because it is reliant upon #2580 which needs to be done with the major FederatedStore changes.
@t92549 t92549 added the tech-debt Relates to Technical Debt label Jan 28, 2022
@t92549 t92549 added this to the v2.0.0-alpha-0.5 milestone Jan 28, 2022
@t92549 t92549 mentioned this issue Jan 28, 2022
Merged
@GCHQDev404 GCHQDev404 mentioned this issue Jan 11, 2023
Closed
@GCHQDev404
Copy link
Contributor

I think this is should happen after #2791 because the traits is often based on values in the schema.

@GCHQDeveloper314
Copy link
Member

The related getSchema ticket (now closed) is not blocking this I don't think. The only difficulty with this will be updating tests to mock the operation instead of the method.

GCHQDev404 added a commit that referenced this issue Feb 21, 2023
@GCHQDev404
gh-2581 getTraits to use operation, rebranched from gh-2881
d89bb76
GCHQDev404 added a commit that referenced this issue Feb 21, 2023
@GCHQDev404
gh-2581 getTraits to use operation, tidy up.
fff7467
@github-actions github-actions bot mentioned this issue Feb 21, 2023
Closed
GCHQDev404 added a commit that referenced this issue Feb 21, 2023
@GCHQDev404
gh-2581 getTraits to use operation, tidy up.
1f2e327
GCHQDev404 added a commit that referenced this issue Feb 21, 2023
@GCHQDev404
gh-2581 getTraits to use operation, default getGetTraitsHandler retur…
31e366a 
…ns empty set.
GCHQDev404 added a commit that referenced this issue Feb 21, 2023
@GCHQDev404
gh-2581 getTraits to use operation, default getGetTraitsHandler retur…
82d8e1c 
…ns empty set.
t92549 pushed a commit that referenced this issue Feb 22, 2023
@GCHQDev404
Gh 2890 cache service static instance bug (#2893)
b9fb46e 
* gh-2890 cache service single static instance bug removal.

* Revert "gh-2890 cache service single static instance bug removal."

This reverts commit 59b23ab.

* gh-2890 cache service single static instance bug removal. K.I.S.S

* gh-2890 cache service single static instance bug removal. K.I.S.S Tests

* gh-2890 cache service. delete backwards compatibility 1.12

* gh-2890 cache service single static instance bug removal. K.I.S.S Tests

* gh-2890 cache service single static instance bug removal. spotbugs

* gh-2890 cache service static instance bug. proof test.

* checkstyle

* gh-2581 getTraits to use operation, tidy up.

---------

Co-authored-by: GCHQDev404 <GCHQDev404@users.noreply.github.com>
@t92549 t92549 added the enhancement Improvement to existing functionality/feature label Feb 22, 2023
@github-actions github-actions bot mentioned this issue Feb 22, 2023
Merged
@GCHQDeveloper314 GCHQDeveloper314 linked a pull request Feb 22, 2023 that will close this issue
Gh-2581: Replace getTraits usages with operation #2901
Merged
@GCHQDeveloper314
Copy link
Member

There are some problems with the GetTrait operation returning different results to the method. See PR #2901 for details.

@GCHQDeveloper314
Copy link
Member

Problems with the PR have been resolved by using the operation option to not get current traits - ready for PR to be reviewed.

@GCHQDeveloper314 GCHQDeveloper314 mentioned this issue Mar 1, 2023
Open
GCHQDeveloper314 added a commit that referenced this issue Mar 1, 2023
@GCHQDeveloper314
Gh-2581: Replace getTraits usages with operation (#2901)
c53e8c3 
* Remove getTraits from Store

* Remove getTraits from Graph

* Remove getTraits from Map Store

* Remove getTraits from Accumulo Store

* Remove getTraits from Core Rest

* Remove getTraits from Proxy Store

* Fix to ensure user info doesn't get lost
Federated Store needs the user

* Remove getTraits from Federated Store

* Remove getTraits from Spring Rest

* Revert disabling IT for Federated Store
Fails due to QUERY_AGGREGATION Trait not being reported as a Trait of this store

* Change uses of GetTraits operation to get all supported traits
By default this operation gets the currently supported traits. It needs to get all supported instead as that is what the method used to do.

* Improve description of endpoint for Spring

* Simplify test in FederatedStoreUtilTest

* Fix for handling null User in OperationChainValidator

* Update Copyright

* Restore getTraits method for Map Store internal use
Using the static field directly would work, except this is overriden in some child classes which requires a getter to be used.

* Enable handler for GetTraits in proxy store
Previously this was unused as the handler was not added. Handler works the same as getTrait method did, removing visibility trait from returned traits. Also set other unused handler to null.

* Copyright corrections

* Fix test to include newly supported Operation in Proxy Store

* ProxyStoreBasicIT should get all supported traits and not current
This has no effect on the test currently but makes it clear what should be tested

* Revert superseded change identified in PR comment
GCHQDeveloper314 added a commit that referenced this issue Mar 1, 2023
@GCHQDeveloper314
Merge branch 'gh-2581-replace-gettrait-usages-with-operation' into dy…
df95443 
…namic-schema-test
GCHQDeveloper314 added a commit that referenced this issue Mar 1, 2023
@GCHQDeveloper314
Merge branch 'gh-2581-replace-gettrait-usages-with-operation' into gh…
9595bcb 
…-2580-fix-federatedstore-checks-for-DYNAMIC_SCHEMA
@GCHQDeveloper314
Copy link
Member

Closed by #2901

@GCHQDev404 GCHQDev404 mentioned this issue Mar 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@GCHQDeveloper314 GCHQDeveloper314

Labels
enhancement Improvement to existing functionality/feature tech-debt Relates to Technical Debt
Projects
None yet
Milestone
v2.0.0-alpha-0.5
3 participants
@GCHQDev404 @t92549 @GCHQDeveloper314