[ICAR-pydantic] Circular reference between types and resources because of icarInventoryTransactionType

[jrobichaud]

I detected a circular dependency with icarInventoryTransactionType:

ICAR/types/icarInventoryTransactionType.json

Line 6 in 990e3c0

"$ref": "../resources/icarEventCoreResource.json"

The type icarInventoryTransactionType refers to a resource but it is not supposed to happen.

ICAR Dependency diagram:

flowchart LR
    enums
    resources

    collections
    types
    resources --> types
    resources --> enums
    types --> enums
    collections --> resources
    types --> |'types' should not depend on 'resources'| resources

    linkStyle 4 stroke: red,color: red

Loading

I believe it should be a resource instead.

This affects the ICAR-pydantic when generating the modules since it creates circular imports (when a module import another one and vice versa). (in the meantime, I will assume it is a resource as a workaround)

[cookeac]

You will be hitting this from the ICAR-Pydantic perspective because a module there corresponds roughly to an ADE folder. This doesn't generate a circular dependency in other languages.

An early decision was taken in ADE that resources which would be exposed as concrete schemas (and in APIs) are in the resources folder and any "base" schemas that would be aggregated into concrete schemas would be in the types folder. Maybe we need to adjust that decision to support ICAR-Pydantic.

I would value feedback from other contributors.

[jrobichaud]

icarInventoryTransactionType is the only one causing this issue.

Is that IcarEventCoreResource and IcarResource that should be types then? They are never directly exposed I think, only used for inheritance.

[cookeac]

Re icarInventoryResource and icarResource - that is correct, they should have been placed in the resources folder under the orignal logic. The fact that they were not is a good argument (to me) for moving icarInventoryTransactionType. The bigger issue is that for consistency we would also change the file name to contain "resource" but this would be a breaking change, and should probably be avoided.

[cookeac]

The group resolved to move the file into the resources folder and if possible to rename it to icarInventoryTransactionResource as long as this did not break implementations.

[AlexeyHardCode]

@cookeac The icarInventoryTransactionResource file/model already exists and includes the additional property product. We may need to revisit the dependencies of the following models: icarFeedTransactionResource, icarMedicineTransactionResource, and icarFeedTransactionResource.

[erwinspeybroeck]

Alexey will create a pull request, so we can discuss this issue

[mtech-ashelepaev]

Added PR #598

reference "./resources/icarEventCoreResource.json" has moved from the icarInventoryTransactionType to each transaction resource: icarFeedTransactionResource, icarMedicineTransactionResource, and icarFeedTransactionResource.