Address remaining items from fifth round of PR review feedback

jportner · jportner · commit e2f5d6573c45 · 2020-04-06T13:15:52.000-04:00
diff --git a/docs/api/saved-objects/bulk_create.asciidoc b/docs/api/saved-objects/bulk_create.asciidoc
@@ -104,7 +104,7 @@ The API returns the following:
       "type": "dashboard",
       "error": {
         "statusCode": 409,
-        "message": "version conflict, document already exists"
+        "message": "Saved object [dashboard/be3733a0-9efe-11e7-acb3-3dab96693fab] conflict"
       }
     }
   ]
diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.addtonamespaces.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.addtonamespaces.md
@@ -4,6 +4,8 @@
 
 ## SavedObjectsRepository.addToNamespaces() method
 
+Adds one or more namespaces to a given multi-namespace saved object. This method and \[`deleteFromNamespaces`<!-- -->\][SavedObjectsRepository.deleteFromNamespaces()](./kibana-plugin-core-server.savedobjectsrepository.deletefromnamespaces.md) are the only ways to change which Spaces a multi-namespace saved object is shared to.
+
 <b>Signature:</b>
 
 ```typescript
diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.deletefromnamespaces.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.deletefromnamespaces.md
@@ -4,6 +4,8 @@
 
 ## SavedObjectsRepository.deleteFromNamespaces() method
 
+Removes one or more namespaces from a given multi-namespace saved object. If no namespaces remain, the saved object is deleted entirely. This method and \[`addToNamespaces`<!-- -->\][SavedObjectsRepository.addToNamespaces()](./kibana-plugin-core-server.savedobjectsrepository.addtonamespaces.md) are the only ways to change which Spaces a multi-namespace saved object is shared to.
+
 <b>Signature:</b>
 
 ```typescript
diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectsrepository.md
@@ -15,14 +15,14 @@ export declare class SavedObjectsRepository
 
 |  Method | Modifiers | Description |
 |  --- | --- | --- |
-|  [addToNamespaces(type, id, namespaces, options)](./kibana-plugin-core-server.savedobjectsrepository.addtonamespaces.md) |  |  |
+|  [addToNamespaces(type, id, namespaces, options)](./kibana-plugin-core-server.savedobjectsrepository.addtonamespaces.md) |  | Adds one or more namespaces to a given multi-namespace saved object. This method and \[<code>deleteFromNamespaces</code>\][SavedObjectsRepository.deleteFromNamespaces()](./kibana-plugin-core-server.savedobjectsrepository.deletefromnamespaces.md) are the only ways to change which Spaces a multi-namespace saved object is shared to. |
 |  [bulkCreate(objects, options)](./kibana-plugin-core-server.savedobjectsrepository.bulkcreate.md) |  | Creates multiple documents at once |
 |  [bulkGet(objects, options)](./kibana-plugin-core-server.savedobjectsrepository.bulkget.md) |  | Returns an array of objects by id |
 |  [bulkUpdate(objects, options)](./kibana-plugin-core-server.savedobjectsrepository.bulkupdate.md) |  | Updates multiple objects in bulk |
 |  [create(type, attributes, options)](./kibana-plugin-core-server.savedobjectsrepository.create.md) |  | Persists an object |
 |  [delete(type, id, options)](./kibana-plugin-core-server.savedobjectsrepository.delete.md) |  | Deletes an object |
 |  [deleteByNamespace(namespace, options)](./kibana-plugin-core-server.savedobjectsrepository.deletebynamespace.md) |  | Deletes all objects from the provided namespace. |
-|  [deleteFromNamespaces(type, id, namespaces, options)](./kibana-plugin-core-server.savedobjectsrepository.deletefromnamespaces.md) |  |  |
+|  [deleteFromNamespaces(type, id, namespaces, options)](./kibana-plugin-core-server.savedobjectsrepository.deletefromnamespaces.md) |  | Removes one or more namespaces from a given multi-namespace saved object. If no namespaces remain, the saved object is deleted entirely. This method and \[<code>addToNamespaces</code>\][SavedObjectsRepository.addToNamespaces()](./kibana-plugin-core-server.savedobjectsrepository.addtonamespaces.md) are the only ways to change which Spaces a multi-namespace saved object is shared to. |
 |  [find({ search, defaultSearchOperator, searchFields, hasReference, page, perPage, sortField, sortOrder, fields, namespace, type, filter, })](./kibana-plugin-core-server.savedobjectsrepository.find.md) |  |  |
 |  [get(type, id, options)](./kibana-plugin-core-server.savedobjectsrepository.get.md) |  | Gets a single object |
 |  [incrementCounter(type, id, counterFieldName, options)](./kibana-plugin-core-server.savedobjectsrepository.incrementcounter.md) |  | Increases a counter field by one. Creates the document if one doesn't exist for the given id. |
diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
@@ -349,7 +349,7 @@ export class SavedObjectsRepository {
         const indexFound = bulkGetResponse.status !== 404;
         const actualResult = indexFound ? bulkGetResponse.docs[esRequestIndex] : undefined;
         const docFound = indexFound && actualResult.found === true;
-        if (docFound && !this._rawInNamespaces(actualResult, namespace)) {
+        if (docFound && !this.rawDocExistsInNamespace(actualResult, namespace)) {
           const { id, type } = object;
           return {
             tag: 'Left' as 'Left',
@@ -768,7 +768,7 @@ export class SavedObjectsRepository {
         const { type, id, esRequestIndex } = expectedResult.value;
         const doc = bulkGetResponse.docs[esRequestIndex];
 
-        if (!doc.found || !this._rawInNamespaces(doc, namespace)) {
+        if (!doc.found || !this.rawDocExistsInNamespace(doc, namespace)) {
           return ({
             id,
             type,
@@ -819,7 +819,7 @@ export class SavedObjectsRepository {
 
     const docNotFound = response.found === false;
     const indexNotFound = response.status === 404;
-    if (docNotFound || indexNotFound || !this._rawInNamespaces(response, namespace)) {
+    if (docNotFound || indexNotFound || !this.rawDocExistsInNamespace(response, namespace)) {
       // see "404s from missing index" above
       throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
     }
@@ -904,6 +904,11 @@ export class SavedObjectsRepository {
     };
   }
 
+  /**
+   * Adds one or more namespaces to a given multi-namespace saved object. This method and
+   * [`deleteFromNamespaces`]{@link SavedObjectsRepository.deleteFromNamespaces} are the only ways to change which Spaces a multi-namespace
+   * saved object is shared to.
+   */
   async addToNamespaces(
     type: string,
     id: string,
@@ -957,6 +962,11 @@ export class SavedObjectsRepository {
     return {};
   }
 
+  /**
+   * Removes one or more namespaces from a given multi-namespace saved object. If no namespaces remain, the saved object is deleted
+   * entirely. This method and [`addToNamespaces`]{@link SavedObjectsRepository.addToNamespaces} are the only ways to change which Spaces a
+   * multi-namespace saved object is shared to.
+   */
   async deleteFromNamespaces(
     type: string,
     id: string,
@@ -1127,7 +1137,7 @@ export class SavedObjectsRepository {
           const indexFound = bulkGetResponse.status !== 404;
           const actualResult = indexFound ? bulkGetResponse.docs[esRequestIndex] : undefined;
           const docFound = indexFound && actualResult.found === true;
-          if (!docFound || !this._rawInNamespaces(actualResult, namespace)) {
+          if (!docFound || !this.rawDocExistsInNamespace(actualResult, namespace)) {
             return {
               tag: 'Left' as 'Left',
               error: {
@@ -1344,7 +1354,7 @@ export class SavedObjectsRepository {
     return omit(savedObject, 'namespace');
   }
 
-  private _rawInNamespaces(raw: SavedObjectsRawDoc, namespace?: string) {
+  private rawDocExistsInNamespace(raw: SavedObjectsRawDoc, namespace?: string) {
     const rawDocType = raw._source.type as string;
 
     // if the type is namespace isolated, or namespace agnostic, we can continue to rely on the guarantees
@@ -1383,7 +1393,7 @@ export class SavedObjectsRepository {
     const indexFound = response.status !== 404;
     const docFound = indexFound && response.found === true;
     if (docFound) {
-      if (!this._rawInNamespaces(response, namespace)) {
+      if (!this.rawDocExistsInNamespace(response, namespace)) {
         throw SavedObjectsErrorHelpers.createConflictError(type, id);
       }
       return getSavedObjectNamespaces(namespace, response);
@@ -1415,7 +1425,7 @@ export class SavedObjectsRepository {
 
     const indexFound = response.status !== 404;
     const docFound = indexFound && response.found === true;
-    if (!docFound || !this._rawInNamespaces(response, namespace)) {
+    if (!docFound || !this.rawDocExistsInNamespace(response, namespace)) {
       throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
     }
     return response as SavedObjectsRawDoc;
diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md
@@ -2111,7 +2111,6 @@ export interface SavedObjectsRawDoc {
 
 // @public (undocumented)
 export class SavedObjectsRepository {
-    // (undocumented)
     addToNamespaces(type: string, id: string, namespaces: string[], options?: SavedObjectsAddToNamespacesOptions): Promise<{}>;
     bulkCreate<T = unknown>(objects: Array<SavedObjectsBulkCreateObject<T>>, options?: SavedObjectsCreateOptions): Promise<SavedObjectsBulkResponse<T>>;
     bulkGet<T = unknown>(objects?: SavedObjectsBulkGetObject[], options?: SavedObjectsBaseOptions): Promise<SavedObjectsBulkResponse<T>>;
@@ -2123,7 +2122,6 @@ export class SavedObjectsRepository {
     static createRepository(migrator: KibanaMigrator, typeRegistry: SavedObjectTypeRegistry, indexName: string, callCluster: APICaller, extraTypes?: string[], injectedConstructor?: any): ISavedObjectsRepository;
     delete(type: string, id: string, options?: SavedObjectsDeleteOptions): Promise<{}>;
     deleteByNamespace(namespace: string, options?: SavedObjectsDeleteByNamespaceOptions): Promise<any>;
-    // (undocumented)
     deleteFromNamespaces(type: string, id: string, namespaces: string[], options?: SavedObjectsDeleteFromNamespacesOptions): Promise<{}>;
     // (undocumented)
     find<T = unknown>({ search, defaultSearchOperator, searchFields, hasReference, page, perPage, sortField, sortOrder, fields, namespace, type, filter, }: SavedObjectsFindOptions): Promise<SavedObjectsFindResponse<T>>;

Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,7 @@ The API returns the following:`
`104`	`104`	`"type": "dashboard",`
`105`	`105`	`"error": {`
`106`	`106`	`"statusCode": 409,`
`107`		`- "message": "version conflict, document already exists"`
	`107`	`+ "message": "Saved object [dashboard/be3733a0-9efe-11e7-acb3-3dab96693fab] conflict"`
`108`	`108`	`}`
`109`	`109`	`}`
`110`	`110`	`]`