Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unique constraints appearantly not checked when updating document #151

Closed
xyrus02 opened this issue Apr 11, 2019 · 4 comments
Closed

Unique constraints appearantly not checked when updating document #151

xyrus02 opened this issue Apr 11, 2019 · 4 comments
Assignees
Labels
Milestone

Comments

@xyrus02
Copy link

xyrus02 commented Apr 11, 2019

Given a NitriteCollection labeled list exists, for which there is a unique index for field "Data"
Given a document labeled 1 exists in the collection list with field "Data" = "test"
Given another document labeled 2 exists in the collection list with field "Data" = "test2"

[ { "_id": 1, "Data": "test"}, { "_id": 2, "Data": "test2"} ]

Reproduction steps: 2 would be updated so that "Data" changes to "test" knowing that the same value already exists on 1 and would violate the unique constraint. This would be achived by fetching 2 from the collection, setting "Data" to test and attempting to call update on the collection to store the changes.

Expectation: an exception is raised similar to a scenario where a third document with "Data" = "test" would be inserted.

Observation: no exception is raised. The change is stored and now the value "test" is not unique anymore within list - despite the unique constraint.

@xyrus02
Copy link
Author

xyrus02 commented Apr 11, 2019

I suspect the error to be in DataService.java:186:

indexingService.refreshIndexEntry(oldDocument, document, nitriteId);

I think it should be:

indexingService.refreshIndexEntry(oldDocument, update, nitriteId);

@anidotnet
Copy link
Contributor

I could not reproduce this issue. Here is a small test

@Test
public void testIssue151() {
    Document doc1 = new Document().put("id", "test-1").put("fruit", "Apple");
    Document doc2 = new Document().put("id", "test-2").put("fruit", "Ôrange");
    NitriteCollection coll = db.getCollection("test");
    coll.insert(doc1, doc2);

    coll.createIndex("fruit", indexOptions(IndexType.Unique));

    assertEquals(coll.find(eq("fruit", "Apple")).totalCount(), 1);

    Document doc3 = new Document(coll.find(eq("id", "test-2")).firstOrDefault());

    doc3.put("fruit", "Apple");
    coll.update(doc3);

    assertEquals(coll.find(eq("fruit", "Apple")).totalCount(), 2);
}

I am getting below valid exception

org.dizitart.no2.exceptions.UniqueConstraintException: NO2.10004: unique key constraint violation for fruit

	at org.dizitart.no2.internals.IndexingService.refreshIndexEntry(IndexingService.java:229)
	at org.dizitart.no2.internals.DataService.update(DataService.java:186)
	at org.dizitart.no2.internals.NitriteService.update(NitriteService.java:361)
	at org.dizitart.no2.internals.DefaultNitriteCollection.update(DefaultNitriteCollection.java:322)
	at org.dizitart.no2.internals.DefaultNitriteCollection.update(DefaultNitriteCollection.java:311)
	at org.dizitart.no2.internals.DefaultNitriteCollection.update(DefaultNitriteCollection.java:286)
	at org.dizitart.no2.internals.DefaultNitriteCollection.update(DefaultNitriteCollection.java:47)
	at org.dizitart.no2.CollectionFindByIndexTest.testIssue151(CollectionFindByIndexTest.java:389)

Probably you have missed this

Document doc3 = new Document(coll.find(eq("id", "test-2")).firstOrDefault());

Unless you copy it to a new document, any modification will be in-place. Even if you don't call update on collection, the document object will be updated.

On a second thought, I can make a small change, so that you will get a cloned document instead of the actual one and collection will be protected from this kind of mistakes.

@anidotnet anidotnet self-assigned this Apr 13, 2019
@anidotnet anidotnet added the bug label Apr 13, 2019
@anidotnet anidotnet added this to the 3.3.0 milestone Apr 13, 2019
@anidotnet
Copy link
Contributor

Now this fix is available in 3.3.0-SNAPSHOT.

@xyrus02
Copy link
Author

xyrus02 commented Apr 14, 2019

Thanks for picking this up and making the interface easier to use. I see your point with the necessity to clone the item before modifying it. I was indeed unter the impression that any "get" operation provides me with a clone, thus the whole point for an "update" operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

2 participants