opcservices: Enforce unique relationship id.

Signed-off-by: Nikolay Sivov <nsivov@codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard@winehq.org>

dlls/opcservices/package.c

@@ -1160,6 +1160,20 @@ static const IOpcRelationshipVtbl opc_relationship_vtbl =
     opc_relationship_GetTargetMode,
 };
 
+static struct opc_relationship *opc_relationshipset_get_rel(struct opc_relationship_set *relationship_set,
+        const WCHAR *id)
+{
+    size_t i;
+
+    for (i = 0; i < relationship_set->count; i++)
+    {
+        if (!strcmpW(id, relationship_set->relationships[i]->id))
+            return relationship_set->relationships[i];
+    }
+
+    return NULL;
+}
+
 static HRESULT opc_relationship_create(struct opc_relationship_set *set, const WCHAR *id, const WCHAR *type,
         IUri *target_uri, OPC_URI_TARGET_MODE target_mode, IOpcRelationship **out)
 {
@@ -1179,7 +1193,6 @@ static HRESULT opc_relationship_create(struct opc_relationship_set *set, const W
     relationship->source_uri = set->source_uri;
     IOpcUri_AddRef(relationship->source_uri);
 
-    /* FIXME: test that id is unique */
     if (id)
         relationship->id = opc_strdupW(id);
     else
@@ -1190,8 +1203,16 @@ static HRESULT opc_relationship_create(struct opc_relationship_set *set, const W
             static const WCHAR fmtW[] = {'R','%','0','8','X',0};
             DWORD generated;
 
+            /* FIXME: test that generated id is unique */
             RtlGenRandom(&generated, sizeof(generated));
             sprintfW(relationship->id, fmtW, generated);
+
+            if (opc_relationshipset_get_rel(set, relationship->id))
+            {
+                WARN("Newly generated id %s already exists.\n", debugstr_w(relationship->id));
+                IOpcRelationship_Release(&relationship->IOpcRelationship_iface);
+                return E_FAIL;
+            }
         }
     }
 
@@ -1258,20 +1279,6 @@ static ULONG WINAPI opc_relationship_set_Release(IOpcRelationshipSet *iface)
     return refcount;
 }
 
-static struct opc_relationship *opc_relationshipset_get_item(struct opc_relationship_set *relationship_set,
-        const WCHAR *id)
-{
-    size_t i;
-
-    for (i = 0; i < relationship_set->count; i++)
-    {
-        if (!strcmpW(id, relationship_set->relationships[i]->id))
-            return relationship_set->relationships[i];
-    }
-
-    return NULL;
-}
-
 static HRESULT WINAPI opc_relationship_set_GetRelationship(IOpcRelationshipSet *iface, const WCHAR *id,
         IOpcRelationship **relationship)
 {
@@ -1288,7 +1295,7 @@ static HRESULT WINAPI opc_relationship_set_GetRelationship(IOpcRelationshipSet *
     if (!id)
         return E_POINTER;
 
-    if ((ret = opc_relationshipset_get_item(relationship_set, id)))
+    if ((ret = opc_relationshipset_get_rel(relationship_set, id)))
     {
         *relationship = &ret->IOpcRelationship_iface;
         IOpcRelationship_AddRef(*relationship);
@@ -1314,6 +1321,9 @@ static HRESULT WINAPI opc_relationship_set_CreateRelationship(IOpcRelationshipSe
     if (!type || !target_uri)
         return E_POINTER;
 
+    if (id && opc_relationshipset_get_rel(relationship_set, id))
+        return OPC_E_DUPLICATE_RELATIONSHIP;
+
     if (IUri_GetPropertyLength(target_uri, Uri_PROPERTY_SCHEME_NAME, &length, 0) == S_OK && length != 0
             && target_mode == OPC_URI_TARGET_MODE_INTERNAL)
         return OPC_E_INVALID_RELATIONSHIP_TARGET;
@@ -1337,7 +1347,7 @@ static HRESULT WINAPI opc_relationship_set_RelationshipExists(IOpcRelationshipSe
     if (!id || !exists)
         return E_POINTER;
 
-    *exists = opc_relationshipset_get_item(relationship_set, id) != NULL;
+    *exists = opc_relationshipset_get_rel(relationship_set, id) != NULL;
 
     return S_OK;
 }

dlls/opcservices/tests/opcservices.c

@@ -384,14 +384,20 @@ static void test_relationship(void)
     hr = IOpcRelationshipSet_CreateRelationship(rels, NULL, typeW, target_uri, OPC_URI_TARGET_MODE_INTERNAL, &rel);
     ok(SUCCEEDED(hr), "Failed to create relationship, hr %#x.\n", hr);
 
-    hr = IOpcRelationshipSet_CreateRelationship(rels, NULL, typeW, target_uri, OPC_URI_TARGET_MODE_INTERNAL, &rel2);
-    ok(SUCCEEDED(hr), "Failed to create relationship, hr %#x.\n", hr);
-
     /* Autogenerated relationship id */
     hr = IOpcRelationship_GetId(rel, &id);
     ok(SUCCEEDED(hr), "Failed to get id, hr %#x.\n", hr);
     ok(lstrlenW(id) == 9 && *id == 'R', "Unexpected relationship id %s.\n", wine_dbgstr_w(id));
 
+    hr = IOpcRelationshipSet_CreateRelationship(rels, id, typeW, target_uri, OPC_URI_TARGET_MODE_INTERNAL, &rel2);
+    ok(hr == OPC_E_DUPLICATE_RELATIONSHIP, "Failed to create relationship, hr %#x.\n", hr);
+
+    hr = IOpcRelationshipSet_CreateRelationship(rels, id, typeW, target_uri2, OPC_URI_TARGET_MODE_INTERNAL, &rel2);
+    ok(hr == OPC_E_DUPLICATE_RELATIONSHIP, "Failed to create relationship, hr %#x.\n", hr);
+
+    hr = IOpcRelationshipSet_CreateRelationship(rels, NULL, typeW, target_uri, OPC_URI_TARGET_MODE_INTERNAL, &rel2);
+    ok(SUCCEEDED(hr), "Failed to create relationship, hr %#x.\n", hr);
+
     ret = 123;
     hr = IOpcRelationshipSet_RelationshipExists(rels, NULL, &ret);
     ok(hr == E_POINTER, "Unexpected hr %#x.\n", hr);

include/opcbase.idl

@@ -45,6 +45,7 @@ cpp_quote("#define OPC_E_NONCONFORMING_URI MAKE_HRESULT(SEVERITY_ERROR, FACILITY
 cpp_quote("#define OPC_E_RELATIONSHIP_URI_REQUIRED MAKE_HRESULT(SEVERITY_ERROR, FACILITY_OPC, 0x3)")
 cpp_quote("#define OPC_E_DUPLICATE_PART MAKE_HRESULT(SEVERITY_ERROR, FACILITY_OPC, 0xb)")
 cpp_quote("#define OPC_E_INVALID_RELATIONSHIP_TARGET MAKE_HRESULT(SEVERITY_ERROR, FACILITY_OPC, 0x12)")
+cpp_quote("#define OPC_E_DUPLICATE_RELATIONSHIP MAKE_HRESULT(SEVERITY_ERROR, FACILITY_OPC, 0x13)")
 cpp_quote("#define OPC_E_NO_SUCH_PART MAKE_HRESULT(SEVERITY_ERROR, FACILITY_OPC, 0x18)")
 cpp_quote("#define OPC_E_NO_SUCH_RELATIONSHIP MAKE_HRESULT(SEVERITY_ERROR, FACILITY_OPC, 0x48)")
 cpp_quote("#define OPC_E_ENUM_COLLECTION_CHANGED MAKE_HRESULT(SEVERITY_ERROR, FACILITY_OPC, 0x50)")