vcs: deprecate setters in LocalChangeList

Prior to the recent fix, `ChangeListWorker.getCopyByName()` didn't
perform a defensive copying of LocalChangeList.
As a result, `ChangeListManager.findChangeList("").setComment("")`
might'vs actually work.
But it didn't notify listeners and the change might've been reverted,
if performed during CLM refresh.
Also, `setName()` could break inner mappings in ChangeListWorker.

We can't remove these setters from API yet, so just replace them with
an actual call to the ChangeListManager.
This commit is contained in:
Aleksey Pivovarov
2017-05-29 21:33:41 +03:00
parent bcc2ffd582
commit 8e9954a0aa
7 changed files with 48 additions and 22 deletions
@@ -903,7 +903,7 @@ public class TaskManagerImpl extends TaskManager implements ProjectComponent, Pe
if (associatedTask != null) {
associatedTask.removeChangelist(new ChangeListInfo(changeList));
}
changeList.setComment(comment);
myChangeListManager.editComment(name, comment);
}
task.addChangelist(new ChangeListInfo(changeList));
myChangeListManager.setDefaultChangeList(changeList);
@@ -50,19 +50,13 @@ public abstract class LocalChangeList implements Cloneable, ChangeList {
@NotNull
public abstract String getName();
public abstract void setName(@NotNull String name);
@Nullable
public abstract String getComment();
public abstract void setComment(@Nullable String comment);
public abstract boolean isDefault();
public abstract boolean isReadOnly();
public abstract void setReadOnly(boolean isReadOnly);
/**
* Get additional data associated with this changelist.
*/
@@ -74,4 +68,23 @@ public abstract class LocalChangeList implements Cloneable, ChangeList {
public boolean hasDefaultName() {
return DEFAULT_NAME.equals(getName());
}
/**
* Use {@link ChangeListManager#editName}
*/
@Deprecated
public abstract void setName(@NotNull String name);
/**
* Use {@link ChangeListManager#editComment}
*/
@Deprecated
public abstract void setComment(@Nullable String comment);
/**
* Use {@link ChangeListManager#setReadOnly}
*/
@Deprecated
public abstract void setReadOnly(boolean isReadOnly);
}
@@ -142,7 +142,7 @@ class ChangeListManagerSerialization {
String comment = listNode.getAttributeValue(ATT_COMMENT);
LocalChangeListImpl list = LocalChangeListImpl.createEmptyChangeListImpl(project, name, id);
list.setComment(comment);
list.setCommentImpl(comment);
for (Element changeNode : listNode.getChildren(NODE_CHANGE)) {
list.addChange(readChange(changeNode));
@@ -152,7 +152,7 @@ class ChangeListManagerSerialization {
list.setDefault(true);
}
if (ATT_VALUE_TRUE.equals(listNode.getAttributeValue(ATT_READONLY))) {
list.setReadOnly(true);
list.setReadOnlyImpl(true);
}
return list;
@@ -186,9 +186,9 @@ public class ChangeListWorker {
}
public boolean setReadOnly(String name, boolean value) {
final LocalChangeList list = myMap.get(name);
final LocalChangeListImpl list = myMap.get(name);
if (list != null) {
list.setReadOnly(value);
list.setReadOnlyImpl(value);
}
return list != null;
}
@@ -207,7 +207,7 @@ public class ChangeListWorker {
}
LocalChangeListImpl newList = LocalChangeListImpl.createEmptyChangeListImpl(myProject, name, null);
newList.setComment(description);
newList.setCommentImpl(description);
newList.setData(data);
myMap.put(name, newList);
@@ -313,7 +313,7 @@ public class ChangeListWorker {
final LocalChangeListImpl list = myMap.get(fromName);
if (list == null || list.isReadOnly()) return false;
list.setName(toName);
list.setNameImpl(toName);
myMap.remove(fromName);
myMap.put(toName, list);
@@ -327,7 +327,7 @@ public class ChangeListWorker {
final String oldComment = list.getComment();
if (!Comparing.equal(oldComment, newComment)) {
list.setComment(newComment);
list.setCommentImpl(newComment);
}
return oldComment;
}
@@ -77,8 +77,7 @@ public class LocalChangeListImpl extends LocalChangeList {
return myName;
}
@Override
public void setName(@NotNull String name) {
public void setNameImpl(@NotNull String name) {
myName = validateName(name);
}
@@ -96,8 +95,7 @@ public class LocalChangeListImpl extends LocalChangeList {
return myComment;
}
@Override
public void setComment(@Nullable String comment) {
public void setCommentImpl(@Nullable String comment) {
myComment = comment != null ? comment : "";
}
@@ -115,8 +113,7 @@ public class LocalChangeListImpl extends LocalChangeList {
return myIsReadOnly;
}
@Override
public void setReadOnly(final boolean isReadOnly) {
public void setReadOnlyImpl(final boolean isReadOnly) {
myIsReadOnly = isReadOnly;
}
@@ -166,4 +163,20 @@ public class LocalChangeListImpl extends LocalChangeList {
public LocalChangeListImpl copy() {
return new LocalChangeListImpl(this);
}
@Override
public void setName(@NotNull String name) {
ChangeListManager.getInstance(myProject).editName(myName, name);
}
@Override
public void setComment(@Nullable String comment) {
ChangeListManager.getInstance(myProject).editComment(myName, comment);
}
@Override
public void setReadOnly(boolean isReadOnly) {
ChangeListManager.getInstance(myProject).setReadOnly(myName, isReadOnly);
}
}
@@ -182,7 +182,7 @@ public class ChangeListChooserPanel extends JPanel {
}
else {
//update description if changed
localChangeList.setComment(myListPanel.getDescription());
manager.editComment(changeListName, myListPanel.getDescription());
}
rememberSettings(project, localChangeList.isDefault(), myListPanel.getMakeActiveCheckBox().isSelected());
if (myListPanel.getMakeActiveCheckBox().isSelected()) {
@@ -86,7 +86,7 @@ public class HgTestChangeListManager {
}
final LocalChangeList list = peer.getDefaultChangeList();
assertNotNull(list);
list.setComment("A comment to a commit");
peer.editComment(list.getName(), "A comment to a commit");
UIUtil.invokeAndWaitIfNeeded((Runnable)() -> Assert.assertTrue(peer.commitChangesSynchronouslyWithResult(list, changes)));
ensureUpToDate();
}