Хранение стандартных реквизитов + небольшие правки #523
Conversation
WalkthroughДобавлены стандартные реквизиты: новый value‑type StandardAttribute, десериализация/парсинг XML для стандартных реквизитов, описание StdAtrInfo, автоматическое наполнение StdAttributeFiller, кеш в AbstractReaderContext и изменения в MDReaderContext; обновлены читатели Designer/EDT и тесты под увеличившиеся коллекции. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant XML as XML (Designer/EDT)
participant Reader as Reader
participant Unm as Unmarshaller
participant Ctx as MDReaderContext
participant Filler as StdAttributeFiller
participant SA as StandardAttribute
XML->>Reader: встречает узлы standardAttributes
Reader->>Unm: делегирует чтение узлов
Unm->>Ctx: создаёт/настраивает MDReaderContext для атрибута
Unm->>SA: десериализует поля через StandardAttributeConverter
Note right of Unm: чтение полей по ожидаемым типам из контекста
Reader->>Ctx: вызывает build()
Ctx->>Filler: если realClass реализует AttributeOwner → fill(this)
Filler->>Ctx: создаёт/настраивает дочерние контексты, устанавливает nameRu, type, mdoReference
Ctx-->>Reader: возвращает собранную структуру с заполненными стандартными реквизитами
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
theshadowco
force-pushed
the
feature/stdAttr
branch
from
9d353f6 to
6d0cfbc
Compare
theshadowco
force-pushed
the
feature/stdAttr
branch
from
6d0cfbc to
338e9d3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (15)
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/Unmarshaller.java (1)
60-61: Проверьте ясность именования констант.Добавлены новые константы для обработки стандартных реквизитов. Однако,
TYPE_FIELDможет вызвать путаницу с уже существующей константойVALUE_TYPE_FIELD(строка 59). Рассмотрите возможность использования более конкретного имени, напримерELEMENT_TYPE_FIELDилиFORM_ELEMENT_TYPE_FIELD.src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/Unmarshaller.java (1)
61-66: StandardAttributes: проверьте корректность маппинга имени поля и отсутствие дублейСейчас для узла StandardAttribute устанавливается значение в поле "Attribute". Это выбивается из принятой схемы (обычно используем имя узла/коллекции) и может:
- положить элементы не в ту коллекцию;
- перетирать предыдущее значение при многократных вызовах сеттера;
- дублировать элементы вместе с наполнением через StdAttributeFiller.
Рекомендации:
- Убедиться, что у владельца действительно есть сеттер/коллекция с именем "Attribute" для стандартных реквизитов. Иначе используйте "StandardAttribute" (или "StandardAttributes") для консистентности с моделью/билдером.
- Рассмотреть унификацию через readItemNode для узла StandardAttributes, если fieldType("StandardAttribute") возвращает StandardAttribute.class.
- Проверьте отсутствия дублей при одновременном чтении из Designer и автодобавлении в MDReaderContext (StdAttributeFiller.fill).
Готов предложить правку после уточнения фактического имени свойства в модели.
Also applies to: 162-166
src/test/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationTest.java (2)
35-36: @slf4j в тесте не используетсяЕсли логгер не планируется, можно убрать аннотацию/импорт, чтобы не раздувать поверхность. Нит.
Also applies to: 44-44
86-87: Жёсткие константы размеров делают тесты хрупкимиПосле добавления стандартных реквизитов значения выросли, но такие ассёрты будут часто ломаться при эволюции фикстур. Рассмотрите:
- проверять нижнюю границу (isGreaterThan/Between);
- или вычислять ожидание как сумму по группам (как сделано ниже для children), чтобы тест валидировал инвариант, а не «магическое число».
Also applies to: 199-201, 236-238
src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java (2)
137-139: Мелкая стилистика ассерта на тип значенияМожно упростить: assertThat(attribute.getValueType()).contains(ValueTypes.get("CatalogRef.Пользователи")) — читается проще.
156-165: testSSLFixture сейчас почти пустойИмеет смысл добавить хотя бы базовые проверки (кол-во атрибутов/наличие конкретного StandardAttribute), чтобы тест приносил пользу, либо объединить с существующим сценарием SSL.
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDReaderContext.java (3)
105-112: Новый конструктор для StandardAttribute: проверьте область видимостиКонструктор нужен для динамического создания контекстов стандартных реквизитов, но «public» расширяет API. Если используется только внутри ридеров/filler’а, рассмотрите package‑private или javadoc с назначением.
142-145: StdAttributeFiller.fill в build(): добавьте идемпотентностьbuild() может вызываться повторно; без защиты наполнение стандартных реквизитов потенциально приведёт к дублям в childrenContexts и росту коллекций. Рекомендация — локальный флаг:
+ private volatile boolean stdAttrsFilled; ... - if (AttributeOwner.class.isAssignableFrom(realClass)) { - StdAttributeFiller.fill(this); - } + if (AttributeOwner.class.isAssignableFrom(realClass) && !stdAttrsFilled) { + StdAttributeFiller.fill(this); + stdAttrsFilled = true; + }Либо обеспечить идемпотентность внутри StdAttributeFiller (проверять наличие контекстов до добавления).
72-74: Публичный getter для childrenContexts раскрывает внутренностиЛучше предоставить целевой метод наподобие addChildContext(String, MDReaderContext) и скрыть саму Map, чтобы не нарушить инварианты и снизить связность.
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.java (2)
119-124: Кэш контекста: ок, но стоит слегка укрепить API
- Идея с case‑insensitive ключами и ConcurrentHashMap — удачна.
- Рекомендуется добавить null‑guard для key в getFromCache и перегрузку без default (например, Optional) для читаемости.
- setValue записывает в кэш даже для «служебных» полей (например, name/nameRu), что используется далее — это ок. Важно подтвердить, что TransformationUtils.setValue безошибочно игнорирует неизвестные поля (иначе вызовы setValue("nameRu")/setValue("name") могут падать).
Пример точечной доработки:
@SuppressWarnings("unchecked") - public <T> T getFromCache(String key, T defaultValue) { - return (T) cache.getOrDefault(key.toLowerCase(Locale.ROOT), defaultValue); - } + public <T> T getFromCache(String key, T defaultValue) { + var k = java.util.Objects.requireNonNull(key, "key"); + return (T) cache.getOrDefault(k.toLowerCase(Locale.ROOT), defaultValue); + } + + public <T> java.util.Optional<T> getFromCache(String key) { + var k = java.util.Objects.requireNonNull(key, "key"); + return java.util.Optional.ofNullable((T) cache.get(k.toLowerCase(Locale.ROOT))); + }И просьба подтвердить: TransformationUtils.setValue не бросает при неизвестном имени поля? Если да — можно оставить текущие вызовы setValue("name"/"nameRu"); если нет — лучше предварительно проверять fieldType != null.
Also applies to: 129-137, 146-149, 169-172
207-208: Детерминированная сортировка модулейСортировка по Module::toString может зависеть от реализации toString/локали. Лучше явный ключ: тип + URI.
- modules.sort(Comparator.comparing(Module::toString)); + modules.sort( + Comparator.comparing(Module::getModuleType) + .thenComparing(m -> m.getUri().toString()) + );src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/StandardAttributeConverter.java (1)
38-55: Упростить за счёт повторного использования общего UnmarshallerСейчас парсинг StandardAttribute реализован вручную. Можно переиспользовать общий путь (super.read), а атрибут name подхватить из XML‑атрибута до чтения:
@Override public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) { - var readerContext = new MDReaderContext(reader); - var attributeName = reader.getAttribute("name"); - readerContext.setName(attributeName); - readerContext.setValue("name", attributeName); - while (reader.hasMoreChildren()) { - reader.moveDown(); - var name = reader.getNodeName(); - var fieldClass = readerContext.fieldType(name); - if (fieldClass != null) { - var value = ExtendXStream.readValue(context, fieldClass); - readerContext.setValue(name, value); - } - reader.moveUp(); - } - return readerContext; + // читаем имя до прохода Unmarshaller + var attrName = reader.getAttribute("name"); + var ctx = (MDReaderContext) super.read(reader, context); + if (attrName != null) { + ctx.setName(attrName); + ctx.setValue("name", attrName); + } + return ctx; }Дополнительно просьба подтвердить: TransformationUtils.setValue безопасен при setValue("name") для StandardAttribute (такого поля нет в модели, используется только кэш). Если нет — оградить установку проверкой fieldType("name") или try/catch.
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java (2)
58-62: Громкость лога при отсутствии реестра стандартных реквизитовWARN для штатной ситуации шумит. Предлагаю DEBUG/INFO.
- if (stdAttributes.isEmpty()) { - LOGGER.warn("Для {} нет настроенных стандартных реквизитов", parentContext.getMdoType()); - } + if (stdAttributes.isEmpty()) { + LOGGER.debug("Для {} нет настроенных стандартных реквизитов", parentContext.getMdoType()); + }
87-96: Кейс‑инсенситивность кэша: использовать getFromCache вместо прямого map.getДля единообразия и устойчивости к регистру ключей лучше не обращаться к raw map.
- var nameRu = stdAttribute.getFromCache("nameRu", ""); + var nameRu = stdAttribute.getFromCache("nameRu", "");И аналогично в местах прямого доступа в StdAtrInfo (см. computeOwner).
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java (1)
159-172: Доступ к кэшу: избегать прямого get для устойчивости к региструСейчас используется parentContext.getCache().get("owners"). Кэш кейс‑инсенситивный, но напрямую это неочевидно. Лучше через getFromCache.
- var owners = parentContext.getCache().get("owners"); + var owners = parentContext.getFromCache("owners", null);С последующей проверкой на null. Это выровняет стиль с остальным кодом и снизит риск регистровых ошибок.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (60)
src/test/resources/fixtures/external/ТестоваяВнешняяОбработка.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/external/ТестоваяВнешняяОбработка_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/external/ТестовыйВнешнийОтчет.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/external/ТестовыйВнешнийОтчет_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/AccountingRegisters.РегистрБухгалтерии1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/AccumulationRegisters.РегистрНакопления1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/BusinessProcesses.БизнесПроцесс1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/BusinessProcesses.БизнесПроцесс1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CalculationRegisters.РегистрРасчета1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/CalculationRegisters.РегистрРасчета1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Catalogs.Справочник1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Catalogs.Справочник1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfAccounts.ПланСчетов1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfAccounts.ПланСчетов1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfCalculationTypes.ПланВидовРасчета1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfCharacteristicTypes.ПланВидовХарактеристик1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ChartsOfCharacteristicTypes.ПланВидовХарактеристик1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Documents.Документ1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Documents.Документ1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Enums.Перечисление1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Enums.Перечисление1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ExchangePlans.ПланОбмена1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ExchangePlans.ПланОбмена1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ExternalDataSources.ТекущаяСУБД.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/ExternalDataSources.ТекущаяСУБД_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/InformationRegisters.РегистрСведений1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/InformationRegisters.РегистрСведений1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Reports.Отчет1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Reports.Отчет1_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Tasks.Задача1.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_3_18/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_3_18/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_3_24/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_5_1/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_ext/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses_ext/Configuration_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/BusinessProcesses.Задание.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/BusinessProcesses.Задание_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Catalogs.ВерсииФайлов.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Catalogs.ВерсииФайлов_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Catalogs.Заметки.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Catalogs.Заметки_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/DataProcessors.ЗагрузкаКурсовВалют.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/DataProcessors.ЗагрузкаКурсовВалют_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Documents.Анкета.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Documents.Анкета_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Enums.СтатусыОбработчиковОбновления.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Enums.СтатусыОбработчиковОбновления_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/ExchangePlans.ОбновлениеИнформационнойБазы.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/ExchangePlans.ОбновлениеИнформационнойБазы_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/InformationRegisters.СклоненияПредставленийОбъектов.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/InformationRegisters.СклоненияПредставленийОбъектов_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/InformationRegisters.ЭлектронныеПодписи.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/InformationRegisters.ЭлектронныеПодписи_edt.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Tasks.ЗадачаИсполнителя.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/ssl_3_1/Tasks.ЗадачаИсполнителя_edt.jsonis excluded by!**/*.json
📒 Files selected for processing (22)
src/main/java/com/github/_1c_syntax/bsl/mdo/children/StandardAttribute.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/ReturnValueReuse.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.java(7 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDReaderContext.java(6 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/EnumConverter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MDChildConverter.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/StandardAttributeConverter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/Unmarshaller.java(4 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/Unmarshaller.java(5 hunks)src/test/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationTest.java(5 hunks)src/test/java/com/github/_1c_syntax/bsl/mdclasses/ExternalDataProcessorTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/mdclasses/ExternalReportTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/AccumulationRegisterTest.java(2 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/CalculationRegisterTest.java(2 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java(5 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccountsTest.java(2 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/InformationRegisterTest.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/smoke/RightTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/StandardAttributeConverter.java (3)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.java (1)
AbstractReaderContext(52-210)src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AbstractReadConverter.java (1)
AbstractReadConverter(32-38)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MDChildConverter.java (1)
DesignerConverter(45-87)
src/main/java/com/github/_1c_syntax/bsl/mdo/children/StandardAttribute.java (2)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDReaderContext.java (1)
EqualsAndHashCode(56-175)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java (1)
Value(39-224)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java (2)
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/Unmarshaller.java (1)
UtilityClass(44-224)src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/Unmarshaller.java (1)
UtilityClass(45-155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: QA
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (26)
src/test/java/com/github/_1c_syntax/bsl/mdo/CalculationRegisterTest.java (1)
24-24: Тест корректно обновлён под добавление стандартных реквизитов.Изменения правильно отражают добавление стандартных атрибутов:
- Общее количество атрибутов увеличилось с 2 до 14 (добавлено 12 стандартных)
- Количество дочерних элементов увеличилось с 3 до 15
- Метод
getAttributes()теперь возвращает 12 стандартных атрибутов (ранее возвращал пустую коллекцию)Проверка
allMatch(attribute -> attribute.getKind() == AttributeKind.STANDARD)корректно верифицирует, что все возвращаемые атрибуты имеют тип STANDARD.Пожалуйста, убедитесь, что ожидаемое количество стандартных атрибутов (12) соответствует реальной реализации для регистров расчёта в вашей версии платформы 1С.
Also applies to: 44-45, 48-50
src/test/java/com/github/_1c_syntax/bsl/mdo/AccumulationRegisterTest.java (3)
24-24: Импорт необходим и корректно используется.Импорт
AttributeKindтребуется для проверки типа атрибутов в строке 46.
44-46: Проверка стандартных атрибутов выглядит корректно.Утверждение проверяет, что все 5 атрибутов имеют тип
STANDARD, что соответствует целям PR по добавлению стандартных реквизитов. Убедитесь, что тестовые данные дляРегистрНакопления1действительно содержат именно 5 стандартных атрибутов и не предполагают пользовательских атрибутов.
52-52: Увеличение количества дочерних элементов согласуется с добавлением стандартных атрибутов.Изменение с 2 на 7 дочерних элементов математически корректно: 1 ресурс + 1 измерение + 5 стандартных атрибутов = 7 элементов. Это подтверждает, что стандартные атрибуты теперь включаются в коллекцию
getChildren().src/test/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccountsTest.java (2)
24-24: Импорт добавлен корректно.Импорт
AttributeKindнеобходим для новой проверки на строке 48, которая валидирует, что все атрибуты имеют вид STANDARD.
44-48: Семантика методов getAttributes() и getAllAttributes() подтверждена
getAttributes() используется исключительно в тестах, в продакшен-коде вызовов нет, изменение не нарушает обратную совместимость.src/test/java/com/github/_1c_syntax/bsl/mdclasses/ExternalDataProcessorTest.java (1)
62-62: Корректное обновление теста.Увеличение ожидаемого количества дочерних элементов с 6 до 7 соответствует добавлению стандартных реквизитов в объекты.
src/test/java/com/github/_1c_syntax/bsl/smoke/RightTest.java (1)
47-47: Корректное обновление теста.Увеличение ожидаемого количества классов, реализующих
AccessRightsOwner, с 43 до 44 соответствует добавлению классаStandardAttribute.src/test/java/com/github/_1c_syntax/bsl/mdclasses/ExternalReportTest.java (1)
66-66: Корректное обновление теста.Увеличение ожидаемого количества дочерних элементов с 9 до 10 соответствует добавлению стандартных реквизитов.
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/EnumConverter.java (1)
1-76: Только форматирование кода.Изменения в этом файле представляют собой исключительно переформатирование без изменения логики или поведения.
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MDChildConverter.java (2)
29-29: LGTM!Импорт класса
StandardAttributeнеобходим для использования в методеcanConvert.
69-69: Корректное исключение StandardAttribute из общего конвертера.Добавление исключения для
StandardAttributeследует существующему паттерну сObjectTemplate, что указывает на необходимость специальной обработки этого типа.src/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.java (2)
46-46: LGTM!Импорт класса
StandardAttributeнеобходим для регистрации XML-алиаса.
248-248: Корректная регистрация XML-алиаса для стандартных реквизитов.Регистрация алиаса
standardAttributes→StandardAttribute.classобеспечивает десериализацию стандартных реквизитов из формата EDT.src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java (2)
46-46: LGTM!Импорт класса
StandardAttributeнеобходим для регистрации XML-алиаса.
251-251: Корректная регистрация XML-алиаса для стандартных реквизитов.Регистрация алиаса
StandardAttribute→StandardAttribute.classобеспечивает десериализацию стандартных реквизитов из формата Designer.src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/Unmarshaller.java (4)
26-26: LGTM!Импорт класса
StandardAttributeнеобходим для обработки узлов стандартных реквизитов.
108-113: LGTM!Рефакторинг с использованием локальной переменной
valueи динамического определенияfieldClassулучшает читаемость и поддерживает обработку стандартных реквизитов.
125-125: Хорошее использование константы.Замена магической строки
"type"на константуTYPE_FIELDулучшает поддерживаемость кода.
89-92: Проверьте наличие поля или геттер/сеттер attributes в MDReaderContext или его супертипах — по результатам поиска в коде таких элементов не найдено, что может привести к ошибке десериализации XML-узла standardAttributes.src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/Unmarshaller.java (1)
167-169: Fallback к String через Objects.requireNonNullElse — удачное упрощениеХорошее улучшение читаемости и безопасности по null, поведение прежнее.
src/test/java/com/github/_1c_syntax/bsl/mdo/InformationRegisterTest.java (1)
37-38: Расширение кейсов CSV выглядит корректноДобавление не‑EDT варианта и нового регистра повышает покрытие. Без замечаний.
src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java (2)
53-56: Обновлённые размеры коллекций каталога соответствуют добавлению стандартных реквизитовРост размеров children/plainChildren/attributes/storageFields ожидаем. Ассерты выглядят консистентно с остальными проверками.
Убедитесь, что рост связан именно со стандартными реквизитами, а не с дублированием при чтении Designer + автозаполнении.
Also applies to: 61-63, 73-81, 104-121
140-154: Проверки StandardAttribute.PredefinedDataName — отличноХорошо, что проверены имя, локали, MDO‑ссылка, kind и тип значения (STRING). Это подтверждает корректную десериализацию и наполнение.
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDReaderContext.java (1)
125-129: Ленивая инициализация mdoReference — окИзбегает повторного пересоздания при повторных сборках. Без замечаний.
src/main/java/com/github/_1c_syntax/bsl/mdo/children/StandardAttribute.java (1)
48-55: Иммутабельная модель стандартного реквизита — выглядит корректно
- Семантика @Value/@builder, кастомный getValueType и possibleRights — ок.
- getDescription с fallback по синониму/полным именам — ожидаемо.
Also applies to: 157-166, 167-182
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java
Show resolved
Hide resolved
.../java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java
Show resolved
Hide resolved
.../java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java
Outdated
Show resolved
Hide resolved
theshadowco
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java (1)
85-85: TODO: реализовать удаление ненужных стандартных реквизитов.В коде оставлен комментарий о необходимости продумать удаление лишних стандартных реквизитов. Это может привести к тому, что в результате останутся атрибуты, которые не должны присутствовать для данного типа объекта.
Хотите, чтобы я создал задачу для отслеживания этой доработки?
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java (2)
131-156: Рассмотрите защиту от NumberFormatException.Использование
Integer.parseIntна значениях из кэша может вызватьNumberFormatExceptionпри некорректных данных. Хотя безопасные значения по умолчанию предоставлены черезgetFromCache, для большей надежности можно добавить обработку исключений или использовать более безопасные методы парсинга (например,NumberUtils.toIntиз Apache Commons).Пример с обработкой исключения:
private static ValueTypeDescription computeDescription(MDReaderContext parentContext) { - var descriptionLength = Integer.parseInt(parentContext.getFromCache("descriptionLength", "100")); + int descriptionLength; + try { + descriptionLength = Integer.parseInt(parentContext.getFromCache("descriptionLength", "100")); + } catch (NumberFormatException e) { + LOGGER.debug("Неверный формат descriptionLength, используется значение по умолчанию", e); + descriptionLength = 100; + } return ValueTypeDescription.createString(descriptionLength); }
174-223: TODO: завершить реализацию вычислений типов.Несколько методов вычисления типов (
computeRecorder,computeRecordType,computeRoutePoint,computeBusinessProcess,computeType,computeCalculationType) возвращаютEMPTYс комментариями TODO. Это приемлемо для начальной реализации, но требует дальнейшей доработки для полноценной функциональности.Хотите, чтобы я помог реализовать эти методы или создал задачи для отслеживания этих доработок?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.java(7 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/StandardAttributeConverter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/Unmarshaller.java(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java (2)
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/Unmarshaller.java (1)
UtilityClass(45-154)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/Unmarshaller.java (1)
UtilityClass(44-224)
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/StandardAttributeConverter.java (2)
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/AbstractReadConverter.java (1)
AbstractReadConverter(32-38)src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MDChildConverter.java (1)
DesignerConverter(45-87)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java (1)
src/main/java/com/github/_1c_syntax/bsl/mdo/children/StandardAttribute.java (1)
Value(48-183)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: QA
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (12)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/package-info.java (1)
1-25: LGTM!Файл документации пакета оформлен корректно с надлежащей лицензией и описанием.
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.java (4)
119-123: LGTM!Использование
ConcurrentHashMapобеспечивает потокобезопасный кэш для хранения атрибутов контекста.
146-150: LGTM!Кэширование значений с приведением ключей к нижнему регистру обеспечивает регистронезависимый доступ к данным.
169-172: LGTM!Метод
getFromCacheкорректно реализует паттерн безопасного получения значений с дефолтным значением.
207-207: Проверьте намеренность сортировки локального списка modules. Сортировка по Module::toString() изменяет исходный порядок элементов, сформированный moduleTypes. Убедитесь, что это не нарушит логику обработки и загрузки модулей.src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/Unmarshaller.java (2)
88-91: LGTM!Корректная обработка узла
standardAttributesс маппингом на коллекциюattributesчерез классStandardAttribute.
107-112: LGTM!Улучшенная логика чтения значений с обработкой случаев, когда
fieldClassравенnull— значение читается какString.src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/StandardAttributeConverter.java (1)
38-54: Проверьте: возврат контекста вместо построенного объекта.Метод
unmarshalвозвращаетMDReaderContextвместо вызоваreaderContext.build(). Убедитесь, что это соответствует общему паттерну десериализации в проекте, где построение объекта может происходить на более позднем этапе.Проверка показывает, что
AbstractReadConverter.read()также возвращаетMDReaderContext(см. relevant_code_snippets дляAbstractReadConverter.java), что подтверждает корректность этого подхода.src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAttributeFiller.java (3)
63-66: LGTM!Проблема с направлением проверки
isAssignableFromисправлена. Теперь корректно используетсяStandardAttribute.class.isAssignableFrom(context.getRealClass())для фильтрации существующих стандартных реквизитов.
277-284: LGTM!Потокобезопасность обеспечена использованием
Collections.synchronizedList(new ArrayList<>())при создании новых коллекций дочерних контекстов.
98-271: LGTM!Реестр стандартных реквизитов для различных типов метаданных заполнен корректно и покрывает основные типы объектов 1С (регистры, справочники, документы, планы счетов и т.д.).
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java (1)
48-97: LGTM!Статические дескрипторы стандартных реквизитов определены корректно с соответствующими типами данных и функциями вычисления.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/mdo/children/StandardAttribute.java (2)
76-80: Рассмотрите переименование поля type для ясности.Поле
typeс аннотацией@Getter(AccessLevel.NONE)может вводить в заблуждение, поскольку:
- Lombok не генерирует getter для этого поля
- Вместо этого реализован явный метод
getValueType()(строки 163-165)Рассмотрите переименование поля в
valueTypeилиvalueTypeDescriptionдля соответствия имени метода и улучшения читаемости.Применить этот diff:
@Default @Getter(AccessLevel.NONE) - ValueTypeDescription type = ValueTypeDescription.EMPTY; + ValueTypeDescription valueType = ValueTypeDescription.EMPTY; ... @Override public ValueTypeDescription getValueType() { - return type; + return valueType; }
61-64: Документируйте в JavaDoc, что у всех стандартных атрибутов одного владельца одинаковый uuid, и fullName служит единственным различителем в equals/hashCode.fullName подтверждён уникальным среди StdAtrInfo.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/github/_1c_syntax/bsl/mdo/children/StandardAttribute.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/github/_1c_syntax/bsl/mdo/children/StandardAttribute.java (2)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDReaderContext.java (1)
EqualsAndHashCode(56-175)src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.java (1)
Value(39-224)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: QA
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
🔇 Additional comments (2)
src/main/java/com/github/_1c_syntax/bsl/mdo/children/StandardAttribute.java (2)
45-54: LGTM: Корректное объявление класса.Использование Lombok-аннотаций (@value, @builder) и настройка toString/equals/hashCode на основе uuid и fullName выглядят правильно для value-type класса стандартного атрибута.
135-183: LGTM: Методы реализованы корректно.Все методы имеют корректную реализацию:
possibleRights()возвращает фиксированный список прав (VIEW, EDIT)getObjectBelonging()всегда возвращает OWN, что правильно для стандартных реквизитовgetIndexing()всегда возвращает DONT_INDEX, так как у стандартных реквизитов нет настроек индексированияgetDescription()имеет грамотную fallback-логику с приоритетом: synonym(code) → synonym(any) → fullName(code)
Описание
Связанные задачи
Closes: #114
Чеклист
Общие
gradlew precommit)Дополнительно