[
https://issues.apache.org/jira/browse/UNOMI-983?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jonathan Sinovassin-Naïk reassigned UNOMI-983:
----------------------------------------------
Assignee: Jonathan Sinovassin-Naïk
> Saving an item with no itemId overwrites a single shared document
> -----------------------------------------------------------------
>
> Key: UNOMI-983
> URL: https://issues.apache.org/jira/browse/UNOMI-983
> Project: Apache Unomi
> Issue Type: Bug
> Reporter: Jonathan Sinovassin-Naïk
> Assignee: Jonathan Sinovassin-Naïk
> Priority: Major
>
> {{POST /cxs/profiles}} accepts a body that carries no {{itemId}}. Apache
> Unomi 3.1 stores every such
> item under the document id {{<tenantId>_null}}, so the second call overwrites
> the item the first call
> created. The response returns 200 and the literal string {{null}} as the
> {{itemId}}.
> The failure is silent. Unomi logs no warning and no error, and a caller that
> reads the response back
> sees a saved item.
> Apache Unomi 2.7.x and 3.0.x behave differently, and the second write creates
> a second item there.
> h2. Where the behaviour comes from
> Two parts of the code produce it.
> {{ElasticSearchPersistenceServiceImpl.getDocumentIDForItemType}} builds the
> document id by prefixing
> the tenant, and guards no null:
> {code:java}
> private String getDocumentIDForItemType(String itemId, String itemType) {
> String tenantId = getTenantId();
> String baseId = systemItems.contains(itemType) ? (itemId + "_" +
> itemType.toLowerCase()) : itemId;
> return tenantId + "_" + baseId;
> }
> {code}
> The same method on {{unomi-2.7.x}} and on {{unomi-3.0.x}} returns the item id
> unchanged, so
> Elasticsearch generates a distinct document id for each item:
> {code:java}
> private String getDocumentIDForItemType(String itemId, String itemType) {
> return systemItems.contains(itemType) ? (itemId + "_" +
> itemType.toLowerCase()) : itemId;
> }
> {code}
> {{ProfileServiceImpl.save(Profile, boolean)}} already refuses a null item id
> and returns {{null}}.
> The REST endpoint does not reach that guard, because it calls
> {{saveOrMerge}}, which carries no such
> check.
> h2. Steps to reproduce
> # Start Apache Unomi 3.1.0-SNAPSHOT.
> # Post a first item with no {{itemId}} to {{/cxs/profiles}}:
> {code:json}
> { "itemType": "persona",
> "properties": { "firstName": "First", "lastName": "Item" },
> "systemProperties": {} }
> {code}
> # Post a second item with no {{itemId}} to {{/cxs/profiles}}:
> {code:json}
> { "itemType": "persona",
> "properties": { "firstName": "Second", "lastName": "Item" },
> "systemProperties": {} }
> {code}
> # Read the persona documents from the profile index.
> h2. Expected result
> The two calls create two items. Refusing both calls with a 400 is also a
> correct answer, because
> {{itemId}} carries the identity of an item.
> h2. Actual result
> One document holds the second item, and the first item is gone:
> {noformat}
> _id= system_europeanVisitor {"firstName": "European", "lastName": "Visitor"}
> _id= system_usVisitor {"firstName": "U.S.", "lastName": "Visitor"}
> _id= default_null {"firstName": "Second", "lastName": "Item"}
> {noformat}
> Each call returns 200, and the response body carries {{"itemId":"null"}}.
> h2. Scope
> Every write that reaches the persistence layer with a null item id is
> affected, whatever the item
> type. An integration that relied on Elasticsearch to generate the document id
> now loses data on the
> second write.
> h2. Proposed fix
> Refuse a null item id where the identity is decided, rather than inventing
> one in the persistence
> layer. Two changes carry that:
> * {{ProfileServiceImpl.saveOrMerge}} applies the same null check as
> {{save(Profile, boolean)}}, so the REST endpoint answers 400 instead of 200.
> * {{ElasticSearchPersistenceServiceImpl.getDocumentIDForItemType}} throws on
> a null item id, so no caller can reach the shared document by another route.
> Generating a UUID in place of the null is the other possible answer. It
> restores the 3.0 outcome,
> and it hides a caller that never states the identity of what it writes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
