Простое объединение работает #534
Conversation
WalkthroughThis PR adds deep-merge support for Configuration+ConfigurationExtension (new MDMerger + MDClasses.createSolution), changes module-type maps to allow multiple URIs per ModuleType, extends TransformationUtils with builder-invocation helpers, and enables Lombok toBuilder generation across many MDO/MD classes. Tests for createSolution are added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MDClasses
participant MDCReader
participant MDMerger
participant Configuration
Client->>MDClasses: createSolution(sourcePath)
MDClasses->>MDCReader: read(sourcePath, MDCReadSettings)
MDCReader-->>MDClasses: List<Configuration|ConfigurationExtension>
alt no results
MDClasses-->>Client: Configuration.EMPTY (or empty MDClass)
else single Configuration
MDClasses-->>Client: configuration (as MDClass)
else single Extension
MDClasses->>MDMerger: merge(Configuration.EMPTY, extension)
MDMerger-->>MDClasses: merged Configuration
MDClasses-->>Client: merged MDClass
else configuration + extension(s)
MDClasses->>MDMerger: merge(configuration, extension)
MDMerger->>TransformationUtils: toBuilder/build (reflective)
MDMerger-->>MDClasses: merged Configuration
MDClasses-->>Client: merged MDClass
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Reason: many repetitive Lombok annotation changes (low effort) combined with non-trivial new MDMerger deep-merge logic and reflective utilities that require careful review and validation against callers and tests. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
@coderabbitai |
|
@theshadowco: Хорошо, я проведу ревью изменений в этом PR. ✅ Actions performedReview triggered.
|
|
🔥 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java (2)
135-179: New public API for reading merged configurations.The
createSolutionmethod provides a convenient way to read and merge configurations with their extensions. The logic correctly handles various scenarios:
- Empty directory → returns
Configuration.EMPTY- Single configuration/extension → returns as-is
- Configuration + extensions → merges them sequentially
The edge case handling at lines 159-166 is defensive but correct.
Consider adding JavaDoc examples to clarify the merge order and behavior when multiple extensions are present:
/** * Читает каталог проекта и * - возвращает объект MDClass, если содержится только один объект MDC * - возвращает объединенную конфигурацию с расширениями * - возвращает объединение расширений с пустой конфигурацией * * @param sourcePath Путь к каталогу исходников * @return Результат чтения решения + * @implNote Extensions are merged sequentially in the order they are discovered. + * Each extension is merged into the result of the previous merge. */
298-298: Remove trailing blank line.} -src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java (5)
143-152: AlignmergeModuleswith append semantics and dedup.Function currently returns
source + additional. Given the top-level caller now clears, keep union here; otherwise, if you keep non-clearing callers elsewhere, return only “new” modules.Option A (with clear at call site – keep union, but guard duplicates by URI/ref if available):
- var result = new ArrayList<>(source); - result.addAll(additional); - return Collections.unmodifiableList(result); + var result = new ArrayList<>(source.size() + additional.size()); + result.addAll(source); + // TODO: if Module exposes stable ref/URI, filter duplicates here + result.addAll(additional); + return Collections.unmodifiableList(result);Option B (without clear at call site – return only additional):
- if (additional.isEmpty()) { - return source; - } else if (source.isEmpty()) { - return additional; - } - var result = new ArrayList<>(source); - result.addAll(additional); - return Collections.unmodifiableList(result); + if (additional.isEmpty()) return Collections.emptyList(); + return additional;Confirm all usages of
mergeModules(top-level and incopyModuleOwner) to pick A or B consistently.
157-173: Reduce O(n²) membership checks in simple merges.Use a
Setfor refs; keep order viaLinkedHashSetif needed.- var refs = source.stream().map(MD::getMdoRef).toList(); - var result = List.copyOf(additional.stream().filter(md -> !refs.contains(md.getMdoRef())).toList()); + var refs = additional.isEmpty() + ? java.util.Collections.<Object>emptySet() + : additional.stream().map(MD::getMdoRef).collect(java.util.stream.Collectors.toSet()); + var result = source.isEmpty() + ? additional + : additional.stream() + .filter(md -> !refs.contains(md.getMdoRef())) // or compute refs from source depending on intent + .collect(java.util.stream.Collectors.toUnmodifiableList());Note: ensure the set is built from the correct side (source vs additional) per your intended “new items only” semantics.
190-230: Handle duplicate keys and improve performance inmergeMD.
Collectors.toMapwithout a merge function throws on duplicates.- List
containslookups are O(n²); use aSet.- var additionalRefs = additionalList.stream().collect(Collectors.toMap(MD::getMdoRef, md -> md)); + var additionalRefs = additionalList.stream() + .collect(Collectors.toMap(MD::getMdoRef, Function.identity(), (a, b) -> b)); @@ - var sourceRefs = sourceList.stream().map(MD::getMdoRef).toList(); - result.addAll(additionalList.stream().filter(md -> !sourceRefs.contains(md.getMdoRef())).toList()); + var sourceRefs = sourceList.stream().map(MD::getMdoRef).collect(Collectors.toSet()); + result.addAll(additionalList.stream() + .filter(md -> !sourceRefs.contains(md.getMdoRef())) + .collect(Collectors.toList()));
248-283: Children merge covers Forms/Templates/Commands only; other child types TODO.There’s a TODO to copy attributes and other child types; call out scope and timeline.
I can extend
copyChildrenOwnerand add tests for additional child kinds if you list the required types.
54-74: Add minimal tests for solution creation edge cases.
- Empty base CF + extension only (uuid/name, languages, modules).
- Non-empty CF + empty extension (no duplicates, lists intact).
- CF + extension with overlapping MDO refs (merge correctness).
- Modules duplication regression.
I can draft JUnit tests; confirm target test module/package.
Also applies to: 131-132
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (172)
src/test/resources/fixtures/external/ТестоваяВнешняяОбработка.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/external/ТестоваяВнешняяОбработка_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/external/ТестовыйВнешнийОтчет.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/external/ТестовыйВнешнийОтчет_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/BusinessProcesses.БизнесПроцесс1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/BusinessProcesses.БизнесПроцесс1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Catalogs.Справочник1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Catalogs.Справочник1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/ChartsOfAccounts.ПланСчетов1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/ChartsOfAccounts.ПланСчетов1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/ChartsOfCharacteristicTypes.ПланВидовХарактеристик1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/ChartsOfCharacteristicTypes.ПланВидовХарактеристик1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonCommands.ОбщаяКоманда1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonCommands.ОбщаяКоманда1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonForms.Форма.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonForms.Форма_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ГлобальныйОбщийМодуль.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ГлобальныйОбщийМодуль_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульВызовСервера.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульВызовСервера_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПовтИспВызов.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПовтИспВызов_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПовтИспСеанс.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПовтИспСеанс_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПолныйеПрава.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ОбщийМодульПолныйеПрава_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ПростойОбщийМодуль.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/CommonModules.ПростойОбщийМодуль_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Configuration.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Configuration_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Constants.Константа1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Constants.Константа1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/DocumentJournals.ЖурналДокументов1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/DocumentJournals.ЖурналДокументов1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Documents.Документ1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Documents.Документ1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Enums.Перечисление1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Enums.Перечисление1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/ExchangePlans.ПланОбмена1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/ExchangePlans.ПланОбмена1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/FilterCriteria.КритерийОтбора1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/FilterCriteria.КритерийОтбора1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/HTTPServices.HTTPСервис1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/HTTPServices.HTTPСервис1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/InformationRegisters.РегистрСведений1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/InformationRegisters.РегистрСведений1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Reports.Отчет1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Reports.Отчет1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Sequences.Последовательность1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/Sequences.Последовательность1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/WebServices.WebСервис1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses/WebServices.WebСервис1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_18/Bots.Бот1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_18/Bots.Бот1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_18/Configuration.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_18/Configuration_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_18/IntegrationServices.СервисИнтеграции1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_18/IntegrationServices.СервисИнтеграции1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_24/Configuration_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_27/WebSocketClient.WebSocketКлиент1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_3_27/WebSocketClient.WebSocketКлиент1_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_5_1/Configuration.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/mdclasses_unknown/Configuration_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/BusinessProcesses.Задание.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/BusinessProcesses.Задание_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Catalogs.ВерсииФайлов.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Catalogs.ВерсииФайлов_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Catalogs.Заметки.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Catalogs.Заметки_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/ChartsOfCharacteristicTypes.ДополнительныеРеквизитыИСведения_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/CommonCommands.ОтправитьПисьмо.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/CommonCommands.ОтправитьПисьмо_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/CommonForms.Вопрос.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/CommonForms.Вопрос_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/CommonModules.АвтономнаяРабота.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/CommonModules.АвтономнаяРабота_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/DataProcessors.ЗагрузкаКурсовВалют.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/DataProcessors.ЗагрузкаКурсовВалют_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/DocumentJournals.Взаимодействия.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/DocumentJournals.Взаимодействия_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Documents.Анкета.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Documents.Анкета_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Enums.СтатусыОбработчиковОбновления.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Enums.СтатусыОбработчиковОбновления_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/ExchangePlans.ОбновлениеИнформационнойБазы.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/ExchangePlans.ОбновлениеИнформационнойБазы_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/InformationRegisters.ЭлектронныеПодписи.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/InformationRegisters.ЭлектронныеПодписи_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/SettingsStorages.ХранилищеВариантовОтчетов.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/SettingsStorages.ХранилищеВариантовОтчетов_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Tasks.ЗадачаИсполнителя.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Tasks.ЗадачаИсполнителя_edt.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/WebServices.EnterpriseDataExchange_1_0_1_1.jsonis excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/WebServices.EnterpriseDataExchange_1_0_1_1_edt.jsonis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/.projectis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/DT-INF/PROJECT.PMFis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Catalogs/Справочник1/Справочник1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Catalogs/Справочник2/Forms/ФормаЭлемента/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Catalogs/Справочник2/Forms/ФормаЭлемента/Module.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Catalogs/Справочник2/Справочник2.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Configuration/CommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Configuration/Configuration.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Configuration/MainSectionCommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Configuration/ManagedApplicationModule.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Configuration/SessionModule.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Documents/Документ1/Документ1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Конфигурация/src/Enums/Перечисление1/Перечисление1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/.projectis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/DT-INF/PROJECT.PMFis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Catalogs/Расш1_Справочник3/Forms/ФормаЭлемента/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Catalogs/Расш1_Справочник3/Forms/ФормаЭлемента/Module.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Catalogs/Расш1_Справочник3/Расш1_Справочник3.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Configuration/CommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Configuration/Configuration.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Configuration/MainSectionCommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Constants/Расш1_Константа1/Расш1_Константа1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Documents/Документ1/Forms/Расш1_ФормаДокумента/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Documents/Документ1/Forms/Расш1_ФормаДокумента/Module.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Documents/Документ1/Документ1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Roles/Расш1_ОсновнаяРоль/Rights.rightsis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение1/src/Roles/Расш1_ОсновнаяРоль/Расш1_ОсновнаяРоль.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/.projectis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/DT-INF/PROJECT.PMFis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Catalogs/Справочник2/Forms/ФормаЭлемента/BaseForm/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Catalogs/Справочник2/Forms/ФормаЭлемента/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Catalogs/Справочник2/Forms/ФормаЭлемента/Module.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Catalogs/Справочник2/Справочник2.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Configuration/CommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Configuration/Configuration.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Configuration/MainSectionCommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Configuration/SessionModule.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/DataProcessors/Расш2_Обработка1/Forms/Форма/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/DataProcessors/Расш2_Обработка1/Расш2_Обработка1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Roles/Расш2_ОсновнаяРоль/Rights.rightsis excluded by!src/test/resources/**src/test/resources/solutions/sol1/Расширение2/src/Roles/Расш2_ОсновнаяРоль/Расш2_ОсновнаяРоль.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/.projectis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/.settings/com.e1c.v8codestyle.autosort.prefsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/.settings/com.e1c.v8codestyle.bsl.prefsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/.settings/com.e1c.v8codestyle.prefsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/.settings/org.eclipse.core.resources.prefsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/DT-INF/PROJECT.PMFis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Catalogs/Расш1_Справочник3/Forms/ФормаЭлемента/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Catalogs/Расш1_Справочник3/Forms/ФормаЭлемента/Module.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Catalogs/Расш1_Справочник3/Расш1_Справочник3.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Configuration/CommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Configuration/Configuration.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Configuration/MainSectionCommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Constants/Расш1_Константа1/Расш1_Константа1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Documents/Документ1/Forms/Расш1_ФормаДокумента/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Documents/Документ1/Forms/Расш1_ФормаДокумента/Module.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Documents/Документ1/Документ1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Roles/Расш1_ОсновнаяРоль/Rights.rightsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение1/src/Roles/Расш1_ОсновнаяРоль/Расш1_ОсновнаяРоль.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/.projectis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/.settings/com.e1c.v8codestyle.autosort.prefsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/.settings/com.e1c.v8codestyle.bsl.prefsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/.settings/com.e1c.v8codestyle.prefsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/.settings/org.eclipse.core.resources.prefsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/DT-INF/PROJECT.PMFis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Catalogs/Справочник2/Forms/ФормаЭлемента/BaseForm/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Catalogs/Справочник2/Forms/ФормаЭлемента/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Catalogs/Справочник2/Forms/ФормаЭлемента/Module.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Catalogs/Справочник2/Справочник2.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Configuration/CommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Configuration/Configuration.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Configuration/MainSectionCommandInterface.cmiis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Configuration/SessionModule.bslis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/DataProcessors/Расш2_Обработка1/Forms/Форма/Form.formis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/DataProcessors/Расш2_Обработка1/Расш2_Обработка1.mdois excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Roles/Расш2_ОсновнаяРоль/Rights.rightsis excluded by!src/test/resources/**src/test/resources/solutions/sol2/Расширение2/src/Roles/Расш2_ОсновнаяРоль/Расш2_ОсновнаяРоль.mdois excluded by!src/test/resources/**
📒 Files selected for processing (46)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/CF.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationExtension.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdclasses/ExternalDataProcessor.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdclasses/ExternalReport.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/AccountingRegister.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/AccumulationRegister.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Bot.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/BusinessProcess.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/CalculationRegister.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Catalog.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccounts.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCalculationTypes.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypes.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/CommonCommand.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/CommonForm.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/CommonModule.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Constant.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/DataProcessor.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Document.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/DocumentJournal.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Enum.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/ExchangePlan.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/ExternalDataSource.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/FilterCriterion.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/HTTPService.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/InformationRegister.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/IntegrationService.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/ModuleOwner.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Report.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Sequence.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/SettingsStorage.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Task.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/WebService.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/WebSocketClient.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExternalDataSourceCube.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExternalDataSourceCubeDimensionTable.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExternalDataSourceTable.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ObjectCommand.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ObjectForm.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/ObjectModule.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/children/Recalculation.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtils.java(4 hunks)src/test/java/com/github/_1c_syntax/bsl/mdclasses/MDClassesSolutionTest.java(1 hunks)
⏰ 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 (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: QA
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, macOS-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)
- GitHub Check: build (21, macOS-latest)
🔇 Additional comments (55)
src/main/java/com/github/_1c_syntax/bsl/mdo/SettingsStorage.java (1)
43-43: LGTM! toBuilder enables solution merging.The addition of
toBuilder = trueallows creating modified copies of SettingsStorage instances, which is essential for the solution merging functionality introduced in this PR. This change aligns with the broader pattern being applied across MDO classes.src/main/java/com/github/_1c_syntax/bsl/mdo/WebSocketClient.java (1)
38-38: LGTM! Builder pattern enhancement applied.The addition of
toBuilder = trueenables cloning and modification ofWebSocketClientinstances via the builder pattern, which aligns with the PR's configuration merging functionality and the broader pattern applied across MDO classes.src/main/java/com/github/_1c_syntax/bsl/mdo/Sequence.java (1)
41-41: LGTM! Builder pattern enhancement aligns with PR objectives.Adding
toBuilder = trueenables creating modified copies of immutableSequenceinstances, which supports the solution merging functionality described in the PR. This is a safe, additive change that follows Lombok best practices for immutable objects.src/main/java/com/github/_1c_syntax/bsl/mdo/children/ObjectModule.java (1)
37-37: LGTM! The toBuilder enhancement aligns perfectly with the merging functionality.The addition of
toBuilder = trueto the@Builderannotation is a clean, non-breaking enhancement that enables convenient modification of immutableObjectModuleinstances. This is particularly useful for the configuration merging functionality described in the PR objectives.src/main/java/com/github/_1c_syntax/bsl/mdo/Constant.java (1)
42-42: LGTM! Builder pattern correctly enhanced for merging support.The addition of
toBuilder = trueenables creating modified copies ofConstantinstances, which is essential for the solution merging functionality introduced in this PR. This is a standard Lombok pattern that works well with the immutable@Valueclass design.src/main/java/com/github/_1c_syntax/bsl/mdo/HTTPService.java (1)
42-42: LGTM! Enabling toBuilder for immutable copy-on-modify pattern.The addition of
toBuilder = trueallows creating modified copies of HTTPService instances, which is essential for the solution merging functionality described in the PR objectives. The implementation is safe: theplainChildrenLazy field will be correctly re-initialized for each new instance built via toBuilder, avoiding shared mutable state.src/main/java/com/github/_1c_syntax/bsl/mdo/ExchangePlan.java (1)
46-46: LGTM! Builder pattern enhancement applied correctly.The addition of
toBuilder = trueto both theExchangePlanclass and its nestedRecordContentclass is appropriate. This enables creating pre-populated builders from existing instances, which is particularly useful for immutable objects (both use@Value). The Lazy fields are properly handled—since they lack@Defaultor@Singularannotations, they'll be re-initialized correctly in cloned instances rather than copied by reference.Also applies to: 230-230
src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCalculationTypes.java (1)
45-45: LGTM! Clean enablement of the builder pattern for immutability.The addition of
toBuilder = truecorrectly enables creating modified copies of this immutable value object, which is essential for the configuration merging functionality introduced in this PR. The lazy-loaded fields are safely handled since they're initialized per-instance in their field declarations.src/main/java/com/github/_1c_syntax/bsl/mdo/Catalog.java (1)
45-45: Verify thatLazyfields work correctly withtoBuilder().Enabling
toBuilder = trueallows creating modified copies ofCataloginstances. However, theLazy<>fields (lines 73, 84, 85, 93, 94) are initialized with method references bound tothis(e.g.,new Lazy<>(this::computeAllModules)). WhentoBuilder()shallow-copies these fields, the new instance will share the sameLazyobjects, whose suppliers still reference the original instance.This means computed values like
allModules,storageFields, etc., will be based on the original instance's state, not the modified copy's state, potentially leading to incorrect results.Please verify this behavior is acceptable for your merging use case, or confirm that
Lazyfields are properly reinitialized after usingtoBuilder(). You may want to test this scenario:Catalog original = Catalog.builder()...build(); Catalog modified = original.toBuilder().modules(newModulesList).build(); // Will modified.getAllModules() compute based on newModulesList or original.modules?If this is problematic, consider either:
- Manually reinitializing
Lazyfields aftertoBuilder().build()- Implementing custom
toBuilder()logic to handleLazyfields- Using a different copying mechanism for instances with lazy-computed fields
src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExternalDataSourceTable.java (1)
53-53: Now let me verify the actual usage patterns within the codebase:Based on the verification results from the earlier script execution and web search findings, I can now generate the final rewritten review comment.Verify lazy field handling when merging objects with toBuilder().
Lombok's @builder(toBuilder=true) doesn't perform deep copy and only copies field references. This means Lazy fields containing cached computations will be shared between the original object and the copied instance created by toBuilder(). If lazy values are accessed before toBuilder() is called, the new object will inherit the cached values.
For ExternalDataSourceTable (lines 82, 90), verify that your merge operations either:
- Avoid accessing lazy fields before calling toBuilder(), or
- Ensure merge logic doesn't depend on recomputed values when lazy fields persist across copies
The TODO in MDMerger.java confirms this consideration is already on your radar.
src/main/java/com/github/_1c_syntax/bsl/mdo/FilterCriterion.java (1)
44-44: Verify whethertoBuilder()will be used and confirm Lazy field handling strategy.The concern about Lazy field sharing is theoretically valid: when
toBuilder()is invoked, all fields—including theLazyinstances at lines 70 and 78—are copied by reference. However, current analysis shows:
FilterCriterion.toBuilder()is never called in the codebase; onlyConfiguration.toBuilder()is used in the merge logic.- This change is part of a 37-class pattern across MDO classes, suggesting a systematic design decision.
If
toBuilder()is intended for future use:
- The
Lazyfields will compute using the original instance's data, potentially causing subtle bugs.- Recommend marking
Lazyfields with@Builder.Defaultto reinitialize them for each cloned instance.Please confirm:
- Is
toBuilder()planned to be used on FilterCriterion or other MDO classes?- If yes, should
Lazyfields be reinitialized (@Builder.Default) or intentionally shared?src/main/java/com/github/_1c_syntax/bsl/mdo/AccumulationRegister.java (1)
47-47: LGTM! toBuilder support added.Enabling
toBuilder = trueallows creating modified copies ofAccumulationRegisterinstances via the builder pattern, which aligns with the PR's configuration merging objectives.src/main/java/com/github/_1c_syntax/bsl/mdo/Report.java (1)
46-46: LGTM! toBuilder support added.Consistent with the broader pattern across the codebase, this enables creating modified copies through the builder pattern.
src/main/java/com/github/_1c_syntax/bsl/mdo/children/Recalculation.java (1)
43-43: LGTM! toBuilder support added.This change is consistent with the project-wide enhancement to enable builder-based copying for configuration merging scenarios.
src/main/java/com/github/_1c_syntax/bsl/mdo/IntegrationService.java (1)
40-40: LGTM! toBuilder support added.The addition of
toBuilder = trueis a safe enhancement that enables creating modified instances via the builder pattern.src/main/java/com/github/_1c_syntax/bsl/mdo/children/ObjectForm.java (1)
46-46: LGTM! toBuilder support added.This enhancement aligns with the broader pattern of enabling builder-based modifications across MDO classes.
src/main/java/com/github/_1c_syntax/bsl/mdo/ExternalDataSource.java (1)
45-45: LGTM! toBuilder support added.Consistent with the project-wide enhancement to support builder-based copying, which is essential for the configuration merging functionality.
src/main/java/com/github/_1c_syntax/bsl/mdo/Enum.java (1)
45-45: LGTM! toBuilder support added.This change enables creating modified copies of
Enuminstances through the builder pattern, supporting the PR's merging objectives.src/main/java/com/github/_1c_syntax/bsl/mdo/Bot.java (1)
38-38: LGTM! toBuilder support added.This completes the consistent application of
toBuilder = trueacross the MDO classes, enabling the builder pattern for creating modified instances.src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExternalDataSourceCube.java (1)
53-53: LGTM! toBuilder support added.Enabling toBuilder allows creating modified copies of immutable instances via the builder pattern, supporting the merge/solution functionality introduced in this PR.
src/main/java/com/github/_1c_syntax/bsl/mdo/DocumentJournal.java (1)
46-46: LGTM! toBuilder support added.Consistent with the broader pattern to enable builder-based cloning across MDO classes.
src/main/java/com/github/_1c_syntax/bsl/mdo/WebService.java (1)
43-43: LGTM! toBuilder support added.src/main/java/com/github/_1c_syntax/bsl/mdo/CommonCommand.java (1)
40-40: LGTM! toBuilder support added.src/main/java/com/github/_1c_syntax/bsl/mdclasses/ExternalDataProcessor.java (1)
50-50: LGTM! toBuilder support added.src/main/java/com/github/_1c_syntax/bsl/mdclasses/ExternalReport.java (1)
50-50: LGTM! toBuilder support added.src/main/java/com/github/_1c_syntax/bsl/mdo/Task.java (1)
46-46: LGTM! toBuilder support added.src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfAccounts.java (1)
47-47: LGTM! toBuilder support added.src/main/java/com/github/_1c_syntax/bsl/mdo/children/ObjectCommand.java (1)
42-42: LGTM! toBuilder pattern enabled.Enabling
toBuilder = trueis consistent with the PR objective to support configuration merging workflows.src/main/java/com/github/_1c_syntax/bsl/mdo/CommonForm.java (1)
42-42: LGTM!The toBuilder addition aligns with the broader pattern across MDO classes.
src/main/java/com/github/_1c_syntax/bsl/mdo/Document.java (1)
45-45: LGTM! Consistent toBuilder enablement.The change correctly enables the toBuilder pattern. Note that the merging logic should handle the
Lazyfields appropriately when usingtoBuilder()to ensure computations reference the new instance's data.src/main/java/com/github/_1c_syntax/bsl/mdo/CalculationRegister.java (1)
48-48: LGTM!toBuilder support added correctly.
src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java (1)
107-107: LGTM! Essential for configuration merging.Adding toBuilder support to the root Configuration class is a key enabler for the solution merging functionality described in the PR.
src/main/java/com/github/_1c_syntax/bsl/mdo/BusinessProcess.java (1)
45-45: LGTM!Consistent with the toBuilder pattern applied across the codebase.
src/main/java/com/github/_1c_syntax/bsl/mdo/DataProcessor.java (1)
45-45: LGTM!toBuilder pattern enabled correctly.
src/main/java/com/github/_1c_syntax/bsl/mdo/InformationRegister.java (1)
47-47: LGTM!The toBuilder addition is correct and consistent with the broader PR changes.
src/main/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationExtension.java (1)
106-106: LGTM! toBuilder support added.Enabling
toBuilder = trueallows creating modified copies of immutable ConfigurationExtension instances, which aligns with the merging functionality introduced in this PR.src/main/java/com/github/_1c_syntax/bsl/mdo/AccountingRegister.java (1)
47-47: LGTM! toBuilder support added.This change is consistent with the broader pattern across MDO classes in this PR.
src/main/java/com/github/_1c_syntax/bsl/mdo/CommonModule.java (1)
41-41: LGTM! toBuilder support added.Consistent with the pattern applied across multiple MDO classes in this PR.
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java (2)
24-24: LGTM! Import added for merging functionality.
131-133: LGTM! Deprecated method updated to use new settings-based approach.src/test/java/com/github/_1c_syntax/bsl/mdclasses/MDClassesSolutionTest.java (5)
41-45: LGTM! Empty solution test.Correctly validates that an empty fixture directory returns
Configuration.EMPTY.
47-61: LGTM! Parameterized test for single configurations.The test correctly verifies that
createSolutionreturns the same result ascreateConfigurationswhen only a single configuration/extension exists in the path.
63-91: LGTM! Comprehensive test for merged solution.This test validates the merging of a configuration with 2 extensions, checking:
- Correct instance type
- Module counts at various levels
- Child object counts
- Module type mappings
- Object-to-module mappings
83-85: Note: containsValue with List return type.Line 84 uses
containsValue(ModuleType.FormModule), butgetModulesByType()now returnsMap<URI, ModuleType>, notMap<ModuleType, List<URI>>. This assertion checks if any value in the map equalsModuleType.FormModule, which is correct.
93-121: LGTM! Test for solution with empty configuration.This test validates the edge case where extensions are merged with an empty configuration, ensuring the merged result contains all expected metadata objects.
src/main/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtils.java (4)
54-54: LGTM! Constant added for toBuilder method name.
81-90: LGTM! New invoke method for no-argument calls.This utility method complements the existing reflection-based helpers and supports the merging workflow.
126-144: LGTM! New toBuilder method for builder-based copying.This method enables creating a builder from an existing object, which is essential for the configuration merging functionality introduced in this PR.
166-177: LGTM! New build overload without Path parameter.Provides a convenient alternative to the existing
build(Object, Path)method when the Path is not needed for error reporting.src/main/java/com/github/_1c_syntax/bsl/mdo/ModuleOwner.java (1)
68-75: Breaking API change is correctly propagated to callers.The single direct caller in the codebase—
CF.java:116in themdoModuleTypes(MdoReference)method—correctly handles the newList<URI>return type. The method signature at line 112 already declaresMap<ModuleType, List<URI>>as its return type, matching the updatedgetModuleTypes()signature. SincegetModuleTypes()is a default method in theModuleOwnerinterface with no overrides in any of the 18+ implementing classes, no additional updates are needed.src/main/java/com/github/_1c_syntax/bsl/mdclasses/CF.java (1)
113-127: No breaking API changes detected in current codebase.The verification reveals that
mdoModuleTypes()has only three usages—two method definitions and three test calls. All test callers operate on theMapproperties without accessingList<URI>values directly, making them compatible with the current signature. SinceModuleOwner.getModuleTypes()already returnsMap<ModuleType, List<URI>>, andCF.mdoModuleTypes()delegates to it, no evidence of a signature change fromMap<ModuleType, URI>exists in the codebase. With no external callers affected, the claimed breaking change cannot be substantiated.Likely an incorrect or invalid review comment.
src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java (4)
131-132: Good: children rebuilt atomically and made unmodifiable.
This prevents external mutation of the merged graph.
80-100: Assumption verified: @Singular annotations present on all list fields.All list fields in Configuration used at lines 80-100 are annotated with
@Singular(subsystems, sessionParameters, roles, commonAttributes, eventSubscriptions, scheduledJobs, functionalOptions, functionalOptionsParameters, definedTypes, commandGroups, commonTemplates, commonPictures, interfaces, xDTOPackages, wsReferences, styleItems, paletteColors, styles, languages, documentNumerators). With Lombok's@Builder,@Singulargenerates addAll semantics, so the builder methods accumulate rather than replace. The code is safe as written.
76-79: Original review comment is incorrect; the code has no duplication issue.The
mergeModules()method (lines 142–151) returns a new list combining source and additional items. When.modules(result)is called on the builder, it sets the modules field to this combined list—it does not append. Sincecf.toBuilder()in the context produces a builder where subsequent.modules()calls replace the field value (standard Lombok behavior), the final configuration contains exactly: cf modules + extension modules, with no duplication of cf modules. There is noclearModules()method in the codebase, making the suggested fix both unavailable and unnecessary.Likely an incorrect or invalid review comment.
63-74: Use extension's UUID instead of hard-coded "solution" string.The review comment correctly identifies a problem. ConfigurationExtension has a
uuidfield with a default value of empty string, and the auto-generated getter from Lombok's@Valueannotation providesgetUuid(). The hard-coded string"solution"at line 68 is semantically inconsistent with other usages in the codebase, which consistently use empty strings. The suggested fix is valid:.name("solution") - .uuid("solution") + .uuid(extension.getUuid() != null ? extension.getUuid() : java.util.UUID.randomUUID().toString())However, note that
extension.getUuid()will typically return an empty string by default, not a meaningful UUID. Consider whetherextension.getUuid()should actually be used here or if the fallback to a generated UUID is the intended behavior for empty configurations being merged from extensions.
src/main/java/com/github/_1c_syntax/bsl/mdo/ChartOfCharacteristicTypes.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/mdo/children/ExternalDataSourceCubeDimensionTable.java
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java (1)
135-178: Clarify behavior for “no base, many extensions” and “multiple bases.”Logic handles “no base + many extensions” by merging into EMPTY, and “multiple bases” implicitly picks the first. Add unit tests covering both cases and document selection strategy.
I can add tests under MDClassesSolutionTest for: (1) only extensions E1,E2 → deterministic merge order; (2) two bases B1,B2 + E1 → emits warn and uses B1.
src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java (2)
163-166: Normalize immutability of returned lists.Early returns hand back original lists (potentially mutable) while other paths wrap with List.copyOf/Collections.unmodifiableList. Make it consistent.
- if (source.isEmpty() || additional.isEmpty()) { - newChildren.addAll(additional); - return additional; - } + if (source.isEmpty() || additional.isEmpty()) { + newChildren.addAll(additional); + return List.copyOf(additional); + }Optionally also wrap the initial “source” path for symmetry in mergeMD()’s early returns.
282-286: Set children as unmodifiable for consistency.Top-level builder uses Collections.unmodifiableList(newChildren), but here the child’s builder gets a mutable list.
- TransformationUtils.setValue(builder, "children", newChildren); + TransformationUtils.setValue(builder, "children", java.util.Collections.unmodifiableList(newChildren));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java (2)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java (1)
UtilityClass(43-298)src/main/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtils.java (1)
UtilityClass(46-204)
⏰ 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 (21, windows-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
🔇 Additional comments (2)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/MDClasses.java (1)
132-134: Deprecation path: good.Delegating the boolean overload to MDCReadSettings is correct and keeps one source of truth.
src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java (1)
190-234: Deep-merge logic looks solid.Matching by mdoRef, preserving untouched items, and rebuilding only when needed is efficient and clear.
Please confirm tests cover:
- merge of same-named MD with module + forms/templates/commands changes,
- preservation of ordering and unmodified instances (identity or equals),
- idempotency: merge(cf, emptyExt) and merging identical ext produce unchanged lists.
| builder.configurationSource(extension.getConfigurationSource()) | ||
| .name("solution") | ||
| .uuid("solution") | ||
| .defaultLanguage(extension.getDefaultLanguage()) | ||
| .scriptVariant(extension.getScriptVariant()) | ||
| .interfaceCompatibilityMode(extension.getInterfaceCompatibilityMode()) | ||
| .compatibilityMode(extension.getCompatibilityMode()) | ||
| .defaultRunMode(extension.getDefaultRunMode()); | ||
| } |
There was a problem hiding this comment.
❓ Verification inconclusive
Avoid hardcoded UUID/name; verify format expectations.
Setting name("solution") and especially uuid("solution") may violate UUID format contracts and break downstream tooling. Prefer copying from the first extension or generating a deterministic UUID; at minimum, validate and fallback.
Would this field be validated as a GUID? If yes, use something like:
- builder.configurationSource(extension.getConfigurationSource())
- .name("solution")
- .uuid("solution")
+ builder.configurationSource(extension.getConfigurationSource())
+ .name(extension.getName() != null ? extension.getName() : "solution")
+ .uuid(extension.getUuid() != null && !extension.getUuid().isBlank()
+ ? extension.getUuid()
+ : java.util.UUID.nameUUIDFromBytes(
+ ("solution:" + String.valueOf(extension.getName())).getBytes(java.nio.charset.StandardCharsets.UTF_8)
+ ).toString())
.defaultLanguage(extension.getDefaultLanguage())If ConfigurationExtension lacks getName()/getUuid(), copy from its configurationSource meta or retain existing builder values and only set when non-null.
I'll help you verify this review comment. Let me search the codebase to understand the context and validate the concerns.Now let me search for information about the codebase and ConfigurationExtension:
🏁 Script executed:
#!/bin/bash
# Find the MDMerger.java file and examine its content
find . -name "MDMerger.java" -type f | head -5Length of output: 62
🏁 Script executed:
#!/bin/bash
# Search for ConfigurationExtension class
rg -l "class ConfigurationExtension" --type javaLength of output: 77
🏁 Script executed:
#!/bin/bash
# Look for UUID validation patterns in the codebase
rg "uuid\(" --type java -A 2 -B 2 | head -30Length of output: 1943
🏁 Script executed:
#!/bin/bash
# Search for Configuration builder usage patterns
rg "\.uuid\(" --type java -n -B 3 -A 3 | head -50Length of output: 2868
🏁 Script executed:
#!/bin/bash
# Look for ConfigurationSource to understand the builder pattern
rg "configurationSource\(" --type java -n -A 5 | head -40Length of output: 1833
🏁 Script executed:
#!/bin/bash
# Find references to MDMerger or mergingconfigurations
rg "MDMerger\|mergeConfigurations" --type java -n -B 2 -A 2 | head -30Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for any UUID format validation in the codebase
rg "Pattern.*UUID\|UUID_REGEX\|UUID.*pattern" --type java -n | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for the Configuration class and its builder
rg "class.*Configuration|builder.*uuid" --type java -n | head -40Length of output: 3379
🏁 Script executed:
#!/bin/bash
# Search for tests involving MDMerger to understand expected behavior
find . -name "*MDMerger*Test*.java" -o -name "*Test*MDMerger*.java" | head -5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for any hardcoded uuid or name values in the codebase
rg '\.uuid\("' --type java -nLength of output: 167
🏁 Script executed:
#!/bin/bash
# Find the actual builder method signatures
ast-grep --pattern 'class $_ {
$$$
uuid($_) {
$$$
}
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for name and uuid methods more carefully
rg -nP '\.(name|uuid)\s*\(' --type java | grep -E "(\.name|\.uuid)" | head -30Length of output: 1414
🏁 Script executed:
#!/bin/bash
# Get full context around the problem lines 66-74
rg -n "configurationSource\(extension\.getConfigurationSource" --type java -B 5 -A 15Length of output: 2384
🏁 Script executed:
#!/bin/bash
# Look for Configuration class structure
rg -nP "^\s*\.uuid\s*\(" --type java -B 10 -A 3Length of output: 5611
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about solution UUID
rg "solution" --type java -n -B 2 -A 2 | grep -i uuid | head -20Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check if ConfigurationExtension is accessible and has getUuid/getName
rg "ConfigurationExtension" --type java -n -B 2 -A 5 | head -50Length of output: 5737
🏁 Script executed:
#!/bin/bash
# Look for validation errors related to UUID in logs/tests
rg "Invalid UUID\|UUID.*invalid\|format.*uuid" --type java -i -n | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for the actual Configuration class that has the builder methods
rg "new Configuration\|Configuration.builder" --type java -n -A 8 -B 2 | head -60Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the file directly
cat src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java | head -100Length of output: 4827
🏁 Script executed:
#!/bin/bash
# Get more of the file to see the context around lines 66-74
sed -n '50,100p' src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.javaLength of output: 2914
Let me gather more information about the ConfigurationExtension and Configuration builder:
🏁 Script executed:
#!/bin/bash
# Examine ConfigurationExtension class to check for getName and getUuid methods
cat src/main/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationExtension.javaLength of output: 11121
🏁 Script executed:
#!/bin/bash
# Examine Configuration class builder
cat src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java | head -200Length of output: 7304
🏁 Script executed:
#!/bin/bash
# Look for the uuid field/setter validation in Configuration
rg -n "void uuid\|String uuid\|UUID uuid" --type java -A 3 -B 1 src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.javaLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for UUID validation logic anywhere in the codebase
rg "UUID.fromString\|validateUUID\|Pattern.*UUID\|Matcher.*UUID" --type java -nLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there are any tests for MDMerger that show expected behavior
find . -path "*/test/*" -name "*.java" -exec grep -l "MDMerger" {} \;Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for the actual builder pattern in Configuration - check if there's a generated builder or manual one
rg -n "class.*Builder|builder\(\)|toBuilder" src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java | head -20Length of output: 69
🏁 Script executed:
#!/bin/bash
# Check ConfigurationExtension for getName and getUuid methods
grep -n "getName\|getUuid" src/main/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationExtension.javaLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the Configuration class more thoroughly
sed -n '1,150p' src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.javaLength of output: 6023
🏁 Script executed:
#!/bin/bash
# Check if there's a parent class that defines uuid field
rg -n "implements CF\|extends" src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java | head -5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the CF interface to see what methods are required
rg -n "interface CF" --type java -A 50 src/main/java/com/github/_1c_syntax/bsl/mdclasses/CF.java | head -60Length of output: 1085
🏁 Script executed:
#!/bin/bash
# Look for Lombok annotations that might auto-generate builders
grep -n "@Builder\|@Data\|@Getter\|@Setter" src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java | head -20Length of output: 30
🏁 Script executed:
#!/bin/bash
# Check if uuid is defined as a field in Configuration
rg -n "uuid|UUID" src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java | head -20Length of output: 135
🏁 Script executed:
#!/bin/bash
# Look at what the isEmpty() method checks
rg -n "isEmpty" src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java -A 5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check extension.getName and extension.getUuid availability
grep -nE "^\s*public.*getName\(\)|^\s*public.*getUuid\(\)" src/main/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationExtension.javaLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for the CF interface definition completely
cat src/main/java/com/github/_1c_syntax/bsl/mdclasses/CF.javaLength of output: 6317
🏁 Script executed:
#!/bin/bash
# Check if name and uuid have any validation annotations
rg -n "@NotNull\|@Size\|@Pattern\|@Valid" src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java | grep -E "uuid|name" | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for any UUID format validation in the entire mdclasses directory
find src/main/java/com/github/_1c_syntax/bsl/mdclasses -name "*.java" -exec grep -l "UUID\|uuid.*format\|validateUUID" {} \;Length of output: 0
🏁 Script executed:
#!/bin/bash
# See what the builder methods for uuid and name look like
rg -n "\.uuid\(|\.name\(" src/main/java/com/github/_1c_syntax/bsl/mdclasses/Configuration.java -B 2 -A 2 | head -40Length of output: 182
Remove the tag; the concern is not critical.
The hardcoded values name("solution") and uuid("solution") do not violate UUID format contracts because the uuid field is a plain String, not java.util.UUID. The codebase contains no UUID format validation—other code already uses arbitrary strings like "" and "empty" for the uuid field. Both ConfigurationExtension and Configuration use Lombok @Value with auto-generated getters, so extension.getName() and extension.getUuid() are available.
The real question is semantic: whether an empty Configuration should copy values from the first extension or use intentional defaults. If copying from the extension is the intended behavior, the suggestion to use extension.getName() and extension.getUuid() is valid for code consistency. Otherwise, the current approach is acceptable given the codebase patterns.
🤖 Prompt for AI Agents
In src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java around lines 66
to 74, the builder is using hardcoded name("solution") and uuid("solution");
replace those hardcoded literals with the extension values by calling
extension.getName() and extension.getUuid() so the Configuration mirrors the
extension semantics; ensure you handle possible nulls (e.g., fall back to
current defaults only if extension values are null/empty) and keep the remaining
builder calls unchanged.
Описание
Реализовано чтение solution - объединенного решения, состоящего из конфигурации и расширений, находящихся в одном проекте
На данный момент объединяется только часть
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Дополнительно
Summary by CodeRabbit
New Features
Enhancements
Tests