use correct serialization implementation in the copyProfile – otherwise when modifiable model of scheme is created, lazy loaded element data is not used (and as result, locked attribute is not correctly set)

testPreserveCompatibility is removed because we not DO NOT TOUCH files unless it is really modified In production we "do not preserve compatibility" (flag) since spring 2016
This commit is contained in:
Vladimir Krivosheev
2016-10-16 18:59:52 +02:00
parent eedb2ad320
commit 361fdda465
13 changed files with 81 additions and 121 deletions
@@ -27,7 +27,6 @@ import com.intellij.openapi.util.JDOMUtil;
import com.intellij.openapi.util.WriteExternalException;
import com.intellij.profile.Profile;
import com.intellij.profile.codeInspection.InspectionProfileManager;
import com.intellij.profile.codeInspection.InspectionProjectProfileManager;
import com.intellij.profile.codeInspection.ProjectInspectionProfileManager;
import com.intellij.profile.codeInspection.ui.header.InspectionToolsConfigurable;
import com.intellij.psi.PsiModifier;
@@ -43,8 +42,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import static com.intellij.profile.ProfileEx.serializeProfile;
import static com.intellij.testFramework.PlatformTestUtil.assertElementsEqual;
import static com.intellij.testFramework.Assertions.assertThat;
/**
* @author Anna.Kozlova
@@ -80,7 +78,7 @@ public class InspectionProfileTest extends LightIdeaTestCase {
profile.readExternal(element);
final ModifiableModel model = profile.getModifiableModel();
model.commit();
assertElementsEqual(element, serializeProfile(profile));
assertThat(profile.writeScheme()).isEqualTo(element);
}
private static InspectionProfileImpl createProfile() {
@@ -150,7 +148,7 @@ public class InspectionProfileTest extends LightIdeaTestCase {
ModifiableModel model = profile.getModifiableModel();
model.commit();
assertElementsEqual(loadProfile(), serializeProfile(profile));
assertThat(profile.writeScheme()).isEqualTo(loadProfile());
}
private static Element loadProfile() throws IOException, JDOMException {
@@ -206,7 +204,7 @@ public class InspectionProfileTest extends LightIdeaTestCase {
profile.readExternal(element);
final ModifiableModel model = profile.getModifiableModel();
model.commit();
assertElementsEqual(element, serializeProfile(profile));
assertThat(profile.writeScheme()).isEqualTo(element);
}
public void testMergeUnusedDeclarationAndUnusedSymbol() throws Exception {
@@ -218,7 +216,7 @@ public class InspectionProfileTest extends LightIdeaTestCase {
profile.readExternal(element);
ModifiableModel model = profile.getModifiableModel();
model.commit();
assertElementsEqual(element, serializeProfile(profile));
assertThat(profile.writeScheme()).isEqualTo(element);
//settings to merge
@@ -290,7 +288,7 @@ public class InspectionProfileTest extends LightIdeaTestCase {
"</profile>";
assertEquals(mergedText, serialize(profile));
Element toImportElement = serializeProfile(profile);
Element toImportElement = profile.writeScheme();
final InspectionProfileImpl importedProfile =
InspectionToolsConfigurable.importInspectionProfile(toImportElement, InspectionProfileManager.getInstance(), getProject(), null);
@@ -300,9 +298,9 @@ public class InspectionProfileTest extends LightIdeaTestCase {
profile.readExternal(mergedElement);
model = profile.getModifiableModel();
model.commit();
assertElementsEqual(mergedElement, serializeProfile(profile));
assertThat(profile.writeScheme()).isEqualTo(mergedElement);
assertElementsEqual(mergedElement, serializeProfile(importedProfile));
assertThat(importedProfile.writeScheme()).isEqualTo(mergedElement);
}
public void testStoredMemberVisibility() throws Exception {
@@ -510,7 +508,7 @@ public class InspectionProfileTest extends LightIdeaTestCase {
"</profile>",
serialize(profile));
Element element = serializeProfile(profile);
Element element = profile.writeScheme();
list.add(createTool("bar", true));
list.add(createTool("disabled", false));
@@ -537,7 +535,7 @@ public class InspectionProfileTest extends LightIdeaTestCase {
}
private static String serialize(InspectionProfileImpl profile) throws WriteExternalException {
return JDOMUtil.writeElement(serializeProfile(profile));
return JDOMUtil.writeElement(profile.writeScheme());
}
private static InspectionProfileImpl createProfile(@NotNull InspectionToolRegistrar registrar) {
@@ -596,7 +594,7 @@ public class InspectionProfileTest extends LightIdeaTestCase {
profile.enableTool(id, getProject());
}
assertEquals(0, countInitializedTools(profile));
serializeProfile(profile);
profile.writeScheme();
List<InspectionToolWrapper> initializedTools = getInitializedTools(profile);
if (initializedTools.size() > 0) {
for (InspectionToolWrapper initializedTool : initializedTools) {
@@ -619,20 +617,6 @@ public class InspectionProfileTest extends LightIdeaTestCase {
assertEquals(1, countInitializedTools(foo));
}
public void testPreserveCompatibility() throws Exception {
InspectionProfileImpl foo = new InspectionProfileImpl("foo", InspectionToolRegistrar.getInstance(), InspectionProjectProfileManager.getInstance(getProject()));
String test = "<profile version=\"1.0\" is_locked=\"false\">\n" +
" <option name=\"myName\" value=\"idea.default\" />\n" +
" <inspection_tool class=\"AbstractMethodCallInConstructor\" enabled=\"true\" level=\"WARNING\" enabled_by_default=\"true\" />\n" +
" <inspection_tool class=\"AssignmentToForLoopParameter\" enabled=\"true\" level=\"WARNING\" enabled_by_default=\"true\">\n" +
" <option name=\"m_checkForeachParameters\" value=\"false\" />\n" +
" </inspection_tool>\n" +
"</profile>";
foo.readExternal(JDOMUtil.loadDocument(test).getRootElement());
foo.initInspectionTools(getProject());
assertEquals(test, JDOMUtil.writeElement(serializeProfile(foo)));
}
public static int countInitializedTools(@NotNull Profile foo) {
return getInitializedTools((InspectionProfileImpl)foo).size();
}
@@ -41,7 +41,7 @@ class InspectionSchemeTest {
@Test fun loadSchemes() {
val schemeFile = fsRule.fs.getPath("inspection/Bar.xml")
val schemeData = """
<inspections profile_name="Bar" version="1.0">
<inspections version="1.0">
<option name="myName" value="Bar" />
<inspection_tool class="Since15" enabled="true" level="ERROR" enabled_by_default="true" />
"</inspections>""".trimIndent()
@@ -17,7 +17,6 @@ package com.intellij.profile;
import com.intellij.openapi.options.Scheme;
import com.intellij.util.xmlb.annotations.Transient;
import org.jdom.Element;
import org.jetbrains.annotations.NotNull;
/**
@@ -25,8 +24,6 @@ import org.jetbrains.annotations.NotNull;
* Date: 20-Nov-2005
*/
public interface Profile extends Comparable, Scheme {
void copyFrom(@NotNull Profile profile);
@Transient
boolean isProjectLevel();
@@ -42,8 +39,4 @@ public interface Profile extends Comparable, Scheme {
@NotNull
ProfileManager getProfileManager();
void readExternal(Element element);
void writeExternal(Element element);
}
@@ -26,13 +26,13 @@ import com.intellij.openapi.util.JDOMUtil;
import com.intellij.openapi.util.io.FileUtil;
import com.intellij.profile.codeInspection.InspectionProfileManager;
import com.intellij.profile.codeInspection.SeverityProvider;
import org.jdom.Document;
import org.jdom.Element;
import org.jdom.JDOMException;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
@@ -50,7 +50,6 @@ public class InspectionProfileConvertor {
@NonNls private static final String NAME_ATT = "name";
@NonNls private static final String VERSION_ATT = "version";
@NonNls private static final String PROFILE_NAME_ATT = "profile_name";
@NonNls private static final String OPTION_TAG = "option";
@NonNls private static final String DISPLAY_LEVEL_MAP_OPTION = "DISPLAY_LEVEL_MAP";
@NonNls protected static final String VALUE_ATT = "value";
@@ -110,11 +109,9 @@ public class InspectionProfileConvertor {
return;
}
try {
Document doc = JDOMUtil.loadDocument(files[0]);
Element root = doc.getRootElement();
Element root = JDOMUtil.load(files[0]);
if (root.getAttributeValue(VERSION_ATT) == null){
root.setAttribute(PROFILE_NAME_ATT, OLD_DEFAUL_PROFILE);
JDOMUtil.writeDocument(doc, new File(profileDirectory, OLD_DEFAUL_PROFILE + XML_EXTENSION), "\n");
JDOMUtil.writeParent(root, new FileOutputStream(new File(profileDirectory, OLD_DEFAUL_PROFILE + XML_EXTENSION)), "\n");
FileUtil.delete(files[0]);
}
}
@@ -33,10 +33,7 @@ import com.intellij.openapi.util.*;
import com.intellij.profile.ProfileEx;
import com.intellij.profile.ProfileManager;
import com.intellij.profile.codeInspection.InspectionProfileManager;
import com.intellij.profile.codeInspection.ProjectInspectionProfileManager;
import com.intellij.profile.codeInspection.ProjectInspectionProfileManagerKt;
import com.intellij.profile.codeInspection.SeverityProvider;
import com.intellij.project.ProjectKt;
import com.intellij.psi.PsiElement;
import com.intellij.psi.search.scope.packageSet.NamedScope;
import com.intellij.util.ArrayUtil;
@@ -281,26 +278,15 @@ public class InspectionProfileImpl extends ProfileEx implements ModifiableModel,
@NotNull
public Element writeScheme() {
if (myDataHolder != null) {
return myDataHolder.read();
}
Element element = new Element("profile");
Element result = isProjectLevel() ? element.setAttribute("version", "1.0") : element.setAttribute("profile_name", getName());
serializeInto(result, false);
if (isProjectLevel() && ProjectKt.isDirectoryBased(((ProjectInspectionProfileManager)myProfileManager).getProject())) {
return new Element("component").setAttribute("name", "InspectionProjectProfileManager").addContent(result);
}
return result;
return myDataHolder == null ? super.writeScheme() : myDataHolder.read();
}
@Override
public void serializeInto(@NotNull Element element, boolean preserveCompatibility) {
public void writeExternal(@NotNull Element element) {
// must be first - compatibility
element.setAttribute(VERSION_TAG, VALID_VERSION);
super.serializeInto(element, preserveCompatibility);
super.writeExternal(element);
synchronized (myLock) {
if (!myInitialized) {
@@ -857,24 +843,23 @@ public class InspectionProfileImpl extends ProfileEx implements ModifiableModel,
initInspectionTools(project);
for (Element scopeElement : scopes.getChildren(SCOPE)) {
final String profile = scopeElement.getAttributeValue(ProjectInspectionProfileManagerKt.PROFILE);
if (profile != null) {
final InspectionProfileImpl inspectionProfile = (InspectionProfileImpl)getProfileManager().getProfile(profile);
if (inspectionProfile != null) {
final NamedScope scope = getProfileManager().getScopesManager().getScope(scopeElement.getAttributeValue(NAME));
if (scope != null) {
for (InspectionToolWrapper toolWrapper : inspectionProfile.getInspectionTools(null)) {
final HighlightDisplayKey key = HighlightDisplayKey.find(toolWrapper.getShortName());
try {
InspectionToolWrapper toolWrapperCopy = copyToolSettings(toolWrapper);
HighlightDisplayLevel errorLevel = inspectionProfile.getErrorLevel(key, null, project);
getTools(toolWrapper.getShortName(), project).addTool(scope, toolWrapperCopy, inspectionProfile.isToolEnabled(key), errorLevel);
}
catch (Exception e) {
LOG.error(e);
}
}
}
final String profile = scopeElement.getAttributeValue(PROFILE);
InspectionProfileImpl inspectionProfile = profile == null ? null : (InspectionProfileImpl)getProfileManager().getProfile(profile);
NamedScope scope = inspectionProfile == null ? null : getProfileManager().getScopesManager().getScope(scopeElement.getAttributeValue(NAME));
if (scope == null) {
continue;
}
for (InspectionToolWrapper toolWrapper : inspectionProfile.getInspectionTools(null)) {
final HighlightDisplayKey key = HighlightDisplayKey.find(toolWrapper.getShortName());
try {
InspectionToolWrapper toolWrapperCopy = copyToolSettings(toolWrapper);
HighlightDisplayLevel errorLevel = inspectionProfile.getErrorLevel(key, null, project);
getTools(toolWrapper.getShortName(), project)
.addTool(scope, toolWrapperCopy, inspectionProfile.isToolEnabled(key), errorLevel);
}
catch (Exception e) {
LOG.error(e);
}
}
}
@@ -15,7 +15,8 @@
*/
package com.intellij.profile;
import com.intellij.profile.codeInspection.ProjectInspectionProfileManagerKt;
import com.intellij.profile.codeInspection.ProjectInspectionProfileManager;
import com.intellij.project.ProjectKt;
import com.intellij.util.xmlb.SmartSerializer;
import com.intellij.util.xmlb.annotations.OptionTag;
import com.intellij.util.xmlb.annotations.Transient;
@@ -29,6 +30,7 @@ import org.jetbrains.annotations.NotNull;
public abstract class ProfileEx implements Profile {
public static final String SCOPE = "scope";
public static final String NAME = "name";
public static final String PROFILE = "profile";
private final SmartSerializer mySerializer;
@@ -84,18 +86,12 @@ public abstract class ProfileEx implements Profile {
myProfileManager = profileManager;
}
@Override
public void readExternal(Element element) {
mySerializer.readExternal(this, element);
}
public void serializeInto(@NotNull Element element, boolean preserveCompatibility) {
mySerializer.writeExternal(this, element, preserveCompatibility);
}
@Override
public final void writeExternal(Element element) {
serializeInto(element, true);
public void writeExternal(@NotNull Element element) {
mySerializer.writeExternal(this, element, false);
}
public boolean equals(Object o) {
@@ -114,15 +110,21 @@ public abstract class ProfileEx implements Profile {
return 0;
}
@Override
public final void copyFrom(@NotNull Profile profile) {
readExternal(serializeProfile(profile));
public final void copyFrom(@NotNull ProfileEx profile) {
readExternal(profile.writeScheme());
}
@NotNull
public static Element serializeProfile(@NotNull Profile profile) {
Element result = new Element(ProjectInspectionProfileManagerKt.PROFILE);
profile.writeExternal(result);
return result;
public Element writeScheme() {
Element element = new Element(PROFILE);
if (isProjectLevel()) {
element.setAttribute("version", "1.0");
}
writeExternal(element);
if (isProjectLevel() && ProjectKt.isDirectoryBased(((ProjectInspectionProfileManager)myProfileManager).getProject())) {
return new Element("component").setAttribute("name", "InspectionProjectProfileManager").addContent(element);
}
return element;
}
}
@@ -33,6 +33,7 @@ import com.intellij.openapi.util.Disposer
import com.intellij.openapi.util.text.StringUtil
import com.intellij.packageDependencies.DependencyValidationManager
import com.intellij.profile.Profile
import com.intellij.profile.ProfileEx
import com.intellij.project.isDirectoryBased
import com.intellij.psi.search.scope.packageSet.NamedScopeManager
import com.intellij.psi.search.scope.packageSet.NamedScopesHolder
@@ -50,8 +51,6 @@ import org.jetbrains.concurrency.runAsync
import java.util.*
import java.util.function.Function
const val PROFILE = "profile"
private const val VERSION = "1.0"
private const val SCOPE = "scope"
private const val NAME = "name"
@@ -308,7 +307,7 @@ class ProjectInspectionProfileManager(val project: Project,
if (currentScheme == null) {
currentScheme = InspectionProfileImpl(PROJECT_DEFAULT_PROFILE_NAME, InspectionToolRegistrar.getInstance(), this,
InspectionProfileImpl.getDefaultProfile(), null)
currentScheme.copyFrom(applicationProfileManager.currentProfile)
currentScheme.copyFrom(applicationProfileManager.currentProfile as ProfileEx)
currentScheme.isProjectLevel = true
currentScheme.name = PROJECT_DEFAULT_PROFILE_NAME
schemeManager.addScheme(currentScheme)
@@ -233,11 +233,11 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
val bytes = URLUtil.openStream(url).readBytes()
lazyPreloadScheme(bytes, isUseOldFileNameSanitize) { name, parser ->
val attributeProvider = Function<String, String?> { parser.getAttributeValue(null, it) }
val schemeName = name ?: (processor as LazySchemeProcessor).getName(attributeProvider)
val fileName = PathUtilRt.getFileName(url.path)
val extension = getFileExtension(fileName, true)
val externalInfo = ExternalInfo(fileName.substring(0, fileName.length - extension.length), extension)
val schemeName = name ?: (processor as LazySchemeProcessor).getName(attributeProvider, externalInfo.fileNameWithoutExtension)
externalInfo.schemeName = schemeName
val scheme = (processor as LazySchemeProcessor).createScheme(SchemeDataHolderImpl(bytes, externalInfo), schemeName, attributeProvider, true)
@@ -316,7 +316,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
processPendingCurrentSchemeName(scheme)
}
messageBus?.let { it.connect().subscribe(VirtualFileManager.VFS_CHANGES, SchemeFileTracker()) }
messageBus?.connect()?.subscribe(VirtualFileManager.VFS_CHANGES, SchemeFileTracker())
return schemes.subList(newSchemesOffset, schemes.size)
}
@@ -438,7 +438,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
val bytes = input.readBytes()
lazyPreloadScheme(bytes, isUseOldFileNameSanitize) { name, parser ->
val attributeProvider = Function<String, String?> { parser.getAttributeValue(null, it) }
val schemeName = name ?: processor.getName(attributeProvider)
val schemeName = name ?: processor.getName(attributeProvider, fileNameWithoutExtension)
if (!checkExisting(schemeName)) {
return null
}
@@ -573,7 +573,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
var externalInfo: ExternalInfo? = schemeToInfo.get(scheme)
val currentFileNameWithoutExtension = externalInfo?.fileNameWithoutExtension
val parent = processor.writeScheme(scheme)
val element = if (parent is Element) parent else (parent as Document).detachRootElement()
val element = parent as? Element ?: (parent as Document).detachRootElement()
if (element.isEmpty()) {
externalInfo?.scheduleDelete()
return
@@ -685,7 +685,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
val bundledScheme = readOnlyExternalizableSchemes.get(scheme.name)
if (bundledScheme == null) {
if ((processor as? LazySchemeProcessor)?.let { it.isSchemeEqualToBundled(scheme) } ?: false) {
if ((processor as? LazySchemeProcessor)?.isSchemeEqualToBundled(scheme) ?: false) {
externalInfo?.scheduleDelete()
return true
}
@@ -875,9 +875,7 @@ class SchemeManagerImpl<T : Scheme, MUTABLE_SCHEME : T>(val fileSpec: String,
private fun collectExistingNames(schemes: Collection<T>): Collection<String> {
val result = THashSet<String>(schemes.size)
for (scheme in schemes) {
result.add(scheme.name)
}
schemes.mapTo(result) { it.name }
return result
}
@@ -22,7 +22,6 @@ import com.intellij.codeInsight.daemon.impl.HighlightInfoType;
import com.intellij.codeInsight.daemon.impl.SeveritiesProvider;
import com.intellij.codeInsight.daemon.impl.SeverityRegistrar;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightingSettingsPerFile;
import com.intellij.codeInspection.InspectionProfile;
import com.intellij.codeInspection.InspectionsBundle;
import com.intellij.configurationStore.BundledSchemeEP;
import com.intellij.configurationStore.SchemeDataHolder;
@@ -93,8 +92,8 @@ public class ApplicationInspectionProfileManager extends BaseInspectionProfileMa
mySchemeManager = schemeManagerFactory.create(INSPECTION_DIR, new InspectionProfileProcessor() {
@NotNull
@Override
public String getName(@NotNull Function<String, String> attributeProvider) {
return "unnamed";
public String getName(@NotNull Function<String, String> attributeProvider, String fileNameWithoutExtension) {
return fileNameWithoutExtension;
}
@NotNull
@@ -102,11 +101,7 @@ public class ApplicationInspectionProfileManager extends BaseInspectionProfileMa
@NotNull String name,
@NotNull Function<String, String> attributeProvider,
boolean isBundled) {
InspectionProfileImpl profile = new InspectionProfileImpl(name, myRegistrar, ApplicationInspectionProfileManager.this, dataHolder);
if (isBundled) {
profile.lockProfile(true);
}
return profile;
return new InspectionProfileImpl(name, myRegistrar, ApplicationInspectionProfileManager.this, dataHolder);
}
@Override
@@ -169,12 +164,10 @@ public class ApplicationInspectionProfileManager extends BaseInspectionProfileMa
}
public void initProfiles() {
if (!myProfilesAreInitialized.compareAndSet(false, true)) {
if (!myProfilesAreInitialized.compareAndSet(false, true) || !LOAD_PROFILES) {
return;
}
if (!LOAD_PROFILES) return;
loadBundledSchemes();
mySchemeManager.loadSchemes();
createDefaultProfile();
@@ -270,12 +263,12 @@ public class ApplicationInspectionProfileManager extends BaseInspectionProfileMa
@NotNull
@Override
public InspectionProfile getCurrentProfile() {
public InspectionProfileImpl getCurrentProfile() {
initProfiles();
Profile current = mySchemeManager.getCurrentScheme();
InspectionProfileImpl current = mySchemeManager.getCurrentScheme();
if (current != null) {
return (InspectionProfile)current;
return current;
}
// use default as base, not random custom profile
@@ -108,7 +108,7 @@ public abstract class InspectionToolsConfigurable extends BaseConfigurable
}
@NotNull
private InspectionProfileImpl copyToNewProfile(ModifiableModel selectedProfile,
private InspectionProfileImpl copyToNewProfile(@NotNull InspectionProfileImpl selectedProfile,
@NotNull Project project,
boolean modifyName,
boolean modifyLevel) {
@@ -54,7 +54,7 @@ interface SchemeExtensionProvider {
}
abstract class LazySchemeProcessor<SCHEME : Scheme, MUTABLE_SCHEME : SCHEME>(private val nameAttribute: String = "name") : SchemeProcessor<SCHEME, MUTABLE_SCHEME>() {
open fun getName(attributeProvider: Function<String, String?>): String {
open fun getName(attributeProvider: Function<String, String?>, fileNameWithoutExtension: String): String {
return attributeProvider.apply(nameAttribute) ?: throw IllegalStateException("name is missed in the scheme data")
}
@@ -44,6 +44,15 @@ class JdomAssert(actual: Element?) : AbstractAssert<JdomAssert, Element?>(actual
return isEqualTo(file.readText())
}
fun isEqualTo(element: Element): JdomAssert {
isNotNull
if (!JDOMUtil.areElementsEqual(actual, element)) {
isEqualTo(JDOMUtil.writeElement(element))
}
return this
}
fun isEqualTo(expected: String): JdomAssert {
isNotNull
@@ -34,7 +34,7 @@ public class CopyrightProfile extends ProfileEx {
//read external
public CopyrightProfile() {
super("", new SmartSerializer());
this("");
}
public CopyrightProfile(String profileName) {