[#311] AddressBook#addPerson() does not copy the Person (#313)

When accepting a new Person, both UniquePersonList and AddressBook
insert the original object, rather than a copy of the object.

Outside classes are able to indirectly modify the list by using the
same object after inserting them into the list. This is undesirable as
UniquePersonList and AddressBook cannot guarantee that the constraints,
such as the uniqueness of the list, continue to hold, which results in
incorrect behavior for both classes.

Let's ensure that UniquePersonList and AddressBook are actually storing
copies of the Person object, rather than the original object, so that
outside classes are not able to modify them after it is inserted into
the list or book.

src/main/java/seedu/address/model/AddressBook.java

@@ -83,9 +83,10 @@ public void resetData(ReadOnlyAddressBook newData) {
      *
      * @throws UniquePersonList.DuplicatePersonException if an equivalent person already exists.
      */
-    public void addPerson(Person p) throws UniquePersonList.DuplicatePersonException {
-        syncMasterTagListWith(p);
-        persons.add(p);
+    public void addPerson(ReadOnlyPerson p) throws UniquePersonList.DuplicatePersonException {
+        Person newPerson = new Person(p);
+        syncMasterTagListWith(newPerson);
+        persons.add(newPerson);
     }
 
     /**

src/main/java/seedu/address/model/Model.java

@@ -3,7 +3,6 @@
 import java.util.Set;
 
 import seedu.address.commons.core.UnmodifiableObservableList;
-import seedu.address.model.person.Person;
 import seedu.address.model.person.ReadOnlyPerson;
 import seedu.address.model.person.UniquePersonList;
 import seedu.address.model.person.UniquePersonList.DuplicatePersonException;
@@ -22,7 +21,7 @@ public interface Model {
     void deletePerson(ReadOnlyPerson target) throws UniquePersonList.PersonNotFoundException;
 
     /** Adds the given person */
-    void addPerson(Person person) throws UniquePersonList.DuplicatePersonException;
+    void addPerson(ReadOnlyPerson person) throws UniquePersonList.DuplicatePersonException;
 
     /**
      * Updates the person located at {@code filteredPersonListIndex} with {@code editedPerson}.

src/main/java/seedu/address/model/ModelManager.java

@@ -10,7 +10,6 @@
 import seedu.address.commons.events.model.AddressBookChangedEvent;
 import seedu.address.commons.util.CollectionUtil;
 import seedu.address.commons.util.StringUtil;
-import seedu.address.model.person.Person;
 import seedu.address.model.person.ReadOnlyPerson;
 import seedu.address.model.person.UniquePersonList;
 import seedu.address.model.person.UniquePersonList.PersonNotFoundException;
@@ -65,7 +64,7 @@ public synchronized void deletePerson(ReadOnlyPerson target) throws PersonNotFou
     }
 
     @Override
-    public synchronized void addPerson(Person person) throws UniquePersonList.DuplicatePersonException {
+    public synchronized void addPerson(ReadOnlyPerson person) throws UniquePersonList.DuplicatePersonException {
         addressBook.addPerson(person);
         updateFilteredListToShowAll();
         indicateAddressBookChanged();

src/main/java/seedu/address/model/person/UniquePersonList.java

@@ -34,12 +34,12 @@ public boolean contains(ReadOnlyPerson toCheck) {
      *
      * @throws DuplicatePersonException if the person to add is a duplicate of an existing person in the list.
      */
-    public void add(Person toAdd) throws DuplicatePersonException {
+    public void add(ReadOnlyPerson toAdd) throws DuplicatePersonException {
         assert toAdd != null;
         if (contains(toAdd)) {
             throw new DuplicatePersonException();
         }
-        internalList.add(toAdd);
+        internalList.add(new Person(toAdd));
     }
 
     /**

src/test/java/seedu/address/logic/commands/AddCommandTest.java

@@ -69,7 +69,7 @@ private AddCommand getAddCommandForPerson(Person person, Model model) throws Ill
      */
     private class ModelStub implements Model {
         @Override
-        public void addPerson(Person person) throws DuplicatePersonException {
+        public void addPerson(ReadOnlyPerson person) throws DuplicatePersonException {
             fail("This method should not be called.");
         }
 
@@ -117,7 +117,7 @@ public void updateFilteredPersonList(Set<String> keywords) {
      */
     private class ModelStubThrowingDuplicatePersonException extends ModelStub {
         @Override
-        public void addPerson(Person person) throws DuplicatePersonException {
+        public void addPerson(ReadOnlyPerson person) throws DuplicatePersonException {
             throw new DuplicatePersonException();
         }
     }
@@ -129,8 +129,8 @@ private class ModelStubAcceptingPersonAdded extends ModelStub {
         final ArrayList<Person> personsAdded = new ArrayList<>();
 
         @Override
-        public void addPerson(Person person) throws DuplicatePersonException {
-            personsAdded.add(person);
+        public void addPerson(ReadOnlyPerson person) throws DuplicatePersonException {
+            personsAdded.add(new Person(person));
         }
     }