Добавление регистра для хранения соответствий объектов интегрируемых … #187
Conversation
WalkthroughДанный pull request направлен на расширение функциональности управления интеграциями в информационной системе. Основные изменения включают создание нового информационного регистра для соответствия объектов интегрируемых систем, добавление новых функций для работы с идентификаторами систем, а также обновление версии конфигурации. Изменения затрагивают общие модули, роли, подсистемы и метаданные, что свидетельствует о комплексном подходе к развитию интеграционных возможностей системы. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
src/cf/InformationRegisters/пбп_СоответствиеОбъектовИнтегрируемыхСистем/Ext/ManagerModule.bsl (1)
28-57: Оптимизировать логику обновления записей.В процедуре
ИзменитьЗаписьесть несколько моментов для улучшения:
- Отсутствует транзакционность
- Избыточные проверки на заполненность
Рекомендуется упростить код:
Процедура ИзменитьЗапись(ПараметрыЗаписи) Экспорт + Если НЕ ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда + Возврат; + КонецЕсли; + + НачатьТранзакцию(); + Попытка НаборЗаписей = СоздатьНаборЗаписей(); - - Если ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда - НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ПараметрыЗаписи.ИнтегрируемаяСистема); - КонецЕсли; + НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ПараметрыЗаписи.ИнтегрируемаяСистема); Если ПараметрыЗаписи.Свойство("СсылкаНаОбъект") И ЗначениеЗаполнено(ПараметрыЗаписи.СсылкаНаОбъект) Тогда НаборЗаписей.Отбор.СсылкаНаОбъект.Установить(ПараметрыЗаписи.СсылкаНаОбъект); КонецЕсли; Если ПараметрыЗаписи.Свойство("ИдентификаторСистемы") И ЗначениеЗаполнено(ПараметрыЗаписи.ИдентификаторСистемы) Тогда НаборЗаписей.Отбор.ИдентификаторСистемы.Установить(ПараметрыЗаписи.ИдентификаторСистемы); КонецЕсли; НаборЗаписей.Прочитать(); Если НаборЗаписей.Количество() Тогда Запись = НаборЗаписей[0]; Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл Запись[КлючИЗначение.Ключ] = КлючИЗначение.Значение; КонецЦикла; НаборЗаписей.Записать(); КонецЕсли; + ЗафиксироватьТранзакцию(); + Исключение + ОтменитьТранзакцию(); + ВызватьИсключение; + КонецПопытки; КонецПроцедурыsrc/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl (2)
297-313: Добавить кэширование результатов запроса.Функция
ПолучитьДополнительныеИдентификаторыИзСоответствияОбъектовСистемможет быть оптимизирована:
- Результаты запроса могут кэшироваться
- Отсутствует обработка ошибок
Рекомендуется:
- Добавить кэширование результатов
- Использовать механизм повторных попыток при сбоях
- Добавить индекс по полям (ИнтегрируемаяСистема, СсылкаНаОбъект)
344-356: Улучшить инициализацию таблицы значений.Функция
ИнициализироватьТаблицуСоответствийОбъектовИнтегрируемыхСистемможет быть улучшена:
- Отсутствует документация по структуре таблицы
- Жестко заданные размеры полей
Рекомендуется:
- Добавить константы для размеров полей
- Улучшить документацию структуры таблицы
- Добавить проверку корректности метаданных
src/cf/InformationRegisters/пбп_СоответствиеОбъектовИнтегрируемыхСистем.xml (4)
173-215: Рекомендуется добавить индексирование для ресурса ДополнительныйИдентификаторДля оптимизации поиска по дополнительному идентификатору рекомендуется включить индексирование:
- <Indexing>DontIndex</Indexing> + <Indexing>Index</Indexing>
216-257: Рекомендуется установить измерение ИнтегрируемаяСистема как ведущееДля оптимизации запросов и улучшения производительности рекомендуется:
- <Master>false</Master> + <Master>true</Master>
258-299: Рекомендуется включить индексирование для измерения СсылкаНаОбъектДля оптимизации поиска по ссылке на объект рекомендуется включить индексирование:
- <Indexing>DontIndex</Indexing> + <Indexing>Index</Indexing>
300-345: Рекомендуется включить индексирование для измерения ИдентификаторСистемыДля оптимизации поиска по идентификатору системы рекомендуется включить индексирование:
- <Indexing>DontIndex</Indexing> + <Indexing>Index</Indexing>src/cf/Roles/пбп_РедактированиеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml (2)
13-39: Рассмотрите возможность ограничения прав конфигурацииТекущая настройка предоставляет все режимы главного окна и клиент аналитической системы. Рекомендуется:
- Проанализировать, какие режимы действительно необходимы для работы с регистром соответствий
- Отключить неиспользуемые режимы для соблюдения принципа наименьших привилегий
40-40: Добавьте пустую строку в конец файлаРекомендуется добавить пустую строку в конец файла для соответствия стандартным практикам форматирования.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl(1 hunks)src/cf/CommonModules/пбп_ОбновлениеИнформационнойБазыПБП/Ext/Module.bsl(1 hunks)src/cf/Configuration.xml(4 hunks)src/cf/DefinedTypes/пбп_ОбъектыИнтегрируемыхСистем.xml(1 hunks)src/cf/InformationRegisters/пбп_СоответствиеОбъектовИнтегрируемыхСистем.xml(1 hunks)src/cf/InformationRegisters/пбп_СоответствиеОбъектовИнтегрируемыхСистем/Ext/ManagerModule.bsl(1 hunks)src/cf/Roles/пбп_РедактированиеСоответствияОбъектовИнтегрируемыхСистем.xml(1 hunks)src/cf/Roles/пбп_РедактированиеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml(1 hunks)src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем.xml(1 hunks)src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml(1 hunks)src/cf/Subsystems/пбп_УправлениеИнтеграциями.xml(2 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/cf/Roles/пбп_РедактированиеСоответствияОбъектовИнтегрируемыхСистем.xml
- src/cf/DefinedTypes/пбп_ОбъектыИнтегрируемыхСистем.xml
- src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем.xml
- src/cf/CommonModules/пбп_ОбновлениеИнформационнойБазыПБП/Ext/Module.bsl
🔇 Additional comments (5)
src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml (1)
13-39: Проверить необходимость прав на конфигурацию.Обе роли включают полный набор прав на работу с основным окном:
- Возможно избыточное предоставление прав
- Отсутствует документация по необходимости каждого права
Рекомендуется проверить необходимость всех прав на конфигурацию:
Also applies to: 25-51
src/cf/Subsystems/пбп_УправлениеИнтеграциями.xml (1)
Line range hint
26-55: Корректная структура подсистемы!Логичное расширение подсистемы управления интеграциями с добавлением всех необходимых компонентов:
- Роли для управления доступом
- Общие картинки для UI
- Шаблоны для работы с внешними системами
- Регистр соответствия объектов
src/cf/Configuration.xml (1)
60-60: Корректное обновление конфигурации!Изменения в конфигурации логично дополняют функциональность:
- Обновлена версия конфигурации
- Добавлены роли для управления доступом к регистру соответствий
- Добавлен определяемый тип для объектов интегрируемых систем
- Зарегистрирован новый регистр сведений
Also applies to: 266-267, 273-274, 339-340, 364-364
src/cf/Roles/пбп_РедактированиеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml (2)
1-2: Корректное объявление XML и пространств имён!Заголовок XML файла содержит все необходимые объявления пространств имён и корректную версию схемы.
3-5: Проверьте настройки безопасностиТекущие настройки:
- Новые объекты не получают права автоматически (безопасно)
- Атрибуты получают права по умолчанию (требует внимания)
- Дочерние объекты не имеют независимых прав (ограничительно)
Убедитесь, что автоматическое назначение прав атрибутам (
setForAttributesByDefault="true") соответствует требованиям безопасности вашей системы.
| Процедура ДобавитьЗапись(ПараметрыЗаписи) Экспорт | ||
|
|
||
| МенеджерЗаписи = СоздатьМенеджерЗаписи(); | ||
| ЗаполнитьЗначенияСвойств(МенеджерЗаписи, ПараметрыЗаписи); | ||
|
|
||
| МенеджерЗаписи.Записать(); | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Добавить проверку параметров и обработку ошибок.
Процедура ДобавитьЗапись требует улучшения обработки ошибок:
- Отсутствует проверка обязательных параметров
- Нет обработки потенциальных исключений при записи
Предлагаю следующие изменения:
Процедура ДобавитьЗапись(ПараметрыЗаписи) Экспорт
+ Если НЕ ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда
+ ВызватьИсключение НСтр("ru='Не указана интегрируемая система'");
+ КонецЕсли;
+
+ Попытка
МенеджерЗаписи = СоздатьМенеджерЗаписи();
ЗаполнитьЗначенияСвойств(МенеджерЗаписи, ПараметрыЗаписи);
МенеджерЗаписи.Записать();
+ Исключение
+ ЗаписьЖурналаРегистрации(НСтр("ru='Ошибка добавления записи соответствия'"),
+ УровеньЖурналаРегистрации.Ошибка,,, ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()));
+ ВызватьИсключение;
+ КонецПопытки;
КонецПроцедуры📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Процедура ДобавитьЗапись(ПараметрыЗаписи) Экспорт | |
| МенеджерЗаписи = СоздатьМенеджерЗаписи(); | |
| ЗаполнитьЗначенияСвойств(МенеджерЗаписи, ПараметрыЗаписи); | |
| МенеджерЗаписи.Записать(); | |
| КонецПроцедуры | |
| Процедура ДобавитьЗапись(ПараметрыЗаписи) Экспорт | |
| Если НЕ ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда | |
| ВызватьИсключение НСтр("ru='Не указана интегрируемая система'"); | |
| КонецЕсли; | |
| Попытка | |
| МенеджерЗаписи = СоздатьМенеджерЗаписи(); | |
| ЗаполнитьЗначенияСвойств(МенеджерЗаписи, ПараметрыЗаписи); | |
| МенеджерЗаписи.Записать(); | |
| Исключение | |
| ЗаписьЖурналаРегистрации(НСтр("ru='Ошибка добавления записи соответствия'"), | |
| УровеньЖурналаРегистрации.Ошибка,,, ПодробноеПредставлениеОшибки(ИнформацияОбОшибке())); | |
| ВызватьИсключение; | |
| КонецПопытки; | |
| КонецПроцедуры |
| Процедура УдалитьЗаписи(ПараметрыЗаписи) Экспорт | ||
|
|
||
| НаборЗаписей = СоздатьНаборЗаписей(); | ||
| Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл | ||
| Если Не ЗначениеЗаполнено(КлючИЗначение.Значение) Тогда | ||
| Продолжить; | ||
| КонецЕсли; | ||
|
|
||
| НаборЗаписей.Отбор[КлючИЗначение.Ключ].Установить(КлючИЗначение.Значение); | ||
| КонецЦикла; | ||
|
|
||
| НаборЗаписей.Очистить(); | ||
| НаборЗаписей.Записать(); | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Добавить проверку параметров и транзакционность.
В процедуре УдалитьЗаписи отсутствуют важные механизмы:
- Проверка входных параметров
- Транзакционность
- Логирование операций удаления
Предлагаю следующие улучшения:
Процедура УдалитьЗаписи(ПараметрыЗаписи) Экспорт
+ Если ТипЗнч(ПараметрыЗаписи) <> Тип("Структура") Тогда
+ ВызватьИсключение НСтр("ru='Некорректный тип параметра'");
+ КонецЕсли;
+
+ НачатьТранзакцию();
+ Попытка
НаборЗаписей = СоздатьНаборЗаписей();
Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл
Если Не ЗначениеЗаполнено(КлючИЗначение.Значение) Тогда
Продолжить;
КонецЕсли;
НаборЗаписей.Отбор[КлючИЗначение.Ключ].Установить(КлючИЗначение.Значение);
КонецЦикла;
НаборЗаписей.Очистить();
НаборЗаписей.Записать();
+
+ ЗаписьЖурналаРегистрации(НСтр("ru='Удаление записей соответствия'"),
+ УровеньЖурналаРегистрации.Информация,,,
+ "Параметры: " + СтрСоединить(ПараметрыЗаписи));
+
+ ЗафиксироватьТранзакцию();
+ Исключение
+ ОтменитьТранзакцию();
+ ВызватьИсключение;
+ КонецПопытки;
КонецПроцедуры📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Процедура УдалитьЗаписи(ПараметрыЗаписи) Экспорт | |
| НаборЗаписей = СоздатьНаборЗаписей(); | |
| Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл | |
| Если Не ЗначениеЗаполнено(КлючИЗначение.Значение) Тогда | |
| Продолжить; | |
| КонецЕсли; | |
| НаборЗаписей.Отбор[КлючИЗначение.Ключ].Установить(КлючИЗначение.Значение); | |
| КонецЦикла; | |
| НаборЗаписей.Очистить(); | |
| НаборЗаписей.Записать(); | |
| КонецПроцедуры | |
| Процедура УдалитьЗаписи(ПараметрыЗаписи) Экспорт | |
| Если ТипЗнч(ПараметрыЗаписи) <> Тип("Структура") Тогда | |
| ВызватьИсключение НСтр("ru='Некорректный тип параметра'"); | |
| КонецЕсли; | |
| НачатьТранзакцию(); | |
| Попытка | |
| НаборЗаписей = СоздатьНаборЗаписей(); | |
| Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл | |
| Если Не ЗначениеЗаполнено(КлючИЗначение.Значение) Тогда | |
| Продолжить; | |
| КонецЕсли; | |
| НаборЗаписей.Отбор[КлючИЗначение.Ключ].Установить(КлючИЗначение.Значение); | |
| КонецЦикла; | |
| НаборЗаписей.Очистить(); | |
| НаборЗаписей.Записать(); | |
| ЗаписьЖурналаРегистрации(НСтр("ru='Удаление записей соответствия'"), | |
| УровеньЖурналаРегистрации.Информация,,, | |
| "Параметры: " + СтрСоединить(ПараметрыЗаписи)); | |
| ЗафиксироватьТранзакцию(); | |
| Исключение | |
| ОтменитьТранзакцию(); | |
| ВызватьИсключение; | |
| КонецПопытки; | |
| КонецПроцедуры |
| Процедура ДобавитьСоответствияОбъектовИнтегрируемыхСистем(ИнтегрируемаяСистема, ТаблицаСоответствий) Экспорт | ||
|
|
||
| НаборЗаписей = РегистрыСведений.пбп_СоответствиеОбъектовИнтегрируемыхСистем.СоздатьНаборЗаписей(); | ||
| НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ИнтегрируемаяСистема); | ||
| НаборЗаписей.Прочитать(); | ||
|
|
||
| Для Каждого СтрокаСоответствие Из ТаблицаСоответствий Цикл | ||
| НоваяЗапись = НаборЗаписей.Добавить(); | ||
| НоваяЗапись.ИнтегрируемаяСистема = ИнтегрируемаяСистема; | ||
| НоваяЗапись.СсылкаНаОбъект = СтрокаСоответствие.СсылкаНаОбъект; | ||
| НоваяЗапись.ИдентификаторСистемы = СтрокаСоответствие.ИдентификаторСистемы; | ||
| НоваяЗапись.ДополнительныйИдентификатор = СтрокаСоответствие.ДополнительныйИдентификатор; | ||
| КонецЦикла; | ||
|
|
||
| НаборЗаписей.Записать(); | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Добавить контроль блокировок и оптимизировать транзакции.
Процедура ДобавитьСоответствияОбъектовИнтегрируемыхСистем требует улучшения:
- Отсутствует контроль блокировок
- Нет обработки конкурентных изменений
Рекомендуется:
- Добавить управление блокировками
- Реализовать механизм обработки конкурентных изменений
- Оптимизировать размер транзакции при большом количестве записей
There was a problem hiding this comment.
- Нет обработки конкурентных изменений
Это не требуется, так как запись набора происходит с замещением
There was a problem hiding this comment.
@thvvmas Вы правы, я ошибся. Запись набора записей регистра сведений с замещением действительно обеспечивает атомарность операции.
✏️ Learnings added
Learnt from: thvvmas
PR: firstBitSportivnaya/PSSL#187
File: src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl:321-337
Timestamp: 2025-01-13T14:59:32.116Z
Learning: В процедуре ДобавитьСоответствияОбъектовИнтегрируемыхСистем регистра сведений пбп_СоответствиеОбъектовИнтегрируемыхСистем не требуется дополнительная обработка конкурентных изменений, так как запись набора записей происходит с замещением, что обеспечивает атомарность операции на уровне платформы 1С.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
- Добавить управление блокировками
Можешь предложить варианты реализации?
There was a problem hiding this comment.
@thvvmas Для управления блокировками в данном случае рекомендую использовать метод УстановитьБлокировкуДанныхРегистраСведений(). Вот пример реализации:
Процедура ДобавитьСоответствияОбъектовИнтегрируемыхСистем(ИнтегрируемаяСистема, ТаблицаСоответствий) Экспорт
Блокировка = Новый БлокировкаДанных;
ЭлементБлокировки = Блокировка.Добавить("РегистрСведений.пбп_СоответствиеОбъектовИнтегрируемыхСистем");
ЭлементБлокировки.УстановитьЗначение("ИнтегрируемаяСистема", ИнтегрируемаяСистема);
Блокировка.Заблокировать();
НаборЗаписей = РегистрыСведений.пбп_СоответствиеОбъектовИнтегрируемыхСистем.СоздатьНаборЗаписей();
НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ИнтегрируемаяСистема);
НаборЗаписей.Прочитать();
Для Каждого СтрокаСоответствие Из ТаблицаСоответствий Цикл
НоваяЗапись = НаборЗаписей.Добавить();
НоваяЗапись.ИнтегрируемаяСистема = ИнтегрируемаяСистема;
НоваяЗапись.СсылкаНаОбъект = СтрокаСоответствие.СсылкаНаОбъект;
НоваяЗапись.ИдентификаторСистемы = СтрокаСоответствие.ИдентификаторСистемы;
НоваяЗапись.ДополнительныйИдентификатор = СтрокаСоответствие.ДополнительныйИдентификатор;
КонецЦикла;
НаборЗаписей.Записать();
КонецПроцедурыЭто обеспечит:
- Эксклюзивную блокировку записей регистра для конкретной интегрируемой системы
- Предотвращение параллельных изменений во время записи
- Автоматическое снятие блокировки после завершения процедуры
There was a problem hiding this comment.
3. Оптимизировать размер транзакции при большом количестве записей
Как ты предлагаешь решить эту проблему?
There was a problem hiding this comment.
@thvvmas Для оптимизации размера транзакции предлагаю разбить обработку на пакеты. Вот пример реализации:
Процедура ДобавитьСоответствияОбъектовИнтегрируемыхСистем(ИнтегрируемаяСистема, ТаблицаСоответствий) Экспорт
РазмерПакета = 1000; // Оптимальный размер пакета
КоличествоЗаписей = ТаблицаСоответствий.Количество();
Для НомерПакета = 0 По Цел(КоличествоЗаписей / РазмерПакета) Цикл
НачалоПакета = НомерПакета * РазмерПакета;
КонецПакета = Мин(НачалоПакета + РазмерПакета - 1, КоличествоЗаписей - 1);
Блокировка = Новый БлокировкаДанных;
ЭлементБлокировки = Блокировка.Добавить("РегистрСведений.пбп_СоответствиеОбъектовИнтегрируемыхСистем");
ЭлементБлокировки.УстановитьЗначение("ИнтегрируемаяСистема", ИнтегрируемаяСистема);
НачатьТранзакцию();
Попытка
Блокировка.Заблокировать();
НаборЗаписей = РегистрыСведений.пбп_СоответствиеОбъектовИнтегрируемыхСистем.СоздатьНаборЗаписей();
НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ИнтегрируемаяСистема);
НаборЗаписей.Прочитать();
Для Индекс = НачалоПакета По КонецПакета Цикл
СтрокаСоответствие = ТаблицаСоответствий[Индекс];
НоваяЗапись = НаборЗаписей.Добавить();
НоваяЗапись.ИнтегрируемаяСистема = ИнтегрируемаяСистема;
НоваяЗапись.СсылкаНаОбъект = СтрокаСоответствие.СсылкаНаОбъект;
НоваяЗапись.ИдентификаторСистемы = СтрокаСоответствие.ИдентификаторСистемы;
НоваяЗапись.ДополнительныйИдентификатор = СтрокаСоответствие.ДополнительныйИдентификатор;
КонецЦикла;
НаборЗаписей.Записать();
ЗафиксироватьТранзакцию();
Исключение
ОтменитьТранзакцию();
ВызватьИсключение;
КонецПопытки;
КонецЦикла;
КонецПроцедурыОсновные улучшения:
- Обработка данных пакетами по 1000 записей
- Каждый пакет в отдельной транзакции
- Корректная обработка ошибок с откатом транзакции
- Сохранение блокировок на уровне пакета
Это позволит:
- Уменьшить нагрузку на память
- Снизить время блокировки данных
- Повысить устойчивость при обработке больших объемов данных
| Функция ПолучитьСсылкиПоИдентификаторамСистем(ИнтегрируемаяСистема, МассивИдентификаторов) Экспорт | ||
|
|
||
| Запрос = Новый Запрос; | ||
| Запрос.УстановитьПараметр("ИнтегрируемаяСистема", ИнтегрируемаяСистема); | ||
| Запрос.УстановитьПараметр("ИдентификаторСистемы", МассивИдентификаторов); | ||
| Запрос.Текст = "ВЫБРАТЬ | ||
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИдентификаторСистемы КАК ИдентификаторСистемы, | ||
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.СсылкаНаОбъект КАК СсылкаНаОбъект | ||
| |ИЗ | ||
| | РегистрСведений.пбп_СоответствиеОбъектовИнтегрируемыхСистем КАК пбп_СоответствиеОбъектовИнтегрируемыхСистем | ||
| |ГДЕ | ||
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИнтегрируемаяСистема = &ИнтегрируемаяСистема | ||
| | И пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИдентификаторСистемы В(&ИдентификаторСистемы)"; | ||
|
|
||
| Возврат Запрос.Выполнить().Выгрузить(); | ||
|
|
||
| КонецФункции |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Оптимизировать запрос и добавить индексы.
Функция ПолучитьСсылкиПоИдентификаторамСистем требует оптимизации:
- Отсутствует проверка входных параметров
- Запрос может быть неэффективным при большом количестве идентификаторов
Рекомендации:
- Добавить проверку параметров
- Добавить индекс по полям (ИнтегрируемаяСистема, ИдентификаторСистемы) для оптимизации поиска
Функция ПолучитьСсылкиПоИдентификаторамСистем(ИнтегрируемаяСистема, МассивИдентификаторов) Экспорт
+ Если НЕ ЗначениеЗаполнено(ИнтегрируемаяСистема) Тогда
+ ВызватьИсключение НСтр("ru='Не указана интегрируемая система'");
+ КонецЕсли;
+
+ Если МассивИдентификаторов = Неопределено
+ ИЛИ МассивИдентификаторов.Количество() = 0 Тогда
+ Возврат Новый ТаблицаЗначений;
+ КонецЕсли;
Запрос = Новый Запрос;
Запрос.УстановитьПараметр("ИнтегрируемаяСистема", ИнтегрируемаяСистема);
Запрос.УстановитьПараметр("ИдентификаторСистемы", МассивИдентификаторов);
// ...остальной код...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Функция ПолучитьСсылкиПоИдентификаторамСистем(ИнтегрируемаяСистема, МассивИдентификаторов) Экспорт | |
| Запрос = Новый Запрос; | |
| Запрос.УстановитьПараметр("ИнтегрируемаяСистема", ИнтегрируемаяСистема); | |
| Запрос.УстановитьПараметр("ИдентификаторСистемы", МассивИдентификаторов); | |
| Запрос.Текст = "ВЫБРАТЬ | |
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИдентификаторСистемы КАК ИдентификаторСистемы, | |
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.СсылкаНаОбъект КАК СсылкаНаОбъект | |
| |ИЗ | |
| | РегистрСведений.пбп_СоответствиеОбъектовИнтегрируемыхСистем КАК пбп_СоответствиеОбъектовИнтегрируемыхСистем | |
| |ГДЕ | |
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИнтегрируемаяСистема = &ИнтегрируемаяСистема | |
| | И пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИдентификаторСистемы В(&ИдентификаторСистемы)"; | |
| Возврат Запрос.Выполнить().Выгрузить(); | |
| КонецФункции | |
| Функция ПолучитьСсылкиПоИдентификаторамСистем(ИнтегрируемаяСистема, МассивИдентификаторов) Экспорт | |
| Если НЕ ЗначениеЗаполнено(ИнтегрируемаяСистема) Тогда | |
| ВызватьИсключение НСтр("ru='Не указана интегрируемая система'"); | |
| КонецЕсли; | |
| Если МассивИдентификаторов = Неопределено | |
| ИЛИ МассивИдентификаторов.Количество() = 0 Тогда | |
| Возврат Новый ТаблицаЗначений; | |
| КонецЕсли; | |
| Запрос = Новый Запрос; | |
| Запрос.УстановитьПараметр("ИнтегрируемаяСистема", ИнтегрируемаяСистема); | |
| Запрос.УстановитьПараметр("ИдентификаторСистемы", МассивИдентификаторов); | |
| Запрос.Текст = "ВЫБРАТЬ | |
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИдентификаторСистемы КАК ИдентификаторСистемы, | |
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.СсылкаНаОбъект КАК СсылкаНаОбъект | |
| |ИЗ | |
| | РегистрСведений.пбп_СоответствиеОбъектовИнтегрируемыхСистем КАК пбп_СоответствиеОбъектовИнтегрируемыхСистем | |
| |ГДЕ | |
| | пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИнтегрируемаяСистема = &ИнтегрируемаяСистема | |
| | И пбп_СоответствиеОбъектовИнтегрируемыхСистем.ИдентификаторСистемы В(&ИдентификаторСистемы)"; | |
| Возврат Запрос.Выполнить().Выгрузить(); | |
| КонецФункции |
There was a problem hiding this comment.
2. Добавить индекс по полям (ИнтегрируемаяСистема, ИдентификаторСистемы) для оптимизации поиска
В данном случае использование индексации избыточно, потому что индексируемые поля не участвуют в последующей обработке в запросе. Индексы необходимы для большого объема данных, если поля участвуют в соединении временных таблиц, условиях виртуальных таблиц и так далее
There was a problem hiding this comment.
@thvvmas Thank you for the clarification about index optimization. I understand now that indexes would be excessive in this case since the fields are only used in WHERE conditions.
✏️ Learnings added
Learnt from: thvvmas
PR: firstBitSportivnaya/PSSL#187
File: src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl:270-286
Timestamp: 2025-01-13T15:27:11.759Z
Learning: In 1C:Enterprise, indexes should only be added when:
1. There is a large volume of data
2. The fields are used in table joins
3. The fields are used in virtual table conditions
Adding indexes just for WHERE clause filtering without subsequent processing is excessive and not recommended.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <Rights xmlns="http://v8.1c.ru/8.2/roles" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="Rights" version="2.16"> | ||
| <setForNewObjects>false</setForNewObjects> | ||
| <setForAttributesByDefault>true</setForAttributesByDefault> | ||
| <independentRightsOfChildObjects>false</independentRightsOfChildObjects> | ||
| <object> | ||
| <name>InformationRegister.пбп_СоответствиеОбъектовИнтегрируемыхСистем</name> | ||
| <right> | ||
| <name>Read</name> | ||
| <value>true</value> | ||
| </right> | ||
| <right> | ||
| <name>Update</name> | ||
| <value>true</value> | ||
| </right> | ||
| <right> | ||
| <name>View</name> | ||
| <value>true</value> | ||
| </right> | ||
| <right> | ||
| <name>Edit</name> | ||
| <value>true</value> | ||
| </right> | ||
| </object> | ||
| <object> | ||
| <name>Configuration.ПроектнаяБиблиотекаПодсистем</name> | ||
| <right> | ||
| <name>MainWindowModeNormal</name> | ||
| <value>true</value> | ||
| </right> | ||
| <right> | ||
| <name>MainWindowModeWorkplace</name> | ||
| <value>true</value> | ||
| </right> | ||
| <right> | ||
| <name>MainWindowModeEmbeddedWorkplace</name> | ||
| <value>true</value> | ||
| </right> | ||
| <right> | ||
| <name>MainWindowModeFullscreenWorkplace</name> |
There was a problem hiding this comment.
Проверить разграничение прав доступа.
Анализ прав доступа для обеих ролей выявил следующие моменты:
- Роль редактирования имеет минимально необходимый набор прав
- Роль чтения включает избыточные права (Update, Edit)
Необходимо:
- Удалить избыточные права из роли чтения
- Добавить право на удаление записей для роли редактирования
| <object> | ||
| <name>InformationRegister.пбп_СоответствиеОбъектовИнтегрируемыхСистем</name> | ||
| <right> | ||
| <name>Read</name> | ||
| <value>true</value> | ||
| </right> | ||
| </object> |
There was a problem hiding this comment.
Несоответствие прав назначению роли
Роль предназначена для редактирования соответствий объектов, но предоставляет только право на чтение регистра пбп_СоответствиеОбъектовИнтегрируемыхСистем. Отсутствуют необходимые права:
- Update (Изменение)
- Insert (Добавление)
- Delete (Удаление)
Добавьте недостающие права:
<object>
<name>InformationRegister.пбп_СоответствиеОбъектовИнтегрируемыхСистем</name>
<right>
<name>Read</name>
<value>true</value>
</right>
+ <right>
+ <name>Update</name>
+ <value>true</value>
+ </right>
+ <right>
+ <name>Insert</name>
+ <value>true</value>
+ </right>
+ <right>
+ <name>Delete</name>
+ <value>true</value>
+ </right>
</object>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <object> | |
| <name>InformationRegister.пбп_СоответствиеОбъектовИнтегрируемыхСистем</name> | |
| <right> | |
| <name>Read</name> | |
| <value>true</value> | |
| </right> | |
| </object> | |
| <object> | |
| <name>InformationRegister.пбп_СоответствиеОбъектовИнтегрируемыхСистем</name> | |
| <right> | |
| <name>Read</name> | |
| <value>true</value> | |
| </right> | |
| <right> | |
| <name>Update</name> | |
| <value>true</value> | |
| </right> | |
| <right> | |
| <name>Insert</name> | |
| <value>true</value> | |
| </right> | |
| <right> | |
| <name>Delete</name> | |
| <value>true</value> | |
| </right> | |
| </object> |
This comment has been minimized.
This comment has been minimized.
| <Role>пбп_РедактированиеДокументаДемоКаркас</Role> | ||
| <Role>пбп_РедактированиеНастроекИнтеграции</Role> | ||
| <Role>пбп_РедактированиеНастроекОтбораОбъектов</Role> | ||
| <Role>ИнтерактивноеОткрытиеВнешнихОтчетовИОбработок</Role> |
There was a problem hiding this comment.
Что-то поплыл порядок ролей, проверь пожалуйста (и заодно по другим объектам метаданных)
| // Возвращаемое значение: | ||
| // ТаблицаЗначений - соответствие ИдентификаторСистемы и СсылкаНаОбъект | ||
| // | ||
| Функция ПолучитьСсылкиПоИдентификаторамСистем(ИнтегрируемаяСистема, МассивИдентификаторов) Экспорт |
There was a problem hiding this comment.
Нужно добавить ещё метод для получения единичной ссылки
| @@ -258,6 +258,103 @@ | |||
| Возврат Справочники.пбп_НастройкиИнтеграции.НайтиПоРеквизиту("ИдентификаторНастройки", ИдентификаторНастройки); | |||
| КонецФункции | |||
|
|
|||
| // Получить ссылки в информационной базе по идентификаторам интегрируемой системы | |||
There was a problem hiding this comment.
А почему служебный? Мне кажется это надо в программный интерфейс основного модуля выносить
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/cf/Roles/пбп_РедактированиеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml (1)
6-24:⚠️ Potential issueОтсутствуют критически важные права для редактирования
Для роли "РедактированиеСоответствияОбъектовИнтегрируемыхСистем" необходимы все права на редактирование, но отсутствуют:
- Право на добавление записей (Insert)
- Право на удаление записей (Delete)
Добавьте недостающие права:
<object> <name>InformationRegister.пбп_СоответствиеОбъектовИнтегрируемыхСистем</name> + <right> + <name>Insert</name> + <value>true</value> + </right> + <right> + <name>Delete</name> + <value>true</value> + </right> <right> <name>Read</name> <value>true</value>src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml (1)
6-100:⚠️ Potential issueНесогласованность в конфигурации прав доступа
Обнаружены следующие проблемы:
- Установлено право на чтение регистра (
Read=true), но отключен просмотр всех его атрибутов (View=false)- Такая конфигурация может привести к неожиданному поведению при доступе к данным
Рекомендуемые изменения:
- Установить
View=trueдля всех атрибутов регистра- Оставить
Edit=falseдля обеспечения режима "только чтение"Пример для одного объекта:
<object> <name>InformationRegister.пбп_СоответствиеОбъектовИнтегрируемыхСистем.Dimension.ИнтегрируемаяСистема</name> <right> <name>View</name> - <value>false</value> + <value>true</value> </right> <right> <name>Edit</name> <value>false</value> </right> </object>Аналогичные изменения необходимо применить ко всем объектам в файле.
🧹 Nitpick comments (2)
src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl (1)
332-380: Рассмотрите упрощение процедуры за счет удаления блокировок и пакетной обработкиВ процедуре
ДобавитьСоответствияОбъектовИнтегрируемыхСистемиспользование блокировок данных и разбивки на пакеты может быть избыточным. Согласно ранее полученным знаниям, при записи набора записей с замещением атомарность операции обеспечивается на уровне платформы 1С, и дополнительные блокировки и обработка конкурентных изменений не требуются. Удаление блокировок и пакетной обработки может упростить код и повысить производительность.src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl (1)
23-95: Избегайте дублирования кода между модулямиДобавленные функции в области
РаботаСДаннымидублируют функциональность, имеющуюся в модулепбп_ИнтеграцииСлужебный. Рассмотрите возможность рефакторинга для предотвращения дублирования кода, например, путем объединения общих функций в отдельный модуль или использования существующих функций через экспортированные методы.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl(1 hunks)src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl(1 hunks)src/cf/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl(1 hunks)src/cf/Configuration.xml(4 hunks)src/cf/InformationRegisters/пбп_СоответствиеОбъектовИнтегрируемыхСистем/Ext/ManagerModule.bsl(1 hunks)src/cf/Roles/пбп_РедактированиеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml(1 hunks)src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cf/Configuration.xml
🧰 Additional context used
📓 Learnings (1)
src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl (2)
Learnt from: thvvmas
PR: firstBitSportivnaya/PSSL#187
File: src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl:270-286
Timestamp: 2025-01-13T15:27:11.923Z
Learning: In 1C:Enterprise, indexes should only be added when:
1. There is a large volume of data
2. The fields are used in table joins
3. The fields are used in virtual table conditions
Adding indexes just for WHERE clause filtering without subsequent processing is excessive and not recommended.
Learnt from: thvvmas
PR: firstBitSportivnaya/PSSL#187
File: src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl:321-337
Timestamp: 2025-01-13T14:59:32.507Z
Learning: В процедуре ДобавитьСоответствияОбъектовИнтегрируемыхСистем регистра сведений пбп_СоответствиеОбъектовИнтегрируемыхСистем не требуется дополнительная обработка конкурентных изменений, так как запись набора записей происходит с замещением, что обеспечивает атомарность операции на уровне платформы 1С.
🔇 Additional comments (7)
src/cf/Roles/пбп_РедактированиеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml (2)
1-2: XML-структура корректнаОбъявление XML и пространства имен соответствуют стандартам платформы 1С:Предприятие.
3-5: Проверьте настройки наследования правТекущие настройки означают:
- Новые объекты не получат права автоматически (
setForNewObjects=false)- Атрибуты получат права по умолчанию (
setForAttributesByDefault=true)- Дочерние объекты не имеют независимых прав (
independentRightsOfChildObjects=false)Убедитесь, что такое поведение соответствует требованиям бизнес-логики.
src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml (2)
1-2: XML-структура корректна!Объявление XML и корневой элемент содержат все необходимые атрибуты и пространства имён.
3-5: 🛠️ Refactor suggestionПроверить глобальные настройки прав
Для роли чтения рекомендуется установить
setForAttributesByDefaultв значениеfalse, чтобы предотвратить автоматическое назначение прав на атрибуты. Текущая настройка может привести к непреднамеренному предоставлению избыточных прав.<setForNewObjects>false</setForNewObjects> - <setForAttributesByDefault>true</setForAttributesByDefault> + <setForAttributesByDefault>false</setForAttributesByDefault> <independentRightsOfChildObjects>false</independentRightsOfChildObjects>src/cf/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl (1)
146-146: Проверьте корректность изменения ссылок напбп_ИнтеграцииСерверВ строках 146, 153 и 161 ссылки на
пбп_ИнтеграцииСлужебныйбыли заменены напбп_ИнтеграцииСервер. Пожалуйста, убедитесь, что это изменение согласовано по всему проекту и не приведет к нежелательным последствиям в частях кода, которые могли зависеть от прежних ссылок.Also applies to: 153-153, 161-161
src/cf/InformationRegisters/пбп_СоответствиеОбъектовИнтегрируемыхСистем/Ext/ManagerModule.bsl (2)
1-4: Корректное объявление директив и областей!Правильное использование директивы компилятора и объявление программного интерфейса.
77-108: Отличная реализация процедуры удаления!Процедура содержит все необходимые элементы:
- Валидация входных параметров
- Управление транзакциями
- Обработка ошибок
- Логирование операций
| Процедура ДобавитьЗапись(ПараметрыЗаписи) Экспорт | ||
|
|
||
| Если Не ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда | ||
| ВызватьИсключение НСтр("ru='Не указана интегрируемая система'"); | ||
| КонецЕсли; | ||
|
|
||
| Попытка | ||
| МенеджерЗаписи = СоздатьМенеджерЗаписи(); | ||
| ЗаполнитьЗначенияСвойств(МенеджерЗаписи, ПараметрыЗаписи); | ||
|
|
||
| МенеджерЗаписи.Записать(); | ||
| Исключение | ||
| ТекстСообщения = НСтр("ru='Ошибка добавления записи соответствия'"); | ||
| ЗаписьЖурналаРегистрации(ТекстСообщения, УровеньЖурналаРегистрации.Ошибка, | ||
| , , , ПодробноеПредставлениеОшибки(ИнформацияОбОшибке())); | ||
| ВызватьИсключение; | ||
| КонецПопытки; | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Требуется дополнительная валидация параметров.
Рекомендуется добавить проверку обязательных параметров СсылкаНаОбъект и ИдентификаторСистемы:
Процедура ДобавитьЗапись(ПараметрыЗаписи) Экспорт
Если Не ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда
ВызватьИсключение НСтр("ru='Не указана интегрируемая система'");
КонецЕсли;
+
+ Если Не ЗначениеЗаполнено(ПараметрыЗаписи.СсылкаНаОбъект) Тогда
+ ВызватьИсключение НСтр("ru='Не указана ссылка на объект'");
+ КонецЕсли;
+
+ Если Не ЗначениеЗаполнено(ПараметрыЗаписи.ИдентификаторСистемы) Тогда
+ ВызватьИсключение НСтр("ru='Не указан идентификатор системы'");
+ КонецЕсли;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Процедура ДобавитьЗапись(ПараметрыЗаписи) Экспорт | |
| Если Не ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда | |
| ВызватьИсключение НСтр("ru='Не указана интегрируемая система'"); | |
| КонецЕсли; | |
| Попытка | |
| МенеджерЗаписи = СоздатьМенеджерЗаписи(); | |
| ЗаполнитьЗначенияСвойств(МенеджерЗаписи, ПараметрыЗаписи); | |
| МенеджерЗаписи.Записать(); | |
| Исключение | |
| ТекстСообщения = НСтр("ru='Ошибка добавления записи соответствия'"); | |
| ЗаписьЖурналаРегистрации(ТекстСообщения, УровеньЖурналаРегистрации.Ошибка, | |
| , , , ПодробноеПредставлениеОшибки(ИнформацияОбОшибке())); | |
| ВызватьИсключение; | |
| КонецПопытки; | |
| КонецПроцедуры | |
| Процедура ДобавитьЗапись(ПараметрыЗаписи) Экспорт | |
| Если Не ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда | |
| ВызватьИсключение НСтр("ru='Не указана интегрируемая система'"); | |
| КонецЕсли; | |
| Если Не ЗначениеЗаполнено(ПараметрыЗаписи.СсылкаНаОбъект) Тогда | |
| ВызватьИсключение НСтр("ru='Не указана ссылка на объект'"); | |
| КонецЕсли; | |
| Если Не ЗначениеЗаполнено(ПараметрыЗаписи.ИдентификаторСистемы) Тогда | |
| ВызватьИсключение НСтр("ru='Не указан идентификатор системы'"); | |
| КонецЕсли; | |
| Попытка | |
| МенеджерЗаписи = СоздатьМенеджерЗаписи(); | |
| ЗаполнитьЗначенияСвойств(МенеджерЗаписи, ПараметрыЗаписи); | |
| МенеджерЗаписи.Записать(); | |
| Исключение | |
| ТекстСообщения = НСтр("ru='Ошибка добавления записи соответствия'"); | |
| ЗаписьЖурналаРегистрации(ТекстСообщения, УровеньЖурналаРегистрации.Ошибка, | |
| , , , ПодробноеПредставлениеОшибки(ИнформацияОбОшибке())); | |
| ВызватьИсключение; | |
| КонецПопытки; | |
| КонецПроцедуры |
| Процедура ИзменитьЗапись(ПараметрыЗаписи) Экспорт | ||
|
|
||
| НаборЗаписей = СоздатьНаборЗаписей(); | ||
|
|
||
| Если ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда | ||
| НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ПараметрыЗаписи.ИнтегрируемаяСистема); | ||
| КонецЕсли; | ||
|
|
||
| Если ПараметрыЗаписи.Свойство("СсылкаНаОбъект") | ||
| И ЗначениеЗаполнено(ПараметрыЗаписи.СсылкаНаОбъект) Тогда | ||
| НаборЗаписей.Отбор.СсылкаНаОбъект.Установить(ПараметрыЗаписи.СсылкаНаОбъект); | ||
| КонецЕсли; | ||
|
|
||
| Если ПараметрыЗаписи.Свойство("ИдентификаторСистемы") | ||
| И ЗначениеЗаполнено(ПараметрыЗаписи.ИдентификаторСистемы) Тогда | ||
| НаборЗаписей.Отбор.ИдентификаторСистемы.Установить(ПараметрыЗаписи.ИдентификаторСистемы); | ||
| КонецЕсли; | ||
|
|
||
| НаборЗаписей.Прочитать(); | ||
|
|
||
| Если НаборЗаписей.Количество() Тогда | ||
| Запись = НаборЗаписей[0]; | ||
| Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл | ||
| Запись[КлючИЗначение.Ключ] = КлючИЗначение.Значение; | ||
| КонецЦикла; | ||
|
|
||
| НаборЗаписей.Записать(); | ||
| КонецЕсли; | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
Необходимо улучшить обработку ошибок и логику обновления записей.
Выявлены следующие проблемы:
- Отсутствует обработка ошибок
- Нет управления транзакциями
- При наличии нескольких записей обновляется только первая без предупреждения
- Отсутствует логирование изменений
Предлагаемые изменения:
Процедура ИзменитьЗапись(ПараметрыЗаписи) Экспорт
+ НачатьТранзакцию();
+ Попытка
НаборЗаписей = СоздатьНаборЗаписей();
Если ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда
НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ПараметрыЗаписи.ИнтегрируемаяСистема);
КонецЕсли;
Если ПараметрыЗаписи.Свойство("СсылкаНаОбъект")
И ЗначениеЗаполнено(ПараметрыЗаписи.СсылкаНаОбъект) Тогда
НаборЗаписей.Отбор.СсылкаНаОбъект.Установить(ПараметрыЗаписи.СсылкаНаОбъект);
КонецЕсли;
Если ПараметрыЗаписи.Свойство("ИдентификаторСистемы")
И ЗначениеЗаполнено(ПараметрыЗаписи.ИдентификаторСистемы) Тогда
НаборЗаписей.Отбор.ИдентификаторСистемы.Установить(ПараметрыЗаписи.ИдентификаторСистемы);
КонецЕсли;
НаборЗаписей.Прочитать();
Если НаборЗаписей.Количество() Тогда
+ Если НаборЗаписей.Количество() > 1 Тогда
+ ЗаписьЖурналаРегистрации(НСтр("ru='Найдено несколько записей для обновления'"),
+ УровеньЖурналаРегистрации.Предупреждение,,,
+ "Количество записей: " + НаборЗаписей.Количество());
+ КонецЕсли;
+
Запись = НаборЗаписей[0];
Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл
Запись[КлючИЗначение.Ключ] = КлючИЗначение.Значение;
КонецЦикла;
НаборЗаписей.Записать();
+
+ ЗаписьЖурналаРегистрации(НСтр("ru='Изменение записи соответствия'"),
+ УровеньЖурналаРегистрации.Информация,,,
+ "Параметры: " + СтрСоединить(ПараметрыЗаписи));
+
+ ЗафиксироватьТранзакцию();
КонецЕсли;
+ Исключение
+ ОтменитьТранзакцию();
+ ВызватьИсключение;
+ КонецПопытки;
КонецПроцедуры📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Процедура ИзменитьЗапись(ПараметрыЗаписи) Экспорт | |
| НаборЗаписей = СоздатьНаборЗаписей(); | |
| Если ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда | |
| НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ПараметрыЗаписи.ИнтегрируемаяСистема); | |
| КонецЕсли; | |
| Если ПараметрыЗаписи.Свойство("СсылкаНаОбъект") | |
| И ЗначениеЗаполнено(ПараметрыЗаписи.СсылкаНаОбъект) Тогда | |
| НаборЗаписей.Отбор.СсылкаНаОбъект.Установить(ПараметрыЗаписи.СсылкаНаОбъект); | |
| КонецЕсли; | |
| Если ПараметрыЗаписи.Свойство("ИдентификаторСистемы") | |
| И ЗначениеЗаполнено(ПараметрыЗаписи.ИдентификаторСистемы) Тогда | |
| НаборЗаписей.Отбор.ИдентификаторСистемы.Установить(ПараметрыЗаписи.ИдентификаторСистемы); | |
| КонецЕсли; | |
| НаборЗаписей.Прочитать(); | |
| Если НаборЗаписей.Количество() Тогда | |
| Запись = НаборЗаписей[0]; | |
| Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл | |
| Запись[КлючИЗначение.Ключ] = КлючИЗначение.Значение; | |
| КонецЦикла; | |
| НаборЗаписей.Записать(); | |
| КонецЕсли; | |
| КонецПроцедуры | |
| Процедура ИзменитьЗапись(ПараметрыЗаписи) Экспорт | |
| НачатьТранзакцию(); | |
| Попытка | |
| НаборЗаписей = СоздатьНаборЗаписей(); | |
| Если ЗначениеЗаполнено(ПараметрыЗаписи.ИнтегрируемаяСистема) Тогда | |
| НаборЗаписей.Отбор.ИнтегрируемаяСистема.Установить(ПараметрыЗаписи.ИнтегрируемаяСистема); | |
| КонецЕсли; | |
| Если ПараметрыЗаписи.Свойство("СсылкаНаОбъект") | |
| И ЗначениеЗаполнено(ПараметрыЗаписи.СсылкаНаОбъект) Тогда | |
| НаборЗаписей.Отбор.СсылкаНаОбъект.Установить(ПараметрыЗаписи.СсылкаНаОбъект); | |
| КонецЕсли; | |
| Если ПараметрыЗаписи.Свойство("ИдентификаторСистемы") | |
| И ЗначениеЗаполнено(ПараметрыЗаписи.ИдентификаторСистемы) Тогда | |
| НаборЗаписей.Отбор.ИдентификаторСистемы.Установить(ПараметрыЗаписи.ИдентификаторСистемы); | |
| КонецЕсли; | |
| НаборЗаписей.Прочитать(); | |
| Если НаборЗаписей.Количество() Тогда | |
| Если НаборЗаписей.Количество() > 1 Тогда | |
| ЗаписьЖурналаРегистрации(НСтр("ru='Найдено несколько записей для обновления'"), | |
| УровеньЖурналаРегистрации.Предупреждение,,, | |
| "Количество записей: " + НаборЗаписей.Количество()); | |
| КонецЕсли; | |
| Запись = НаборЗаписей[0]; | |
| Для Каждого КлючИЗначение Из ПараметрыЗаписи Цикл | |
| Запись[КлючИЗначение.Ключ] = КлючИЗначение.Значение; | |
| КонецЦикла; | |
| НаборЗаписей.Записать(); | |
| ЗаписьЖурналаРегистрации(НСтр("ru='Изменение записи соответствия'"), | |
| УровеньЖурналаРегистрации.Информация,,, | |
| "Параметры: " + СтрСоединить(ПараметрыЗаписи)); | |
| ЗафиксироватьТранзакцию(); | |
| КонецЕсли; | |
| Исключение | |
| ОтменитьТранзакцию(); | |
| ВызватьИсключение; | |
| КонецПопытки; | |
| КонецПроцедуры |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl (2)
258-288: Рассмотреть перенос функции в программный интерфейс основного модуля.Текущая реализация корректна, но данная функциональность больше подходит для программного интерфейса основного модуля, а не служебного.
382-398: Вынести длины строк в константы.Рекомендуется вынести магические числа длин строк (100 и 50) в именованные константы на уровне модуля для улучшения поддерживаемости кода.
src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl (3)
25-62: Рассмотреть возможность устранения дублирования кода.Функции
ИнтеграционныйПоток,ИнтегрируемаяСистемаиНастройкаИнтеграцииимеют схожую логику. Рекомендуется создать общий метод для уменьшения дублирования:Функция НайтиПредопределенныйЭлемент(ИмяСправочника, ИдентификаторНастройки) Экспорт Возврат Справочники[ИмяСправочника].НайтиПоРеквизиту("ИдентификаторНастройки", ИдентификаторНастройки); КонецФункции
64-92: Дополнить документацию параметров и возвращаемых значений.В функциях
ПолучитьСсылкиПоИдентификаторамСистем,ПолучитьСсылкуНаОбъектПоИдентификаторуСистемыиПолучитьДополнительныеИдентификаторыИзСоответствияОбъектовСистемрекомендуется:
- Добавить описание параметров
- Добавить описание возвращаемых значений
- Добавить примеры использования
23-94: Добавить описание области РаботаСДанными.Рекомендуется добавить описание назначения области и её общей функциональности перед объявлением:
// Область содержит функции для работы с данными интегрируемых систем: // * Получение предопределенных элементов // * Работа с соответствиями объектов // * Управление идентификаторами
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/images/ОбъектнаяМодельСхемаЗапроса.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
src/cf/CommonModules/пбп_ИнтеграцииСервер/Ext/Module.bsl(1 hunks)src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl(1 hunks)src/cf/CommonModules/пбп_ОбновлениеИнформационнойБазыПБП/Ext/Module.bsl(2 hunks)src/cf/Configuration.xml(5 hunks)src/cf/Roles/пбп_ЧтениеКонстантыИспользоватьПользовательскиеФункции/Ext/Rights.xml(1 hunks)src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/cf/CommonModules/пбп_ОбновлениеИнформационнойБазыПБП/Ext/Module.bsl
- src/cf/Roles/пбп_ЧтениеСоответствияОбъектовИнтегрируемыхСистем/Ext/Rights.xml
- src/cf/Configuration.xml
🧰 Additional context used
📓 Learnings (1)
src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl (2)
Learnt from: thvvmas
PR: firstBitSportivnaya/PSSL#187
File: src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl:270-286
Timestamp: 2025-01-13T15:27:11.923Z
Learning: In 1C:Enterprise, indexes should only be added when:
1. There is a large volume of data
2. The fields are used in table joins
3. The fields are used in virtual table conditions
Adding indexes just for WHERE clause filtering without subsequent processing is excessive and not recommended.
Learnt from: thvvmas
PR: firstBitSportivnaya/PSSL#187
File: src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl:321-337
Timestamp: 2025-01-13T14:59:32.507Z
Learning: В процедуре ДобавитьСоответствияОбъектовИнтегрируемыхСистем регистра сведений пбп_СоответствиеОбъектовИнтегрируемыхСистем не требуется дополнительная обработка конкурентных изменений, так как запись набора записей происходит с замещением, что обеспечивает атомарность операции на уровне платформы 1С.
🔇 Additional comments (4)
src/cf/Roles/пбп_ЧтениеКонстантыИспользоватьПользовательскиеФункции/Ext/Rights.xml (1)
12-15: Корректное добавление права просмотра!Добавление права View для константы пбп_ИспользоватьПользовательскиеФункции логично дополняет существующее право Read, что обеспечивает полный доступ на чтение.
src/cf/CommonModules/пбп_ИнтеграцииСлужебный/Ext/Module.bsl (3)
222-255: Корректная реализация получения ссылок!Функция реализована правильно:
- Проверка входных параметров
- Корректное построение запроса
- Оптимальная работа с регистром без избыточной индексации
290-323: Корректная реализация получения дополнительных идентификаторов!Функция реализована в едином стиле с остальными методами работы с регистром соответствий.
326-380: Оптимальная реализация пакетной обработки!Процедура реализована с учетом всех требований производительности:
- Обработка данных пакетами по 1000 записей
- Корректное управление транзакциями
- Блокировки на уровне пакета
- Обработка ошибок
Analysis Details0 Issues
Coverage and Duplications
Project ID: PSSL |
…систем
Closes #111
Summary by CodeRabbit
Примечания к выпуску
Новые возможности
Улучшения