Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class is not generated #8

Open
gigi opened this issue Dec 14, 2018 · 5 comments
Open

Class is not generated #8

gigi opened this issue Dec 14, 2018 · 5 comments

Comments

@gigi
Copy link

gigi commented Dec 14, 2018

Hi again!
One more unexpected behavior. Can you help, пожалуйста?)

allOf.json

{
  "$id": "allOf.json",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "description": "All of data",
  "allOf": [
    {
      "$ref": "ref.json"
    },
    {
      "required": ["field1"]
    }
  ]
}

ref.json

{
  "$id": "ref.json",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "description": "Ref data",
  "properties": {
    "field1": {
      "type": "object",
      "properties": {
        "field1_sub": {
          "type": "string"
        }
      },
      "required": ["field1_sub"]
    }
  }
}
public function testGenerateAllOf()
{
        $appPath = realpath(__DIR__ . '/../../Tmp') . '/Issue8';
        $appNs = 'Swaggest\PhpCodeBuilder\Tests\Tmp\\Issue8';

        $app = new PhpApp();
        $app->setNamespaceRoot($appNs, '.');

        $schema = Schema::import(__DIR__ . '/../../../resources/allOf.json');
  
        $builder = new PhpBuilder();
        $builder->buildSetters = false;
        $builder->makeEnumConstants = true;

        $builder->classCreatedHook = new ClassHookCallback(
            function (PhpClass $class, $path, $schema) use ($app, $appNs) {
                $class->setNamespace($appNs);
                if ('#' === $path) {
                    $class->setName('Sample'); // Class name for root schema
                }
                $app->addClass($class);
            }
        );


        $builder->getType($schema);

        $app->clearOldFiles($appPath);
        $app->store($appPath);
}

I expect 3 generated classes: RefJson.php, RefJsonField1.php and AllOf.php. But there is only two: RefJson.php and RefJsonField1.php

Maybe something missed in the schemas?

@gigi gigi changed the title Class not generated Class is not generated Dec 14, 2018
@gigi
Copy link
Author

gigi commented Dec 17, 2018

Похоже проблема глубже. Не могу побороть сложные схемы с if и allOf типа

{
  "definitions": {
    "client_platform": {
        "type": "string",
        "enum": [
            "iOS"
        ]
    },
    "device": {
      "type": "object",
      "required": [
        "country",
        "region"
      ],
      "properties": {
        "country": {
          "$ref": "#/definitions/country"
        },
        "region": {
          "$ref": "#/definitions/region"
        }
      }
    },
    "device_ios": {
      "required": [
        "type"
      ],
      "allOf": [
        {
          "$ref": "#/definitions/device"
        },
        {
          "properties": {
            "client_platform": {
              "$ref": "#/definitions/client_platform"
            },
            "type": {
              "type": "string",
              "description": "the type of device",
              "enum": [
                "iphone",
                "ipod",
                "ipad"
              ]
            }
          }
        }
      ]
    },
    "properties": {
      ...
    },
    "allOf": [
      {
        "if": {
          "properties": {
            "device": {
              "properties": {
                "client_platform": {
                  "enum": [
                    "iOS"
                  ]
                }
              }
            }
          }
        },
        "then": {
          "properties": {
            "device": {
              "$ref": "#/definitions/device_ios"
            }
          }
        }
      }
    ]
  }
}

В подобных ситуациях похоже придется делать что-то типа

class DeviceIos extends Device {}

@vearutop
Copy link
Member

Главный случай использования этого генератора - отражение properties на класс PHP для детерминированных схем.
Случаи ветвистой логики валидации без поддерживаются хуже. Я постараюсь исправить генерацию. В этом может помочь более подробное описание ожидаемого результата.

@gigi
Copy link
Author

gigi commented Dec 17, 2018

Можем порассуждать исходя из того, как allOf должен быть описан тут https://github.com/swaggest/php-json-schema.

Т.е. если схема A ссылается на схему B в AllOf, то в конкретном случае надо реализовать какой-то вариант наследования.

Типа такого,

class AllOf extends Ref 
{
    ...
    public static function setUpProperties($properties, Schema $ownerSchema)
    {
        parent::setUpProperties($properties, $ownerSchema)
        $ownerSchema->required[] = 'field1';
    }
    ...
}

Во втором примере с ветвлениями, думаю, можно просто генерировать классы типа

class DeviceIos extends Device {
...
}

с уточняющими логику properties.
А логика формирования самого root-объекта, возможно, должна быть в клиентском коде.

Сейчас в своей архитектуре уже склоняюсь к плоским POPO объектам. Слишком большие затраты получаются на дублирование логики json схемы в php

@vearutop
Copy link
Member

Наследование в пхп ограничивает одним родителем, но жсон схема оперирует композицией, allOf может иметь несколько ссылок.

Возможно было бы удобно структурировать данные древовидно согласно жсон схеме:

/**
 * @property country
 * @property region
 * @property client_platform
 * @property type
 * .....
 **/
class DeviceIos {
   /** @var Device **/
   public $allOf0; 
   
   /** @var DeviceIosAllOf1 **/
   public $allOf1;

   ....
}

Но при этом нужно (?) организовать плоский доступ, возможно с использованием https://github.com/swaggest/php-json-schema#nested-structures.

Как такой вариант?

Композиционные возможности жсон схемы не очень гладко укладываются в модели данных пхп.

В частном случае, когда allOf из двух элементов лишь один из которых референс на объект с пропертями, можно в принципе реализовать схему наследования, но если в будущем добавится еще элемент, свежесгенереный класс будет полностью несовместим с прошлой версией.

@gigi
Copy link
Author

gigi commented Dec 18, 2018

Да, согласен, нужен вариант с композицией

Выглядит норм

class DeviceIos {
   /** @var Device **/
   public $allOf0; 
   
   /** @var DeviceIosAllOf1 **/
   public $allOf1;

   ....
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants