fix: merged schemas reference resolving

[ivan-tymoshenko]

Fix #540

[Eomm]

Did you evaluate the URL class?

[ivan-tymoshenko]

Sorry, I didn't get you.

[Eomm]

The URL class has many convenient methods to calculate if the URL is absolute and relative and compose it.

Looking at this Location class, I have afraid that we are going to replicate the URL class in future.

So I wanted to point it out to reduce the burden you could face by developing it

[climba03003]

I don't think they are the same and URL is really slow.
The usage here only string concatenation.

[Eomm]

what about an absolute ref like schema:app:foo#/definitions/hello?

[ivan-tymoshenko]

Yes, they are working. This PR doesn't change anything for absolute refs. The problem was that when we create merged schema we should have two locations to resolve local refs:

1. We need location inside new "merged" schema in case if we have anyof/oneof .., inside merged schema.
2. We original location to be able to resolve local reference in this schema, but outside of merged schema.

It's hard to explain simply) #480 Fixed the first case, but broke the second. This PR should support both.

[Eomm]

The URL class has many convenient methods to calculate if the URL is absolute and relative and compose it.

Looking at this Location class, I have afraid that we are going to replicate the URL class in future.

So I wanted to point it out to reduce the burden you could face by developing it

[ivan-tymoshenko]

@Eomm I can't replace the hole Location class with an URL, we've got some custom logic here. I can use URL instead of split/join for schemaId and jsonPointer, but I'm not sure that it's a good idea. All the operations we have are quite simple and the URL is very slow in comparison.

[climba03003]

It is currently hard to follow why does it fix the issue and how it is fixed.
Would you mind to add some comment of step flow why it works? and how the usage of mergedJsonPointer?

[ivan-tymoshenko]

@climba03003 @Eomm I will explain with an example. Sorry, it is not simple because the problem is not simple.

{
  $id: 'root',
  type: 'object',
  properties: {
    property1: { type: 'string' },
    property2: {
      type: 'object',
      allOf: [
        {
          properties: {
            property21: { $ref: '#/properties/property1' }
          }
        },
        {
          properties: {
            property22: {
              anyOf: [
                { type: 'string' },
                { type: 'number' }
              ]
            }
          }
        }
      ]
    }
  }
}

1. When we meet property allOf, we create a merged schema and create a new unique id for it.
   

   fast-json-stringify/index.js

   Line 458 in e6b3c5f

   mergedSchema.$id = `merged_${randomUUID()}`

   
   If we meet anyOf/oneOf/if inside the merged schema, we create a reference using the merged schema id.
   property22 will have the next reference: merged_12...223#/properties/property22. To create this reference, we should know the merged schema id.

2. If somewhere inside the allOf schemas we meet a local reference like '#/properties/property1' we have to resolve it. To do this, we should know the original schema reference, which is root in this case.

So that is why we need to keep two schemas ids when we build a serializer inside the allOf property.

[ivan-tymoshenko]

The Location class now has two methods for getting schema id.

1. getSchemaId - you should use it when you want to CREATE a new reference to some place in the schema. For example, for validation.
2. getOriginSchemaId - you should use it when you want to RESOLVE a reference that already exists.

I added corresponding comments in the code.

[mcollina]

Can you move this class into its own file under lib?

[mcollina]

lgtm

[mcollina]

@Eomm @climba03003 @Uzlopak PTAL

[climba03003]

LGTM.
Thanks for the explanation, it's now more clear to me.

[remidewitte]

Thanks. I will test asap :)