Skip to content

Commit 6546d6e

Browse files
authored
Merge pull request #2079 from Haehnchen/feature/doctrine-performance
fix Doctrine yaml index false positive file detection
2 parents 2c7e7d6 + 9071524 commit 6546d6e

File tree

4 files changed

+177
-25
lines changed

4 files changed

+177
-25
lines changed

src/main/java/fr/adrienbrault/idea/symfony2plugin/doctrine/DoctrineUtil.java

Lines changed: 54 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@
88
import com.intellij.psi.xml.XmlAttribute;
99
import com.intellij.psi.xml.XmlFile;
1010
import com.intellij.psi.xml.XmlTag;
11+
import com.jetbrains.php.config.PhpLanguageLevel;
1112
import com.jetbrains.php.lang.documentation.phpdoc.parser.PhpDocElementTypes;
1213
import com.jetbrains.php.lang.documentation.phpdoc.psi.PhpDocComment;
1314
import com.jetbrains.php.lang.documentation.phpdoc.psi.tags.PhpDocTag;
1415
import com.jetbrains.php.lang.psi.PhpFile;
1516
import com.jetbrains.php.lang.psi.elements.PhpClass;
1617
import com.jetbrains.php.lang.psi.elements.PhpPsiElement;
18+
import com.jetbrains.php.refactoring.PhpNameUtil;
1719
import de.espend.idea.php.annotation.util.AnnotationUtil;
1820
import fr.adrienbrault.idea.symfony2plugin.stubs.indexes.visitor.AnnotationElementWalkingVisitor;
1921
import fr.adrienbrault.idea.symfony2plugin.stubs.indexes.visitor.AttributeElementWalkingVisitor;
@@ -192,7 +194,11 @@ private static Collection<Pair<String, String>> getClassRepositoryPair(@NotNull
192194
// we are indexing all yaml files for prefilter on path,
193195
// if false if check on parse
194196
String name = yamlFile.getName().toLowerCase();
195-
boolean iAmMetadataFile = name.contains(".odm.") || name.contains(".orm.") || name.contains(".mongodb.") || name.contains(".couchdb.");
197+
boolean iAmMetadataFile = name.contains(".odm.")
198+
|| name.contains(".orm.")
199+
|| name.contains(".dcm.")
200+
|| name.contains(".mongodb.")
201+
|| name.contains(".couchdb.");
196202

197203
YAMLDocument yamlDocument = PsiTreeUtil.getChildOfType(yamlFile, YAMLDocument.class);
198204
if(yamlDocument == null) {
@@ -205,39 +211,64 @@ private static Collection<Pair<String, String>> getClassRepositoryPair(@NotNull
205211
}
206212

207213
Collection<Pair<String, String>> pairs = new ArrayList<>();
208-
209214
for (YAMLKeyValue yamlKey : ((YAMLMapping) topLevelValue).getKeyValues()) {
210215
String keyText = yamlKey.getKeyText();
211-
if(StringUtils.isBlank(keyText)) {
216+
if (StringUtils.isBlank(keyText) || !PhpNameUtil.isValidNamespaceFullName(keyText, true, PhpLanguageLevel.current(yamlFile.getProject()))) {
212217
continue;
213218
}
214219

215-
String repositoryClassValue = YamlHelper.getYamlKeyValueAsString(yamlKey, "repositoryClass");
220+
boolean isValidEntry = iAmMetadataFile;
221+
222+
if (!isValidEntry) {
223+
@Nullable String type = YamlHelper.getYamlKeyValueAsString(yamlKey, "type");
224+
if ("entity".equalsIgnoreCase(type) || "embeddable".equalsIgnoreCase(type) || "mappedSuperclass".equalsIgnoreCase(type)) {
225+
isValidEntry = YamlHelper.getYamlKeyValue(yamlKey, "fields") != null
226+
|| YamlHelper.getYamlKeyValue(yamlKey, "repositoryClass") != null
227+
|| YamlHelper.getYamlKeyValue(yamlKey, "id") != null
228+
|| YamlHelper.getYamlKeyValue(yamlKey, "embedded") != null
229+
|| YamlHelper.getYamlKeyValue(yamlKey, "associationOverride") != null
230+
|| YamlHelper.getYamlKeyValue(yamlKey, "manyToOne") != null
231+
|| YamlHelper.getYamlKeyValue(yamlKey, "manyToMany") != null
232+
|| YamlHelper.getYamlKeyValue(yamlKey, "oneToMany") != null
233+
|| YamlHelper.getYamlKeyValue(yamlKey, "oneToOne") != null;
234+
}
216235

217-
// check blank condition
218-
String repositoryClass = null;
219-
if(StringUtils.isNotBlank(repositoryClassValue)) {
220-
repositoryClass = repositoryClassValue;
221-
}
236+
if (!isValidEntry) {
237+
@Nullable String db = YamlHelper.getYamlKeyValueAsString(yamlKey, "db");
238+
@Nullable String typeOdm = YamlHelper.getYamlKeyValueAsString(yamlKey, "type");
239+
if ("documents".equalsIgnoreCase(db) || "embeddable".equalsIgnoreCase(db) || "mappedSuperclass".equalsIgnoreCase(db) || "document".equalsIgnoreCase(typeOdm)|| "embeddedDocument".equalsIgnoreCase(typeOdm)) {
240+
isValidEntry = YamlHelper.getYamlKeyValue(yamlKey, "fields") != null
241+
|| YamlHelper.getYamlKeyValue(yamlKey, "embedOne") != null
242+
|| YamlHelper.getYamlKeyValue(yamlKey, "embedMany") != null
243+
|| YamlHelper.getYamlKeyValue(yamlKey, "referenceOne") != null
244+
|| YamlHelper.getYamlKeyValue(yamlKey, "referenceMany") != null
245+
|| YamlHelper.getYamlKeyValue(yamlKey, "collection") != null
246+
|| YamlHelper.getYamlKeyValue(yamlKey, "db") != null;
247+
}
248+
}
222249

223-
// fine repositoryClass exists a valid metadata file
224-
if(!iAmMetadataFile && repositoryClass != null) {
225-
iAmMetadataFile = true;
250+
if (!isValidEntry) {
251+
@Nullable String db = YamlHelper.getYamlKeyValueAsString(yamlKey, "db");
252+
if ("documents".equalsIgnoreCase(db) || "embeddable".equalsIgnoreCase(db) || "mappedSuperclass".equalsIgnoreCase(db)) {
253+
isValidEntry = YamlHelper.getYamlKeyValue(yamlKey, "fields") != null
254+
|| YamlHelper.getYamlKeyValue(yamlKey, "embedOne") != null
255+
|| YamlHelper.getYamlKeyValue(yamlKey, "embedMany") != null
256+
|| YamlHelper.getYamlKeyValue(yamlKey, "referenceOne") != null
257+
|| YamlHelper.getYamlKeyValue(yamlKey, "referenceMany") != null;
258+
}
259+
}
226260
}
227261

228-
// currently not valid metadata file find valid keys
229-
// else we are not allowed to store values
230-
if(!iAmMetadataFile) {
231-
Set<String> keySet = YamlHelper.getKeySet(yamlKey);
232-
if(keySet == null) {
233-
continue;
234-
}
262+
if (!isValidEntry) {
263+
continue;
264+
}
235265

236-
if(!(keySet.contains("fields") || keySet.contains("id") || keySet.contains("collection") || keySet.contains("db") || keySet.contains("indexes"))) {
237-
continue;
238-
}
266+
String repositoryClassValue = YamlHelper.getYamlKeyValueAsString(yamlKey, "repositoryClass");
239267

240-
iAmMetadataFile = true;
268+
// check blank condition
269+
String repositoryClass = null;
270+
if(StringUtils.isNotBlank(repositoryClassValue)) {
271+
repositoryClass = repositoryClassValue;
241272
}
242273

243274
pairs.add(Pair.create(keyText, repositoryClass));

src/main/java/fr/adrienbrault/idea/symfony2plugin/stubs/indexes/DoctrineMetadataFileStubIndex.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public boolean dependsOnFileContent() {
9696

9797
@Override
9898
public int getVersion() {
99-
return 2;
99+
return 3;
100100
}
101101

102102
public static boolean isValidForIndex(FileContent inputData, PsiFile psiFile) {

src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/doctrine/DoctrineUtilTest.java

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@
66
import com.jetbrains.php.lang.psi.PhpPsiElementFactory;
77
import fr.adrienbrault.idea.symfony2plugin.doctrine.DoctrineUtil;
88
import fr.adrienbrault.idea.symfony2plugin.tests.SymfonyLightCodeInsightFixtureTestCase;
9+
import org.jetbrains.yaml.YAMLFileType;
10+
import org.jetbrains.yaml.psi.YAMLFile;
911

1012
import java.util.Collection;
13+
import java.util.Objects;
1114

1215
/**
1316
* @author Daniel Espendiller <daniel@espendiller.net>
@@ -165,4 +168,115 @@ public void testGetClassRepositoryPairForClassConstanta() {
165168
Pair<String, String> white = classRepositoryPair.stream().filter(stringStringPair -> "Foo\\White".equals(stringStringPair.getFirst())).findFirst().get();
166169
assertEquals("Foo\\Foobar", white.getSecond());
167170
}
171+
172+
public void testGetClassRepositoryPairForClassConstantForYaml() {
173+
YAMLFile yamlFile = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "Doctrine\\Tests\\ORM\\Mapping\\User:\n" +
174+
" type: entity\n" +
175+
" fields: {}"
176+
);
177+
178+
assertTrue(Objects.requireNonNull(DoctrineUtil.getClassRepositoryPair(yamlFile)).stream().anyMatch(
179+
stringStringPair -> "Doctrine\\Tests\\ORM\\Mapping\\User".equals(stringStringPair.getFirst())
180+
));
181+
182+
YAMLFile yamlFile2 = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "Doctrine\\Tests\\ORM\\Mapping\\User:\n" +
183+
" type: entity2\n"
184+
);
185+
186+
assertNull(DoctrineUtil.getClassRepositoryPair(yamlFile2));
187+
188+
YAMLFile yamlFile3 = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "User:\n" +
189+
" type: entity\n" +
190+
" embedded:\n" +
191+
" address:\n" +
192+
" class: Address"
193+
);
194+
195+
assertTrue(Objects.requireNonNull(DoctrineUtil.getClassRepositoryPair(yamlFile3)).stream().anyMatch(
196+
stringStringPair -> "User".equals(stringStringPair.getFirst())
197+
));
198+
199+
YAMLFile yamlFile4 = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "MyProject\\Model\\Admin:\n" +
200+
" type: entity\n" +
201+
" associationOverride:\n" +
202+
" address:\n" +
203+
" joinColumn:\n" +
204+
" adminaddress_id:\n" +
205+
" name: adminaddress_id\n" +
206+
" referencedColumnName: id"
207+
);
208+
209+
assertTrue(Objects.requireNonNull(DoctrineUtil.getClassRepositoryPair(yamlFile4)).stream().anyMatch(
210+
stringStringPair -> "MyProject\\Model\\Admin".equals(stringStringPair.getFirst())
211+
));
212+
213+
YAMLFile yamlFile5 = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "MyProject\\Model\\User2:\n" +
214+
" type: mappedSuperclass\n" +
215+
" # other fields mapping\n" +
216+
" manyToOne:\n" +
217+
" address:\n" +
218+
" targetEntity: Address\n"
219+
);
220+
221+
assertTrue(Objects.requireNonNull(DoctrineUtil.getClassRepositoryPair(yamlFile5)).stream().anyMatch(
222+
stringStringPair -> "MyProject\\Model\\User2".equals(stringStringPair.getFirst())
223+
));
224+
225+
YAMLFile yamlFile6 = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "\\Doctrine\\Foobar:\n" +
226+
" type: entity\n" +
227+
" fields: {}"
228+
);
229+
230+
assertTrue(Objects.requireNonNull(DoctrineUtil.getClassRepositoryPair(yamlFile6)).stream().anyMatch(
231+
stringStringPair -> "\\Doctrine\\Foobar".equals(stringStringPair.getFirst())
232+
));
233+
234+
YAMLFile yamlFile7 = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "'\\Doctrine\\Foobar':\n" +
235+
" type: entity\n" +
236+
" fields: {}"
237+
);
238+
239+
assertTrue(Objects.requireNonNull(DoctrineUtil.getClassRepositoryPair(yamlFile7)).stream().anyMatch(
240+
stringStringPair -> "\\Doctrine\\Foobar".equals(stringStringPair.getFirst())
241+
));
242+
}
243+
244+
public void testGetClassRepositoryPairForClassConstantForYamlNoMatch() {
245+
YAMLFile yamlFile = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "Foobar:\n" +
246+
" id: foobar\n" +
247+
" type: foo\n"
248+
);
249+
250+
assertNull(DoctrineUtil.getClassRepositoryPair(yamlFile));
251+
252+
YAMLFile yamlFile2 = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "Doctrine\\Tes-ts\\ORM\\Mapping\\User:\n" +
253+
" type: entity\n" +
254+
" fields: {}"
255+
);
256+
257+
assertNull(DoctrineUtil.getClassRepositoryPair(yamlFile2));
258+
}
259+
260+
public void testGetClassRepositoryPairForClassConstantForYamlForOdm() {
261+
YAMLFile yamlFile = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "Documents\\User:\n" +
262+
" db: documents\n" +
263+
" collection: user\n" +
264+
" fields: {}"
265+
);
266+
267+
assertTrue(Objects.requireNonNull(DoctrineUtil.getClassRepositoryPair(yamlFile)).stream().anyMatch(
268+
stringStringPair -> "Documents\\User".equals(stringStringPair.getFirst())
269+
));
270+
271+
YAMLFile yamlFile2 = (YAMLFile) myFixture.configureByText(YAMLFileType.YML, "User:\n" +
272+
" type: document\n" +
273+
" embedOne:\n" +
274+
" address:\n" +
275+
" targetDocument: Address"
276+
);
277+
278+
assertTrue(Objects.requireNonNull(DoctrineUtil.getClassRepositoryPair(yamlFile2)).stream().anyMatch(
279+
stringStringPair -> "User".equals(stringStringPair.getFirst())
280+
));
281+
}
168282
}

src/test/java/fr/adrienbrault/idea/symfony2plugin/tests/stubs/indexes/DoctrineMetadataFileStubIndexTest.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@ public void setUp() throws Exception {
3131
myFixture.configureByText("doctrine.yml", "" +
3232
"Documents\\Yml\\OdmUser:\n" +
3333
" db: documents\n" +
34+
" collection: ~" +
3435
"\n" +
3536
"Documents\\Yml\\OrmUser:\n" +
37+
" type: entity\n" +
3638
" repositoryClass: Documents\\Yml\\OrmUserRepository"
3739
);
3840

@@ -97,33 +99,38 @@ public void testYamlMetadataValidByFilename() {
9799
public void testYamlMetadataValidByStructure() {
98100
myFixture.configureByText("foo.yml", "" +
99101
"FooFields:\n" +
102+
" type: entity\n" +
100103
" fields: ~"
101104
);
102105
assertIndexContains(DoctrineMetadataFileStubIndex.KEY, "FooFields");
103106

104107
myFixture.configureByText("foo.yml", "" +
105108
"FooId:\n" +
109+
" type: entity\n" +
106110
" id: ~"
107111
);
108112
assertIndexContains(DoctrineMetadataFileStubIndex.KEY, "FooId");
109113

110114
myFixture.configureByText("foo.yml", "" +
111115
"FooCollection:\n" +
116+
" type: document\n" +
112117
" collection: ~"
113118
);
114119
assertIndexContains(DoctrineMetadataFileStubIndex.KEY, "FooCollection");
115120

116121
myFixture.configureByText("foo.yml", "" +
117122
"FooDb:\n" +
123+
" type: document\n" +
118124
" db: ~"
119125
);
120126
assertIndexContains(DoctrineMetadataFileStubIndex.KEY, "FooDb");
121127

122128
myFixture.configureByText("foo.yml", "" +
123129
"FooIndexes:\n" +
130+
" type: entity\n" +
124131
" indexes: ~"
125132
);
126-
assertIndexContains(DoctrineMetadataFileStubIndex.KEY, "FooIndexes");
133+
assertIndexNotContains(DoctrineMetadataFileStubIndex.KEY, "FooIndexes");
127134
}
128135

129136
/**

0 commit comments

Comments
 (0)