Fix saved objects client upsert functionality

[jportner]

Resolves #114918.

Reduce from 2 preflight checks to 1 preflight check.
Also combined two internal functions into a single one.

As far as I can tell, no consumers of multi-namespace objects are actually utilizing upsert yet (the only multi-namespace object at the moment is ML jobs), so I'm just backporting this to 7.16 and leaving it without a release note.

[jportner]

Author's notes for reviewers

[jportner]

practically speaking this will never be undefined

What I mean by this is:
If the object's namespaces field was undefined, then it would not have been found in the first place. But the only way this would be undefined is if the object was found and its source did not include a namespaces field.

At any rate, I thought it would be better not to try to change existing types even though that is the behavior we expect.

[jportner]

preflightGetNamespaces and preflightCheckIncludesNamespace were very similar, so I combined them into a single preflightCheckNamespaces function.

[jportner]

Testing process:

I pulled down #114620 and merged this branch on top. Then I ran the functional tests for SOC.update. The tests passed 🎉

 debg KIBANA_CI_STATS_CONFIG environment variable not found, disabling CiStatsReporter
 debg Loading config file from "/Users/joe/GitHub/kibana-3/test/api_integration/config.js"
 debg Loading config file from "/Users/joe/GitHub/kibana-3/test/common/config.js"
 debg Loading config file from "/Users/joe/GitHub/kibana-3/test/functional/config.js"
 info Config loaded
 debg randomness seed: 1634166443943
 info Starting tests

 └-: apis
   └-> "before all" hook in "apis"
   └-: saved_objects
     └-> "before all" hook in "saved_objects"
     └-: update
       └-> "before all" hook for "should return 200"
       └-> "before all" hook for "should return 200"
         │ debg resolved import for test/api_integration/fixtures/kbn_archiver/saved_objects/basic.json to /Users/joe/GitHub/kibana-3/test/api_integration/fixtures/kbn_archiver/saved_objects/basic.json
         │ info importing 4 saved objects { space: undefined }
         │ succ import success
       └-> should return 200
         └-> "before each" hook: global before each for "should return 200"
         └- ✓ pass  (1.0s) "apis saved_objects update should return 200"
       └-> does not pass references if omitted
         └-> "before each" hook: global before each for "does not pass references if omitted"
         └- ✓ pass  (1.0s) "apis saved_objects update does not pass references if omitted"
       └-> passes references if they are provided
         └-> "before each" hook: global before each for "passes references if they are provided"
         └- ✓ pass  (1.0s) "apis saved_objects update passes references if they are provided"
       └-> passes empty references array if empty references array is provided
         └-> "before each" hook: global before each for "passes empty references array if empty references array is provided"
         └- ✓ pass  (1.0s) "apis saved_objects update passes empty references array if empty references array is provided"
       └-> handles upsert
         └-> "before each" hook: global before each for "handles upsert"
         └- ✓ pass  (2.1s) "apis saved_objects update handles upsert"
       └-: unknown id
         └-> "before all" hook for "should return a generic 404"
         └-> should return a generic 404
           └-> "before each" hook: global before each for "should return a generic 404"
           └- ✓ pass  (28ms) "apis saved_objects update unknown id should return a generic 404"
         └-> "after all" hook for "should return a generic 404"
       └-> "after all" hook for "handles upsert"
         │ debg unloading docs from archive at /Users/joe/GitHub/kibana-3/test/api_integration/fixtures/kbn_archiver/saved_objects/basic.json
         │ info deleting 4 objects { space: undefined }
         │ succ 4 saved objects deleted
       └-> "after all" hook for "handles upsert"
     └-> "after all" hook in "saved_objects"
   └-> "after all" hook in "apis"

6 passing (8.2s)

[pgayvallet]

Thanks for fixing this. Got a few remarks and questions before definitive approval:

[pgayvallet]

incrementCounterInternal also relies on upsert:

kibana/src/core/server/saved_objects/service/lib/repository.ts

Line 1813 in 58f6d90

upsert: raw._source,

I think we need to add the same !preflightResult?.found condition you did in update?

[jportner]

No, that was only necessary because update will use the version properties of the existing object (for optimistic concurrency control). If you make an ES update call with the version properties and the upsert field in your request, ES responds with an error.

incrementCounterInternal doesn't need to include the version properties at all because it only updates object attributes using a script.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 8500d48
• Storybooks Preview

Metrics [docs]

✅ unchanged

History

• 💚 Build #160726 succeeded dbd15e6

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[pgayvallet]

LGTM.

Thanks for making the checkResult changes. I agree this is more verbose for the callers, but just re-reviewing the code, it felt way easier to understand the logic this way.

[kibanamachine]

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.