Repository: HTTP-APIs/hydrus (python)

Issue:
Avoid insertion of duplicate members in a collection
### I'm submitting a 
- [x] bug report.
- [ ] feature request.

### Current Behaviour:
<!-- Describe about the bug -->
Currently, a member with the same ID can be added multiple times in the same collection.
### Expected Behaviour:
<!-- Describe what will happen if bug is removed -->
Member ID in Collection database table should be unique and hydrus should raise and handle the integrity error everytime an object with existing ID is inserted in a collection. A response with 400 (Bad Request) code should be returned instead of a server error.

### Steps to reproduce:
<!-- If you can then please provide the steps to reproduce the bug -->

- Make a PUT request to a Collection endpoint : `http://localhost:8080/serverapi/<collection_name>/<collection-id>`
- Insert an object again with same Object ID in JSON body using PUT request.

### Snapshot:
<!-- If you can then please provide the screenshot of the issue you are facing -->

### Environment:
<!-- Please provide the following environment details --> 
* python version
* pip version
* OS details

### Do you want to work on this issue?
<!-- yes/no -->
Yes


Pull request:
Unique constraint for member ID in collections
<!-- Please create/claim an issue before sending a PR -->
<!-- Add issue number (Eg: fixes #123) -->

Fixes #555

### Checklist
- [x] My branch is up-to-date with upstream/develop branch.
- [x] Everything works and tested for Python 3.6.0 and above.

### Description
<!-- Describe about what this PR does, previous state and new state of the output -->
- Previously,  a member with the same ID can be added multiple times in the same collection.
- Member ID in Collection database table is changed to a unique key and hydrus handles the integrity error and raises InstanceExists error if an object with existing ID is inserted in a collection. A response with 400 (Bad Request) code is returned.

```
{
    "@context": "http://www.w3.org/ns/hydra/context.jsonld",
    "@type": "Error",
    "description": "Instance of type Movie with ID c1b62361-e9c6-4185-b496-b74dff4fb8a4 already exists",
    "statusCode": 400,
    "title": "Instance already exists."
}
```

### Change logs

<!-- #### Added -->
<!-- Edit these points below to describe the new features added with this PR -->
<!-- - Feature 1 -->
<!-- - Feature 2 -->


<!-- #### Changed -->
<!-- Edit these points below to describe the changes made in existing functionality with this PR -->
<!-- - Change 1 -->
<!-- - Change 1 -->


<!-- #### Fixed -->
<!-- Edit these points below to describe the bug fixes made with this PR -->
<!-- - Bug 1 -->


<!-- #### Removed -->
<!-- Edit these points below to describe the removed features with this PR -->
<!-- - Deprecated feature 1 -->


Patch:
diff --git a/.travis.yml b/.travis.yml
index 5b69676..d1acfe8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,9 +1,11 @@
 language: python
 python:
   - "3.6"
-  - "3.6-dev"  # 3.6 development branch
+  - "3.6-dev" # 3.6 development branch
   - "3.7"
   - "3.7-dev" # 3.7 development branch
+  - "3.8"
+  - "3.8-dev" # 3.8 development branch
 install:
   - pip install -r requirements.txt --no-cache
 script: pytest
diff --git a/hydrus/data/db_models.py b/hydrus/data/db_models.py
index 2e08569..1b7b6af 100644
--- a/hydrus/data/db_models.py
+++ b/hydrus/data/db_models.py
@@ -16,6 +16,7 @@ from sqlalchemy import (
 )
 from sqlalchemy.engine import Engine
 from sqlalchemy.ext.declarative import declarative_base
+from sqlalchemy.schema import UniqueConstraint
 from sqlalchemy.sql import func
 from hydra_python_core.doc_writer import DocUrl
 # from hydrus.settings import DB_URL
@@ -112,6 +113,11 @@ class Resource:
             attr_dict['member_type'] = Column(
                 String,
             )
+            # if the class is a collection, then a member id and collection id
+            # should be a unique constraint altogether
+            attr_dict['__table_args__'] = (
+                UniqueConstraint(title, 'collection_id'),
+            )
         return attr_dict
 
     @staticmethod
diff --git a/hydrus/data/resource_based_classes.py b/hydrus/data/resource_based_classes.py
index e23f05a..d2ada2f 100644
--- a/hydrus/data/resource_based_classes.py
+++ b/hydrus/data/resource_based_classes.py
@@ -14,7 +14,7 @@ from hydrus.data.exceptions import (
     PropertyNotGiven,
 )
 from sqlalchemy import exists
-from sqlalchemy.exc import InvalidRequestError
+from sqlalchemy.exc import InvalidRequestError, IntegrityError
 from sqlalchemy.orm.scoping import scoped_session
 from sqlalchemy.orm.exc import NoResultFound
 from typing import Dict, Any
@@ -71,6 +71,11 @@ def insert_object(object_: Dict[str, Any], session: scoped_session,
                 session.commit()
             except InvalidRequestError:
                 session.rollback()
+            except IntegrityError:
+                session.rollback()
+                member_type_ = member['@type']
+                member_id_ = member['id_']
+                raise InstanceExists(member_type_, member_id_)
         return collection_id
     else:
         # when type_ is of a non-collection class


Test patch:
diff --git a/hydrus/tests/functional/test_app.py b/hydrus/tests/functional/test_app.py
index e142829..cc4b996 100644
--- a/hydrus/tests/functional/test_app.py
+++ b/hydrus/tests/functional/test_app.py
@@ -135,6 +135,26 @@ class TestApp():
                                                         data=json.dumps(dummy_object))
                 assert good_response_put.status_code == 201
 
+    def test_Collections_constraint_PUT(self, test_app_client, constants, doc):
+        """Test collection constraints by inserting same object twice."""
+        API_NAME = constants['API_NAME']
+        index = test_app_client.get(f'/{API_NAME}')
+        assert index.status_code == 200
+        endpoints = json.loads(index.data.decode('utf-8'))
+        for endpoint in endpoints['collections']:
+            collection_name = '/'.join(endpoint['@id'].split(f'/{API_NAME}/')[1:])
+            collection = doc.collections[collection_name]['collection']
+            collection_methods = [x.method for x in collection.supportedOperation]
+            if 'PUT' in collection_methods:
+                dummy_object = gen_dummy_object(collection.name, doc)
+                good_response_put = test_app_client.put(endpoint['@id'],
+                                                        data=json.dumps(dummy_object))
+                assert good_response_put.status_code == 201
+                collection_id = good_response_put.location
+                bad_response_put = test_app_client.put(collection_id,
+                                                       data=json.dumps(dummy_object))
+                assert bad_response_put.status_code == 400
+
     def test_collection_object_GET(self, test_app_client, constants, doc):
         """Test GET of a given collection object using ID."""
         API_NAME = constants['API_NAME']
@@ -170,10 +190,16 @@ class TestApp():
             collection = doc.collections[collection_name]['collection']
             collection_methods = [x.method for x in collection.supportedOperation]
             if 'PUT' in collection_methods:
-                dummy_object = gen_dummy_object(collection.name, doc)
+                dummy_object_1 = gen_dummy_object(collection.name, doc)
                 initial_put_response = test_app_client.put(
-                    endpoint['@id'], data=json.dumps(dummy_object))
+                    endpoint['@id'], data=json.dumps(dummy_object_1))
                 assert initial_put_response.status_code == 201
+                collection_id = initial_put_response.location
+                dummy_object_2 = gen_dummy_object(collection.name, doc)
+                second_put_response = test_app_client.put(
+                    collection_id, data=json.dumps(dummy_object_2))
+                assert second_put_response.status_code == 201
+                assert second_put_response.location == collection_id
 
     def test_collection_object_POST(self, test_app_client, constants, doc, socketio):
         """Test POST of a given collection object using ID."""