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

Relative paths resolving to unit test, not the schema #557

Closed
tekmosis opened this issue Jan 8, 2019 · 4 comments · Fixed by #652
Closed

Relative paths resolving to unit test, not the schema #557

tekmosis opened this issue Jan 8, 2019 · 4 comments · Fixed by #652

Comments

@tekmosis
Copy link

tekmosis commented Jan 8, 2019

When using Laravel's packages we have JSON Schemas in multiple directories. Each one can be referenced and we're hoping to use it with relative paths. This works fine in Laravel with the validation on form request, it uses the relative path to the .schema.json

However with this package the relative path is of the unit test.

@erayd
Copy link
Contributor

erayd commented Jan 8, 2019

Could you provide a concise example please?

Relative paths should resolve relative to their parents schema. If there is no parent schema, then a relative path isn't valid - you must supply an absolute one.

@uncaught
Copy link
Contributor

uncaught commented Jan 25, 2021

Yes, there is a bug in the UriResolver::combineRelativePathWithBasePath - I've written a test:

use JsonSchema\Uri\UriResolver;
use PHPUnit\Framework\Constraint\IsEqual;
use PHPUnit\Framework\TestCase;

class UriResolverTest extends TestCase {

  public function testcombineRelativePathWithBasePath(): void {
    $base = '/var/packages/flow-api/tests/UnitTests/DemoData/../../../schema/Customer/CustomerSchema_latest.json';
    $relative = '../../../schema/UuidSchema.json';
    $combined = UriResolver::combineRelativePathWithBasePath($relative, $base);
    $expectedPath =
      '/var/packages/flow-api/tests/UnitTests/DemoData/../../../schema/Customer/../../../schema/UuidSchema.json';
    $expectedRealPath = '/var/packages/schema/UuidSchema.json';
    self::assertThat(
      $combined,
      self::logicalOr(
        new IsEqual($expectedPath),
        new IsEqual($expectedRealPath)
      )
    );
  }
}

Failed asserting that '/var/packages/flow-api/tests/UnitTests/DemoData/../../schema/UuidSchema.json' is equal to '/var/packages/flow-api/tests/UnitTests/DemoData/../../../schema/Customer/../../../schema/UuidSchema.json' or is equal to '/var/packages/schema/UuidSchema.json'.

I've yet to understand, why the Method combineRelativePathWithBasePath is so complicated. Can't you simply use a dirname($basePath) . "/$relativePath"? And if you want to shorten it, a realpath() on the result, will do.

uncaught pushed a commit to uncaught/json-schema that referenced this issue Jan 25, 2021
uncaught pushed a commit to uncaught/json-schema that referenced this issue Jan 25, 2021
uncaught pushed a commit to uncaught/json-schema that referenced this issue Aug 8, 2021
uncaught added a commit to uncaught/json-schema that referenced this issue Aug 8, 2021
uncaught added a commit to uncaught/json-schema that referenced this issue Aug 8, 2021
uncaught added a commit to uncaught/json-schema that referenced this issue Aug 8, 2021
uncaught added a commit to hcomde/octaved-flow-justinrainbow-json-schema that referenced this issue May 6, 2022
@DannyvdSluijs
Copy link
Collaborator

@tekmosis @uncaught in an attempt to cleanup this repo we are trying to filter the issues and see which ones might be closed. Is it safe to assume this is a rather old issue, which might not be worth pursuing? Feel free to close it yourself with some comments if helpful.

Also @uncaught in #652 you're using the word dirname but the name of the class is UriResolver and although a path on disk is a valid uri we should also consider the broader scope of other uri's. Would you say the PR is still something we should look at or is it not something you're still passionate about?

@uncaught
Copy link
Contributor

uncaught commented Feb 6, 2024

We've been using my fork in production for nearly 2 years now. This is a bug and without the fix our tests fail.

The dirname is just a string operation, it is not limited to file paths and also works for URLs. It doesn't work on the actual file system.

uncaught added a commit to uncaught/json-schema that referenced this issue May 31, 2024
DannyvdSluijs pushed a commit to uncaught/json-schema that referenced this issue Sep 20, 2024
DannyvdSluijs pushed a commit to uncaught/json-schema that referenced this issue Sep 20, 2024
DannyvdSluijs added a commit that referenced this issue Sep 20, 2024
* Fix wrong combined paths when traversing upwards. Fixes #557

* docs: add changelog entry

---------

Co-authored-by: Danny van der Sluijs <[email protected]>
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

Successfully merging a pull request may close this issue.

4 participants