Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#1329 Исправлена ошибка в проверке manager-module-named-self-reference #1336

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

VAGoncharov
Copy link
Contributor

@VAGoncharov VAGoncharov commented Jun 12, 2023

Что сделано

  • Исправлена ошибка в проверке manager-module-named-self-reference: проверка срабатывала ложно, если в конфигурации есть несколько объектов метаданных с одинаковым наименованием.

Чек-лист

Общее:

  • ветка PR обновлена из master и нет конфликтов
  • Тесты-кейсы, JUnit тесты правильного и неправильного состояния
  • Измененные Вами исходники отформатированы в соответствии с конвенцией
  • Авто-аудит (SonarQube и CheckStyle) пройден, покрытие кода хорошее, ошибок нет, плохой код устранен
  • Добавлена запись в ИСТОРИЮ ИЗМЕНЕНИЯ, включаемая в пользовательскую документацию плагина

Если применимо:

  • Пользовательская документация на доп.инструменты написана (на русском)
  • Описание проверок - на двух языках

Закрываемые задачи

Closes #1329

@1C-Company @marmyshev прошу сделать аудит

…f-reference

Исправлена ошибка в проверке manager-module-named-self-reference:
проверка срабатывала ложно, если в конфигурации есть несколько объектов
метаданных с одинаковым наименованием.
…eature/1329-manager-module-named-self-reference-bug
feature/1329-manager-module-named-self-reference-bug
Copy link
Collaborator

@marmyshev marmyshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VAGoncharov извини что долго делали ревью.

Comment on lines +152 to +153
EObject eObject = objectDesc.getEObjectOrProxy();
if (!monitor.isCanceled() && eObject.equals(module.getOwner()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачитывать 2 объекта метаданных - только чтобы сравнить их полные имена - это очень дорого.
Предлагаю способ дешевле!

Comment on lines -114 to +176
return StringUtils.equals(bslQualifiedNameProvider.getFullyQualifiedName(module).getSegment(1),
source.getName());
IScope scope = scopeProvider.getScope(source, reference);
String fqn =
topObjectFqnGenerator.generateStandaloneObjectFqn(MANAGERS_FOR_MDOBJECT.get(reference), source.getName());
return scope.getSingleElement(qualifiedNameConverter.toQualifiedName(fqn));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут нет смысла генерить FQN для целевого объекта, а еще получать сам объект из скоупа - это дорогостоящее!

Достаточно проверить URI для текущего объекта (модуль не нужно получать выше!) с именем ссылки:

private boolean isReferenceExcessive(DynamicFeatureAccess source, , EReference reference)
{
URI uri = EcoreUtil.getURI(source).trimFragment(); // тут будет URI текущего модуля
return uri.segmentCount()==5 && source.getName() != null && source.getName().equals(uri.segment(3)) && reference.getName().equals(uri.segment(2));
}

reference.getName() - всегда английское, и имя каталога для типа менеджера - тоже всегда будет английским.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно так же второй способ - более тяжелый т.к. получает лишний объект, но выглядящий менее хакирующе:

private boolean isReferenceExcessive(DynamicFeatureAccess source, Module module, EReference reference)
{
EObject owner = module.getOwner();
return owner instanceof MdObject && source.getName() != null && source.getName().equals(((MdObject)owner).getName) && reference.getEReferenceType().equals(owner.eClass());
}

@@ -80,8 +122,7 @@ protected void check(Object object, ResultAcceptor resultAceptor, ICheckParamete
Expression featureAccessSource = ((DynamicFeatureAccess)object).getSource();
Module module = EcoreUtil2.getContainerOfType(featureAccessSource, Module.class);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Модуль не нужно получать если дальше не используется

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants