Чтение конфигураций из файла, проверка на пустоту конфигурации#526
Conversation
2. реализовано чтение mdclass по переданному пути к файлу корня конфигурации
WalkthroughДобавлен метод isEmpty() в интерфейс CF; читатели MDO/Designer/EDT обновлены для распознавания входного пути по имени файла и нормализации rootPath; из Reader-классов удалены явные @nonnull аннотации, а в пакетах designer/edt добавлены package-level nonnull-аннотации; тесты переписаны на явные @test с конкретными путями. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant MDOReader
participant FS as FileSystem
Caller->>MDOReader: read(rootPath)
alt rootPath указывает на файл
MDOReader->>FS: getFileName(rootPath)
alt имя == DesignerReader.CONFIGURATION_MDO_FILE_NAME
MDOReader->>MDOReader: source = DESIGNER
else имя == EDTReader.CONFIGURATION_MDO_FILE_NAME
MDOReader->>MDOReader: source = EDT
else
MDOReader->>MDOReader: source = UNKNOWN
end
else rootPath — директория
MDOReader->>FS: exists(Designer конфиг путь)
alt существует
MDOReader->>MDOReader: source = DESIGNER
else
MDOReader->>FS: exists(EDT конфиг путь)
alt существует
MDOReader->>MDOReader: source = EDT
else
MDOReader->>MDOReader: source = UNKNOWN
end
end
end
MDOReader-->>Caller: результат
sequenceDiagram
autonumber
actor Client
participant DesignerReader
participant EDTReader
participant FS as FileSystem
rect rgb(235, 245, 255)
note over DesignerReader: Конструктор — файл "Configuration.xml" → root = parent
Client->>DesignerReader: new(path)
DesignerReader->>FS: isRegularFile(path)?
alt файл и имя == "Configuration.xml"
DesignerReader->>DesignerReader: rootPath = path.getParent()
else
DesignerReader->>DesignerReader: rootPath = path
end
end
rect rgb(235, 245, 255)
note over EDTReader: Конструктор — файл "Configuration.mdo" → root = path.getParent().getParent().getParent()
Client->>EDTReader: new(path)
EDTReader->>FS: isRegularFile(path)?
alt файл и имя == "Configuration.mdo"
EDTReader->>EDTReader: rootPath = path.getParent().getParent().getParent()
else
EDTReader->>EDTReader: rootPath = path
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/test/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationTest.java (2)
216-223: Уточните назначение булева параметра вcreateConfiguration.Метод
MDClasses.createConfiguration(configurationPath, true)вызывается с булевым параметромtrue, но его назначение неочевидно из контекста теста. Рекомендуется добавить комментарий, поясняющий значение этого параметра для улучшения читаемости теста.Также обратите внимание, что использование жёстко заданного пути вместо параметризованного теста снижает гибкость, но упрощает понимание конкретного сценария.
Пример комментария:
+ // true означает, что конфигурация должна быть прочитана из файла var mdc = MDClasses.createConfiguration(configurationPath, true);
252-266: Рассмотрите расширение покрытия тестомtestFullExtEdt.Метод
testFullExtEdtвыполняет значительно меньше проверок по сравнению сtestFullExt. ТестtestFullExtвключает вызовcheckChildrenExt(cf)(строка 233), который проверяет состав всех дочерних элементов, тогда какtestFullExtEdtпроверяет только порядок элементов.Рассмотрите возможность добавления аналогичных проверок для обеспечения одинакового уровня покрытия обоих форматов (Designer и EDT).
Применить следующий diff для добавления проверки дочерних элементов:
var cf = (ConfigurationExtension) mdc; assertThat(cf.isEmpty()).isFalse(); + // проверка состава дочерних + checkChildrenExt(cf); + // проверка порядок checkChildrenOrder(cf);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
src/test/resources/fixtures/mdclasses/Configuration.jsonis excluded by!**/*.jsonsrc/test/resources/fixtures/mdclasses/Configuration_edt.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/mdclasses_unknown/Configuration_edt.jsonis excluded by!**/*.json
📒 Files selected for processing (5)
src/main/java/com/github/_1c_syntax/bsl/mdclasses/CF.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.java(2 hunks)src/test/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationTest.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
PR: 1c-syntax/mdclasses#515
File: src/test/java/com/github/_1c_syntax/bsl/mdo/AccountingRegisterTest.java:35-37
Timestamp: 2025-09-05T10:03:02.229Z
Learning: The MDTestUtils.getMDWithSimpleTest method already correctly handles optional 4th parameter "_edt" through ArgumentsAccessor - it doesn't need modification to process the additional CSV parameter for subdirectory selection.
Applied to files:
src/test/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationTest.java
⏰ 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, macOS-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: QA
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (4)
src/test/java/com/github/_1c_syntax/bsl/mdclasses/ConfigurationTest.java (3)
36-36: Импорты добавлены корректно.Новые импорты необходимы для использования аннотации
@Testи явных объектовPathв обновлённых тестовых методах.Also applies to: 41-41
225-225: Проверка isEmpty() реализована корректно.Тест проверяет новый метод
isEmpty(), добавленный согласно задачам PR. УтверждениеisFalse()корректно для загруженного расширения конфигурации.
185-214: Несоответствие с описанием изменений.Сводка AI указывает, что метод
testFullMdclassesбыл обновлён с параметризованного на явную структуру теста, однако в предоставленном коде этот метод остаётся параметризованным и не содержит изменений (отсутствуют маркеры~). Это расхождение может указывать на неточность в автоматической сводке.src/main/java/com/github/_1c_syntax/bsl/mdclasses/CF.java (1)
200-207: isEmpty(): проверка идентичности корректна Configuration.EMPTY объявлен static final и инициализируется единожды, поэтому оператор == уместен. Default-метод не ломает обратную совместимость. Убедитесь, что для расширений предусмотрен аналогичный sentinel (Extension.EMPTY) или переопределён isEmpty.
Test Results 522 files ±0 522 suites ±0 7m 34s ⏱️ + 1m 4s Results for commit fd6b13f. ± Comparison against base commit b061ef8. This pull request removes 2 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.java (1)
98-115: Унифицируйте нормализацию rootPath. Сейчас при передаче каталогаrootPathостаётся в исходном (возможно относительном) виде, тогда как для файла он уже абсолютный. Для предсказуемости лучше нормализовать путь в обоих случаях.- var normalizedPath = path.toAbsolutePath(); + var normalizedPath = path.toAbsolutePath().normalize(); @@ - } else { - rootPath = path; + } else { + rootPath = normalizedPath;src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java (1)
100-111: Приведите rootPath к нормализованному виду и для каталогов. При передаче директорииrootPathостаётся как есть, из-за чего поведение отличается от ветки с файлом. Стоит сразу переходить к абсолютному нормализованному пути.- var normalizedPath = path.toAbsolutePath(); + var normalizedPath = path.toAbsolutePath().normalize(); @@ - } else { - rootPath = path; + } else { + rootPath = normalizedPath;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/designer/package-info.java(1 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/package-info.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-06T14:38:27.354Z
Learnt from: theshadowco
PR: 1c-syntax/mdclasses#524
File: src/main/java/com/github/_1c_syntax/bsl/mdo/support/UsePurposes.java:61-69
Timestamp: 2025-10-06T14:38:27.354Z
Learning: В проекте mdclasses используются JSR-305 аннотации (ParametersAreNonnullByDefault) на уровне пакетов для обеспечения null-safety через статический анализ, что является приемлемым подходом вместо явных runtime проверок.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/reader/edt/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/package-info.java
⏰ 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 (23, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, windows-latest)
- 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: QA
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (2)
src/main/java/com/github/_1c_syntax/bsl/reader/designer/package-info.java (1)
25-26: Аннотации по умолчанию выглядят корректно. Пакет теперь наследует ожидаемое поведение по nullability без дублирования аннотаций на каждом методе.src/main/java/com/github/_1c_syntax/bsl/reader/edt/package-info.java (1)
25-26: Хорошее покрытие аннотациями по умолчанию. Пакет EDT теперь следует тем же правилам nullability, что и designer.
1 participant
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Дополнительно