onWalk is called in alphabetical order about json-schema-validator

[fge]

There is only one reason: because I programmed it this way ;) I guess the code can be changed to preserve key ordering. I did it mostly out of habit, and because syntax checking/keyword validation also did it that way (with the notable exception of format).

HOWEVER: JSON itself has no notion of order. But Jackson uses a LinkedHashMap internally to store object members, which is why the default is indeed to respect the order of appearance.

[kelvinpho]

Is there a way that we can specify an ordering? Perhaps "Alphabetical" or "Preserved" can be options to the walker.

[fge]

It can be done but it will require reimplementation of some pointer collectors and of one part of doWalk() in SchemaWalker. Namely, collection of all pointers for properties, patternProperties, dependencies will need modifying, along with this part of the code:

https://github.com/fge/json-schema-core/blob/master/src/main/java/com/github/fge/jsonschema/walk/SchemaWalker.java#L129

Ideally though, the ordering should be when generating the JSON tree itself -- again, that is a plan for future version of Jackson. Right now it is not easy to do :/

The problem is that the code needs factorization. I think a pair of WalkingConfiguration{,Builder} are in order to satisfy this, paired with a SchemaWalkingFactory. Comments?

[kelvinpho]

I think a Walking(Walker?)Configuration would be a good thing, here are some things that could be considered for the configuration.

1. order in which you walk the properties (alphabetical, reverse alphabetical, key ordering, reverse key ordering)
2. onWalk call on the "old tree" or the "new tree", I noticed that on walk is currently only called on the old tree, and then the new tree's properties. (This was found on the resolving schema walker)

[fge]

Hmm, not sure whether there is an interest to walk either the old or the new tree?

[fge]

There is an important note here: when encountering a JSON Reference, other members of the object should be ignored.

Therefore, if, in ResolvingSchemaWalker, both the old and new tree were walked, this would be a violation of the JSON Reference spec. Not sure we want to go there.

Also, onWalk() should really only be called on the resolved tree. This is a question of inverting these two lines here:

https://github.com/fge/json-schema-core/blob/master/src/main/java/com/github/fge/jsonschema/walk/SchemaWalker.java#L126

I also wonder whether the onTreeChange() (which is raised by the call to resolveTree() line 127) event is actually any useful. I cannot remind why I did it this way, except maybe because I figured out one would may need information from the old tree before switching trees? I don't know. But retrospectively it doesn't sound like a good idea.

[fge]

OK, I think the listener interface needs to change for -core 1.2 (it's too late for 1.0). It also means json-schema-validator will jump to 2.2, since I have other changes planned in the developer API which are incompatible with 2.0.

The events I plan to listen so far are, in order:

• onEnter(JsonPointer ptr): relative push into the tree;
• onTreeChange(SchemaTree oldTree, SchemaTree newTree): tree has changed due to ref resolving;
• onWalk(SchemaTree tree): entering a tree;
• onExit(): pendant of onEnter().

This simplifies listener implementations quite a lot. I still am uneasy as to whether a tree change should be triggered at all. But it doesn't hurt.

Comments?

[kelvinpho]

From what I understand from these changes, if the schema contains a reference, then the reference will be resolved and then onWalk will be called on the "new tree".

The result of this is that onWalk should never return a tree that has a $ref in it unless it is a dangling reference.

[fge]

From what I understand from these changes, if the schema contains a reference, then the reference will be resolved and then onWalk will be called on the "new tree".

Yes, that's it.

The result of this is that onWalk should never return a tree that has a $ref in it unless it is a dangling reference.

Not only dangling. ResolvingSchemaWalker also throws an exception if resolving results in a tree which is either a child or a parent of the tree before resolution. A child because it makes potential subschemas ignored, a parent because it would lead to infinite expansion.

[kelvinpho]

This sounds reasonable to me and should make my listener code much easier. Thanks!

[fge]

OK, some updates. -core 1.1.1 is out but still not there yet. In this version I have modified the listener events so as to reflect the events mentioned above (note that you also .onEnter() the initial tree, that is, the JSON Pointer at that point will be empty).

I have not done the configuration aspect yet. I have some difficulties with how to do it properly, since I'd like to somewhat have a registry of known keywords. One intent is to log more information when walking:

• keywords which are plainly unknown (as warnings);
• keywords which are known, but not entered (as info).

All in all, still some work to do...

[fge]

Closing this one. Tree walking will be in 3.0, it cannot go seamlessly in 2.1.x and 2.2. Though the base of it will certainly be in jackson-coreutils.