Jonathan Sinovassin-Naïk created UNOMI-983:
----------------------------------------------
Summary: 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
=== DESCRIPTION (wiki markup Jira, à coller tel quel) ===
{{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)
