feat: Добавлена поддержка CodeSeries и CheckUnique в Catalog#548
Conversation
WalkthroughAdds parsing and model support for Catalog's checkUnique and codeSeries: new fields on Changes
Sequence Diagram(s)sequenceDiagram
participant File as XML/EDT File
participant XStream as XStream Deserializer
participant Conv as CodeSeries Converter
participant Catalog as Catalog Model
File->>XStream: supply <CheckUnique>/<checkUnique> and codeSeries string
XStream->>Conv: convert codeSeries string to enum
Conv->>Conv: CodeSeries.valueByName(string) (case-insensitive, default WHOLE_CATALOG)
Conv-->>XStream: return CodeSeries enum
XStream->>Catalog: set checkUnique (boolean)
XStream->>Catalog: set codeSeries (CodeSeries)
note right of Catalog: Catalog populated with new fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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: 0
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/mdo/support/CodeSeries.java (1)
53-81: HardenvalueByNameagainst null/whitespace input
valueByNamewill throwNullPointerExceptionifstringisnull, and will not match values with leading/trailing spaces (e.g." WholeCatalog "), silently defaulting toWHOLE_CATALOG. For robustness against parser quirks and defensive use, consider null/whitespace handling:public static CodeSeries valueByName(String string) { - return KEYS.getOrDefault(string.toLowerCase(Locale.ROOT), WHOLE_CATALOG); + if (string == null) { + return WHOLE_CATALOG; + } + + var key = string.trim().toLowerCase(Locale.ROOT); + if (key.isEmpty()) { + return WHOLE_CATALOG; + } + + return KEYS.getOrDefault(key, WHOLE_CATALOG); }This keeps the existing defaulting behavior while avoiding NPEs and tolerating minor formatting issues in source data.
📜 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/mdo/Catalog.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/CodeSeries.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/github/_1c_syntax/bsl/mdo/Catalog.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). (9)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (21, macOS-latest)
🔇 Additional comments (2)
src/main/java/com/github/_1c_syntax/bsl/mdo/support/CodeSeries.java (2)
1-33: Header, package, and Lombok usage look consistent with project styleLicense header, package declaration, and Lombok annotations (
@Getter,@ToString,@Accessors) are coherent and align with the existing patterns in the codebase. No issues here.
38-61: Enum design and naming are clear and match the domainThe enum constants, bilingual names via
MultiName, and@ToString(of = "fullName")give a clear, localized representation of code series. This fits the described semantics (scope of code uniqueness) and should integrate cleanly with the existingEnumWithNameinfrastructure.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @johnnyshut. * #548 (comment) The following files were modified: * `src/main/java/com/github/_1c_syntax/bsl/mdo/support/CodeSeries.java` * `src/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/ExtendXStream.java`
|
@coderabbitai отмени свой pull request |
|
Tip For best results, initiate chat on the files or code changes. @johnnyshut, я не могу отменить этот pull request, потому что он был создан вами, а не мной. PR #548 был создан пользователем johnnyshut (вами) из ветки Если вы хотите закрыть этот PR, вы можете сделать это самостоятельно:
Если у вас есть другие вопросы или вам нужна помощь с этим PR, дайте мне знать! 😊 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java (1)
159-168: Cover both Designer and EDT flows intestCheckUniqueFalseRight now
testCheckUniqueFalseruns only the EDT variant ("true, ssl_3_1, Catalogs.Заметки, _edt"), while other tests in this class (test,testSSL,testSSLFixture) parameterize over both EDT and Designer with two CSV rows. Given the PR goal to supportCheckUniquefrom both Designer (<CheckUnique>) and EDT (<checkUnique>), it’s worth adding the non‑EDT row here as well to exercise both parsing paths.For consistency with
testSSL, you can do:@ParameterizedTest @CsvSource({ - "true, ssl_3_1, Catalogs.Заметки, _edt" + "true, ssl_3_1, Catalogs.Заметки, _edt", + "false, ssl_3_1, Catalogs.Заметки" }) void testCheckUniqueFalse(ArgumentsAccessor argumentsAccessor) { var mdo = MDTestUtils.getMDWithSimpleTest(argumentsAccessor); assertThat(mdo).isInstanceOf(Catalog.class); var catalog = (Catalog) mdo; assertThat(catalog.isCheckUnique()).isFalse(); }This will validate that
isCheckUnique()isfalsefor both Designer and EDT metadata forCatalogs.Заметки, in line with the issue requirements. Based on learnings,MDTestUtils.getMDWithSimpleTestalready supports this CSV shape without further changes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/test/resources/ext/edt/ssl_3_1is excluded by!src/test/resources/**src/test/resources/fixtures/ssl_3_1/Catalogs.Заметки_edt.jsonis excluded by!src/test/resources/**
📒 Files selected for processing (1)
src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
Repo: 1c-syntax/mdclasses PR: 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 through ArgumentsAccessor.size() check and argumentsAccessor.getString(3) call with null handling - no modification needed for CSV parameter processing.
Applied to files:
src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
Repo: 1c-syntax/mdclasses PR: 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/mdo/CatalogTest.java
cb56320 to
054c62f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
Repo: 1c-syntax/mdclasses PR: 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 through ArgumentsAccessor.size() check and argumentsAccessor.getString(3) call with null handling - no modification needed for CSV parameter processing.
Applied to files:
src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
Repo: 1c-syntax/mdclasses PR: 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/mdo/CatalogTest.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). (9)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, ubuntu-latest)
Добавлены поля для управления проверкой уникальности кода справочника: - checkUnique: определяет, нужно ли проверять уникальность кода - codeSeries: определяет область действия уникальности кода Изменения: - Добавлены поля checkUnique и codeSeries в класс Catalog с значениями по умолчанию - Добавлена регистрация CodeSeries в ExtendXStream для сериализации - Добавлен импорт CodeSeries в Catalog Значение по умолчанию для checkUnique: false (код должен быть уникальным) Значение по умолчанию для codeSeries: WHOLE_CATALOG
Обновлены JSON фикстуры для добавления полей checkUnique и codeSeries: - mdclasses: Catalogs.Справочник1.json, Configuration.json - mdclasses_3_24: Configuration_edt.json - ssl_3_1: Catalogs.ВерсииФайлов.json, Catalogs.Заметки.json и их _edt версии Исправлено значение checkUnique в фикстуре Catalogs.Заметки_edt.json с true на false (соответствует реальному значению в XML файле)
Добавлен параметризованный тест testCheckUniqueFalse для проверки, что поле checkUnique установлено в false для справочника "Заметки" в обоих форматах (Designer и EDT). Также выполнена очистка кода: - Удален закомментированный метод checkExtInfo - Обновлен .gitignore (добавлены bin/ и .vscode/)
054c62f to
d8e4aab
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/mdo/support/CodeSeries.java (1)
24-26: Опционально: обезопаситьCodeSeries.valueByNameотnull
valueByNameбез проверки вызываетstring.toLowerCase(Locale.ROOT), что дастNullPointerExceptionприnull. Если входная строка может быть отсутствующей в XML/MDO, логичнее либо:
- трактовать
nullкак “не задано” и возвращатьWHOLE_CATALOG, либо- явно задокументировать/аннотировать параметр как
@NonNullи гарантировать, что парсер никогда не передастnull.Можно, например:
public static CodeSeries valueByName(String string) { if (string == null) { return WHOLE_CATALOG; } return KEYS.getOrDefault(string.toLowerCase(Locale.ROOT), WHOLE_CATALOG); }Also applies to: 53-81
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
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/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_3_24/Configuration_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/**
📒 Files selected for processing (5)
.gitignore(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/Catalog.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/mdo/support/CodeSeries.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/ExtendXStream.java(2 hunks)src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- src/test/java/com/github/_1c_syntax/bsl/mdo/CatalogTest.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). (9)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (2)
src/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/ExtendXStream.java (1)
34-34: Регистрация конвертера дляCodeSeriesсогласована с остальными enum’амиИмпорт и
registerConverter(new EnumConverter<>(CodeSeries.class));добавлены в том же стиле, что и для других перечислений, инфраструктура XStream выглядит непротиворечиво.При пробеге по тестам уже должно покрываться чтение/запись
codeSeries, но на всякий случай проверьте, что есть хотя бы один тест, читающий/пишущий это поле из XML/MDO.Also applies to: 257-278
src/main/java/com/github/_1c_syntax/bsl/mdo/Catalog.java (1)
27-27: Based on my verification, I have confirmed the semantic foundation of this review comment:1C
CheckUniqueSemantics (Verified):
true= platform performs automatic uniqueness checksfalse= no automatic uniqueness checkThe review comment correctly identifies a semantic issue: if the JavaDoc states that
falseenables uniqueness checking, that contradicts the established 1C semantics wherefalsemeans checking is disabled.However, I cannot directly access the repository to verify:
- The exact wording of the current JavaDoc
- The actual default values in the implementation
- Test expectations
- The specific PR requirements
Ясно, что семантика
CheckUniqueв 1C строго определена:true= проверка включена,false= проверка отключена. Если JavaDoc указывает обратное (чтоfalseвключает проверку), это действительно противоречие, которое может привести к ошибкам в API потребителей.Необходимо вручную проверить в коде:
- Текущую формулировку JavaDoc для
checkUnique- Фактический default value (
trueилиfalse)- Соответствие этого default требованиям из PR-описания
- Тесты, проверяющие поведение
Уточнить правильную семантику согласно 1С и привести в соответствие JavaDoc, default value и тесты
2 participants
Описание
Реализовано добавление свойств
CheckUniqueиCodeSeriesв классCatalogВнесенные изменения:
Добавлено свойство
checkUniqueв классCatalog:booleantrueСоздан enum
CodeSeriesдля типизированного представления серии кодов справочника:WHOLE_CATALOG("WholeCatalog") - весь справочникWITHIN_SUBORDINATION("WithinSubordination") - в пределах подчиненияWITHIN_OWNER_SUBORDINATION("WithinOwnerSubordination") - в пределах подчинения владельцуvalueByName()возвращаетWHOLE_CATALOGпо умолчанию, если значение не найденоДобавлено свойство
codeSeriesв классCatalog:CodeSeriesCodeSeries.WHOLE_CATALOGWHOLE_CATALOGЗарегистрирован конвертер для
CodeSeriesвExtendXStream:Обновлены тестовые JSON-файлы:
checkUniqueиcodeSeriesво все тестовые JSON-файлы с корректными значениямиОсобенности реализации:
CheckUniqueиCodeSeriesобязательны и корректно парсятся из XMLCheckUniqueобязательно, полеCodeSeriesопционально — если отсутствует, используется значение по умолчаниюWHOLE_CATALOGTransformationUtils,ExtendXStream) для автоматического маппинга полей без явных изменений в конвертерахСвязанные задачи
Closes #546
Чеклист
Общие
gradlew precommit)Дополнительно
Все тесты проходят успешно. Реализация соответствует требованиям issue #546 и обеспечивает корректную работу с обоими форматами (Designer и EDT).
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.