Работа с конфигом в формате JSON #95

khorevaa · 2017-11-09T12:24:07Z

Устал, формировать ручками xml и добавил как альтернативу работу с JSON

artbear

Отлично дополнение, JSON давно просился! Спасибо.

У меня есть некоторые замечания.

artbear · 2017-11-09T12:28:47Z

src/json-config.os

+КонецФункции
+
+
+Функция ВырезатьКомментарииИзТекстаJSON(Знач JsonСтрока)


Давайте вставим вырезку комментариев из JSON сразу в пакет JSON.
Сейчас вырезание дублируется в нескольких пакетах, например, здесь и в ванесса-раннер.
Кажется, еще где-то я юзал вырезание комментов

/cc @nixel2007 @EvilBeaver @pumbaEO @khorevaa

Не против...

Я скорей отрицательно отношусь к комментам, чем положительно в json

Комментарии нужны... без них тяжко

@artbear очень смешно.... oscript-library/json#4

@Stepa86 done

Вечером выпущу релиз json и можно будет поправить данный PR

artbear · 2017-11-09T12:30:59Z

src/json-config.os

+
+	Для Каждого КлючИЗначение Из ЭлементМассива Цикл
+
+		Если КлючИЗначение.Ключ = "name" Тогда


Предлагаю длиннющий список Если КлючИЗначение.Ключ = ??? Тогда
1 занести в спец.соответствие
2 и получать ключ через Ключ = СоответствиеКлючей[ИсходныйКлюч];

Долой множественный switch
Код будет намного проще.

Сделал

artbear · 2017-11-09T12:31:51Z

src/json-config.os

+
+КонецПроцедуры
+
+Функция НекорректнаяСтруктураНастроек()


Предлагаю изменить имя на СтрокаНекорректнаяСтруктураНастроек

Переименовал

artbear · 2017-11-09T12:33:55Z

src/multi-controller.os

+
+Процедура ПрочитатьНастройкиИзФайлаJSON(Знач ФайлНастроек)
+
+		Конфиг = ЗагрузитьСценарий(ОбъединитьПути(ТекущийСценарий().Каталог, "json-config.os"));


Зачем пользоваться таким способом здесь и в тестах ?
Предлагаю явно добавить классы ЧтениеКонфигаXML_Гитсинк и ЧтениеКонфигаJSON_Гитсинк и юзать их в коде и в тестах

Сделал классы

artbear · 2017-11-09T12:39:54Z

src/json-config.os

+
+	Для Каждого КлючИЗначение Из ГлобальныеНастройки Цикл
+
+		Если КлючИЗначение.Ключ = "email-domain" Тогда


Здесь также можно добавить спец.соответствие ключей и расшифровок

Сделал

artbear · 2017-11-09T12:43:04Z

src/json-config.os

+
+КонецФункции // ВырезатьКомментарииИзТекстаJSON()
+
+Процедура ПрочитатьНастройкиРепозитория(ЭлементМассива)


Совершенно непонятно, что же скрывается в параметре ЭлементМассива метода :(
Предлагаю переименовать.

Переименовал

artbear · 2017-11-09T12:44:01Z

src/json-config.os

+		КонецЕсли;
+
+		Если ПустаяСтрока(КлючИЗначение.Значение) и мНастройки.Свойство(Ключ) Тогда
+			КлючИЗначение.Значение = мНастройки[Ключ];


Разве можно менять значение у КлючЗначение?

В 1С это запрещено :(

Предлагаю использовать промежуточную переменную вместо присвоения в КлючИЗначение.Значение

Тем более, что здесь идет неявная работа - фактически этот код меняет содержание переданного параметра из ЭлементаМассива :(
это совсем нехорошо.

artbear · 2017-11-09T12:49:33Z

tests/multi-controller-xml.os

+	Утверждения.ПроверитьРавенство("каталог2", мНастройки.Репозитарии[1].КаталогХранилища1С);
+	Утверждения.ПроверитьРавенство(мНастройки.ПутьGit, мНастройки.Репозитарии[1].ПутьGit);
+	Утверждения.ПроверитьРавенство(мНастройки.ПутьКПлатформе83, мНастройки.Репозитарии[1].ПутьКПлатформе83);
+	Утверждения.ПроверитьРавенство("server.com", мНастройки.Репозитарии[1].ДоменПочтыДляGit);


Вот здесь лучше явно указать
Утверждения.ПроверитьРавенство(мНастройки.ДоменПочтыДляGit, мНастройки.Репозитарии[1].ДоменПочтыДляGit);
как в строках выше
вместо использования "server.com"

Это не надо править т.к. проверка корректная. Проверяется локальная настройка для репо.

У тебя что глобальная, что локальная настройка совпадают :(
Я про это и писал.

Если ты хочешь проверить именно локальную настройку, тогда нужно в конфиг.json установить локальную настройку, например, localhost вместо server.com

И проверить именно локальную настройку, а не глобальную.

Здесь будешь менять?

Предлагаю добавить в настройку репозитория отдельную настройку для почты и убедиться, что локальная настройка переписывает глобальную

Вообще проверки не мои (изначально) но подумаю, хотя на мой взгляд.. можно и так оставить

artbear · 2017-11-09T12:53:11Z

А вообще я лично планировал использовать пакет params для разруливания подобных настроек - и ком.строка, и json, и многое другое легко использовать.

khorevaa · 2017-11-09T14:05:57Z

Вообще тут немного для другого используется... Хотя можно было прикрутить.. Но да ладно....

artbear

Предлагаю упростить код, а не переусложнять его

artbear · 2017-11-09T14:40:36Z

tests/multi-controller-xml.os

@@ -36,7 +38,17 @@

 Функция ПрочитатьТестовыеНастройки()
 	ФайлНастроек = КаталогFixtures() + "/config.xml";
-	ЧтениеКонфига = Новый ЧтениеКонфига();
+	ЧтениеКонфига = Новый ЧтениеКонфигаXMLПакетнойСинхронизации();


У кода

ЧтениеКонфига = Новый ЧтениеКонфигаXMLПакетнойСинхронизации(); ЧтениеКонфига.СоответствиеКлючамИПараметра = Контроллер.СоответствиеКлючамИПараметра();

куча проблем:

введение лишней публичной переменной, которая не особо и нужна

нарушение инкапсуляции класса и т.п.

Предлагаю сделать проще на базе конструктора ПриСозданииОбъекта(Соответствие)

ЧтениеКонфига = Новый ЧтениеКонфигаXMLПакетнойСинхронизации(Контроллер.СоответствиеКлючамИПараметра());

и такое же исправление для ЧтениеКонфигаJSONПакетнойСинхронизации

artbear · 2017-11-09T14:41:25Z

src/core/Классы/КонтроллерПакетнойСинхронизации.os

+Процедура ПрочитатьНастройкиИзФайлаXML(Знач ФайлНастроек)
+
+		Конфиг = Новый ЧтениеКонфигаXMLПакетнойСинхронизации;
+		Конфиг.СоответствиеКлючамИПараметра = СоответствиеКлючамИПараметра();


И здесь код упростится

artbear · 2017-11-09T14:41:29Z

src/core/Классы/КонтроллерПакетнойСинхронизации.os

+Процедура ПрочитатьНастройкиИзФайлаJSON(Знач ФайлНастроек)
+
+		Конфиг = Новый ЧтениеКонфигаJSONПакетнойСинхронизации;
+		Конфиг.СоответствиеКлючамИПараметра = СоответствиеКлючамИПараметра();


И здесь код упростится

artbear

@khorevaa Я выпустил релиз 1.0.0 пакета json с пропуском комментариев

Исправь в PR, плиз, зависимость гитсинка от версии 1.0.0 json
убери код по вырезке комментариев

khorevaa · 2017-11-13T06:04:27Z

Все сделал, сэр.!! @artbear

nixel2007 · 2017-11-13T07:15:14Z

Если переходите на конструкторы классов, надо и версию движка повысить. Они появились только в 1.0.17

artbear

Остались совсем мелкие замечания

artbear · 2017-11-13T08:41:01Z

src/core/Классы/КонтроллерПакетнойСинхронизации.os

+	СоответствиеКлючамИПараметра.Вставить("auto-set-tags", "АвтоматическаяУстановкаТэговПоВерсиям");
+	СоответствиеКлючамИПараметра.Вставить("process-fatform-modules", "ПереименовыватьФайлМодуляОбычнойФормы");
+
+	Возврат СоответствиеКлючамИПараметра;


Лучше возврать ФиксированноеСоответствие, так будет гарантия, что никто случайно не поменяет коллекцию

Да поменяю..

artbear · 2017-11-13T08:43:14Z

tests/multi-controller-xml.os

+	Утверждения.ПроверитьРавенство("каталог2", мНастройки.Репозитарии[1].КаталогХранилища1С);
+	Утверждения.ПроверитьРавенство(мНастройки.ПутьGit, мНастройки.Репозитарии[1].ПутьGit);
+	Утверждения.ПроверитьРавенство(мНастройки.ПутьКПлатформе83, мНастройки.Репозитарии[1].ПутьКПлатформе83);
+	Утверждения.ПроверитьРавенство("server.com", мНастройки.Репозитарии[1].ДоменПочтыДляGit);


Здесь будешь менять?

artbear · 2017-11-13T08:52:43Z

Не прошли 2 теста http://build.oscript.io/job/oscript-library/job/gitsync/job/PR-95/7/console
Искать в логе по

Тестов пройдено: 23 
  
Тестов не пройдено: 2 
 * Тест_ДолженПрочитатьФайлНастроекJSON : <c:\JenkinsOS\workspace\cript-library_gitsync_PR-95-C5NDOESOMYS37JCIRYHHNAO3G34QENYZLYI4PLESTWSL6XI6KHSA\tests\multi-controller-xml.os> 
 * Тест_ДолженПроверитьЧтоВызванОбработчикСинхронизацииJSON : <c:\JenkinsOS\workspace\cript-library_gitsync_PR-95-C5NDOESOMYS37JCIRYHHNAO3G34QENYZLYI4PLESTWSL6XI6KHSA\tests\multi-controller-xml.os>

khorevaa · 2017-11-13T09:04:52Z

Тест №18: Тест_ДолженПрочитатьФайлНастроекJSON


ОШИБКА: Тест: <Тест_ДолженПрочитатьФайлНастроекJSON>

Файл: <c:\JenkinsOS\workspace\cript-library_gitsync_PR-95-C5NDOESOMYS37JCIRYHHNAO3G34QENYZLYI4PLESTWSL6XI6KHSA\tests\multi-controller-xml.os>

Сообщение: {Модуль C:\Program Files (x86)\OneScript\lib\json\src\json.os / Ошибка в строке: 373 / Свойство объекта не обнаружено (LF)}

Пока СтрНайти( ЗначениеБезКомментариев, Символы.LF + Символы.LF ) > 0 Цикл


---------------------------------------------------------


Тест №19: Тест_ДолженПроверитьЧтоВызванОбработчикСинхронизацииJSON


ОШИБКА: Тест: <Тест_ДолженПроверитьЧтоВызванОбработчикСинхронизацииJSON>

Файл: <c:\JenkinsOS\workspace\cript-library_gitsync_PR-95-C5NDOESOMYS37JCIRYHHNAO3G34QENYZLYI4PLESTWSL6XI6KHSA\tests\multi-controller-xml.os>

Сообщение: {Модуль C:\Program Files (x86)\OneScript\lib\json\src\json.os / Ошибка в строке: 373 / Свойство объекта не обнаружено (LF)}

Пока СтрНайти( ЗначениеБезКомментариев, Символы.LF + Символы.LF ) > 0 Цикл

ошибка в пакете json. У меня локально все проходит

nixel2007 · 2017-11-13T17:39:29Z

@khorevaa LF вроде бы в какой-то из версий 1.0.17 чинили. У тебя последний stable семнашки?

khorevaa · 2017-11-14T05:39:26Z

@nixel2007 у меня ночник.. - месячной давности

khorevaa · 2017-11-15T11:20:27Z

@artbear Проверку теста решил не править. Остальных замечаний вроде нет.

khorevaa · 2017-11-20T08:30:30Z

@artbear - Хочется дальше двигаться.. Что с этип PR?

artbear · 2017-11-23T12:08:33Z

@khorevaa LF вроде бы в какой-то из версий 1.0.17 чинили. У тебя последний stable семнашки?

@khorevaa По поводу Символы.LF - у тебя что-то не так с версией

только что проверил на 1.0.19
запустил простейший скрипт с a = Символы.LF + Символы.LF;
ошибки нет

khorevaa · 2017-11-23T12:50:08Z

@artbear Так у меня все хорошо падает на тест сервер у Вас когда ПР проходит тестирование.

artbear · 2017-11-23T13:11:19Z

Точно, там 17 версия.

А обновлять нельзя, т.к в новых версиях есть проблема движка при прогоне фич через 1bdd :(
И если обновить, то большая часть билдов разных библиотек упадет :(

/cc @dmpas @EvilBeaver

artbear · 2017-11-25T12:41:37Z

выпустил новую версию json для работы на нашем CI

khorevaa added 7 commits November 9, 2017 08:51

Исправление преобразования к булево, при не заданном параметре

69f7003

Пустные строчки

5692abc

Добавил обработку параметра при пакетной синхронизации

99a5bf0

Конфигурация в JSON для команды sync -all

3f348ca

Немного справки

e6428f4

еше справка

f629d15

merge develop

52252d6

artbear requested changes Nov 9, 2017

View reviewed changes

artbear self-assigned this Nov 9, 2017

artbear added this to the 2.3.0 milestone Nov 9, 2017

artbear added enhancement tests labels Nov 9, 2017

Много изменений по замечаниям.

0808da9

artbear requested changes Nov 9, 2017

View reviewed changes

artbear requested changes Nov 12, 2017

View reviewed changes

khorevaa added 2 commits November 13, 2017 08:59

Убрал вырезку комментариев. Поправил зависимость

6be879a

Упростил код. согласно ревью.

17a4781

Повымил версию требуемого движка oscript

b7eab06

artbear requested changes Nov 13, 2017

View reviewed changes

Изменение на фиксированное соответсвие.

7f61cfe

artbear removed this from the 2.3.0 milestone Nov 17, 2017

artbear added this to the 2.4.0 milestone Nov 17, 2017

khorevaa mentioned this pull request Nov 22, 2017

WIP: Поддержка работы с плагинами (PR для обсуждения) #100

Closed

artbear approved these changes Nov 23, 2017

View reviewed changes

artbear merged commit f58d2d7 into oscript-library:develop Nov 23, 2017

khorevaa deleted the json-config branch May 29, 2018 07:38

		КонецФункции


		Функция ВырезатьКомментарииИзТекстаJSON(Знач JsonСтрока)


		Для Каждого КлючИЗначение Из ЭлементМассива Цикл

		Если КлючИЗначение.Ключ = "name" Тогда


		КонецПроцедуры

		Функция НекорректнаяСтруктураНастроек()


		Процедура ПрочитатьНастройкиИзФайлаJSON(Знач ФайлНастроек)

		Конфиг = ЗагрузитьСценарий(ОбъединитьПути(ТекущийСценарий().Каталог, "json-config.os"));


		Для Каждого КлючИЗначение Из ГлобальныеНастройки Цикл

		Если КлючИЗначение.Ключ = "email-domain" Тогда


		КонецФункции // ВырезатьКомментарииИзТекстаJSON()

		Процедура ПрочитатьНастройкиРепозитория(ЭлементМассива)

Работа с конфигом в формате JSON #95

Работа с конфигом в формате JSON #95

Uh oh!

Conversation

khorevaa commented Nov 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

artbear left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khorevaa Nov 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

artbear commented Nov 9, 2017

Uh oh!

khorevaa commented Nov 9, 2017

Uh oh!

artbear left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

khorevaa commented Nov 9, 2017 •

edited

Loading

khorevaa Nov 9, 2017 •

edited

Loading

khorevaa commented Nov 14, 2017 •

edited

Loading