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.

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

[Core] Rewrite saved objects in typescript #36829

Merged
merged 37 commits into from
Jun 6, 2019

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented May 21, 2019

Summary

Migrates the Saved Objects Client / Service to typescript in preparation for moving it to Core. Not all tests have been migrated, but the PR is already quite large so I decided to stop here for now.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@epixa epixa mentioned this pull request May 23, 2019
20 tasks
@rudolf rudolf force-pushed the ts-saved-objects-lib branch 2 times, most recently from 10a6247 to 116f721 Compare May 23, 2019 20:04
@elastic elastic deleted a comment from elasticmachine May 24, 2019
@elastic elastic deleted a comment from elasticmachine May 24, 2019
@elastic elastic deleted a comment from elasticmachine May 24, 2019
@elastic elastic deleted a comment from elasticmachine May 24, 2019
@elastic elastic deleted a comment from elasticmachine May 24, 2019
@rudolf rudolf force-pushed the ts-saved-objects-lib branch 2 times, most recently from f2a3240 to cb6cfc2 Compare May 27, 2019 11:23
@mshustov mshustov mentioned this pull request Jun 4, 2019
3 tasks
@rudolf
Copy link
Contributor Author

rudolf commented Jun 4, 2019

@restrry @eliperelman @mattapperson I've addressed the comments I left on #35747 in the merge commit 89be456. Unfortunately this makes it hard to review so I recommend doing git show 89be456

@rudolf rudolf requested review from mattapperson and removed request for mikecote June 4, 2019 08:15
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elastic elastic deleted a comment from elasticmachine Jun 4, 2019
@elastic elastic deleted a comment from elasticmachine Jun 4, 2019
@elastic elastic deleted a comment from elasticmachine Jun 4, 2019
@rudolf rudolf added the non-issue Indicates to automation that a pull request should not appear in the release notes label Jun 4, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to the spaces and encrypted_saved_objects plugins look good. Super excited to see this all in TS!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rudolf rudolf merged commit ea9721a into elastic:master Jun 6, 2019
@rudolf rudolf deleted the ts-saved-objects-lib branch June 6, 2019 08:49
rudolf added a commit to rudolf/kibana that referenced this pull request Jun 7, 2019
* Convert simple files to TS

* Fix jest tests

* Rename saved_objects_client{.js => .ts}

* WIP saved_objects_client

* saved_objects repository{.js => .ts}

* includedFields support string[] for type paramater

* Repository/saved_objects_client -> TS

* Fix tests and dependencies

* Fix saved objects type errors and simplify

* saved_objects/index saved_objects/service/index -> ts

* Fix saved objects export test after switching to typed mock

* Workaround type error

* Revert "Workaround type error"

This reverts commit de3252267eb2e6bf56a5584d271b55a7afdc1c53.

* Correctly type Server.savedObjects.SaveObjectsClient constructor

* saved_objects/service/lib/index.{js -> ts}

* saved_objects/service/lib/scoped_client_provider{js -> ts}

* Typescriptify scoped_client_provider

* Fix x-pack jest imports

* Add lodash/internal/toPath typings to xpath

* Introduce SavedObjectsClientContract

We need a way to specify that injected dependencies should adhere to the
SavedObjectsClient "contract". We can't use the SavedObjectsClient class
itself since it contains the private _repository property which in TS is
included in the type signature of a class.

* Cleanup and simplify types

* Fix repository#delete should return {}

* Add SavedObjects repository test for uncovered bug

Test for a bug in our previous js implementation that can lead to data
corruption and data loss.

If a bulkGet request is made where one of the objects to fetch is of a type
that isn't allowed, the returned result will include documents which have the
incorrect id and type assigned. E.g. the data of an object with id '1' is
returned with id '2'. Saving '2' will incorrectly override it's data with that
of the data of object '1'.

* SavedObject.updated_at: string and unify saved_object / serializer types

* Cleanup

* Address code review feedback

* Don't mock errors helpers in SavedObjectsClient Mock

* Address CR feedback

* CR Feedback #2

* Add kibana-platform as code owners of Saved Objects

* Better typings for SavedObjectsClient.errors

* Use unknown as default for generic type request paramater

* Bump @types/elasticsearch

* Fix types for isForbiddenError

* Bump x-pack @types/elasticsearch
rudolf added a commit that referenced this pull request Jun 7, 2019
* [Core] Rewrite saved objects in typescript (#36829)

* Convert simple files to TS

* Fix jest tests

* Rename saved_objects_client{.js => .ts}

* WIP saved_objects_client

* saved_objects repository{.js => .ts}

* includedFields support string[] for type paramater

* Repository/saved_objects_client -> TS

* Fix tests and dependencies

* Fix saved objects type errors and simplify

* saved_objects/index saved_objects/service/index -> ts

* Fix saved objects export test after switching to typed mock

* Workaround type error

* Revert "Workaround type error"

This reverts commit de3252267eb2e6bf56a5584d271b55a7afdc1c53.

* Correctly type Server.savedObjects.SaveObjectsClient constructor

* saved_objects/service/lib/index.{js -> ts}

* saved_objects/service/lib/scoped_client_provider{js -> ts}

* Typescriptify scoped_client_provider

* Fix x-pack jest imports

* Add lodash/internal/toPath typings to xpath

* Introduce SavedObjectsClientContract

We need a way to specify that injected dependencies should adhere to the
SavedObjectsClient "contract". We can't use the SavedObjectsClient class
itself since it contains the private _repository property which in TS is
included in the type signature of a class.

* Cleanup and simplify types

* Fix repository#delete should return {}

* Add SavedObjects repository test for uncovered bug

Test for a bug in our previous js implementation that can lead to data
corruption and data loss.

If a bulkGet request is made where one of the objects to fetch is of a type
that isn't allowed, the returned result will include documents which have the
incorrect id and type assigned. E.g. the data of an object with id '1' is
returned with id '2'. Saving '2' will incorrectly override it's data with that
of the data of object '1'.

* SavedObject.updated_at: string and unify saved_object / serializer types

* Cleanup

* Address code review feedback

* Don't mock errors helpers in SavedObjectsClient Mock

* Address CR feedback

* CR Feedback #2

* Add kibana-platform as code owners of Saved Objects

* Better typings for SavedObjectsClient.errors

* Use unknown as default for generic type request paramater

* Bump @types/elasticsearch

* Fix types for isForbiddenError

* Bump x-pack @types/elasticsearch

* Update yarn.lock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform Feature:Saved Objects non-issue Indicates to automation that a pull request should not appear in the release notes review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.3.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants