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

Support reader.next(Class) inside a converter #269

Open
AsafMah opened this issue Dec 19, 2023 · 3 comments
Open

Support reader.next(Class) inside a converter #269

AsafMah opened this issue Dec 19, 2023 · 3 comments

Comments

@AsafMah
Copy link

AsafMah commented Dec 19, 2023

I have a json format that is mixed to sometimes have an object and sometimes an array, for example:

{
[1,2,3],
{"a": "b"},
[4]
}

As far as I could tell, there was no standard way to handle it, so I made a converter like this:

        public static RawRow read(JsonReader<?> reader) throws IOException {
            if (reader.wasNull()) return null;

            switch (reader.last()) {
                // OneApiError
                case '{':
                    return reader.next(Error.class);
                // Row (array of values)
                case '[':
                    return new Row(ArrayAnalyzer.Runtime.JSON_READER.read(reader));
                default:
                    throw reader.newParseError("Expecting '{' or '[' for object start");
            }
        }

Now the Array branch works fine.

But the problem is with reader.next() - it expects to start with "{", but this token is already read at this point.
It would be nice to have either a different method on a flag to tell it the object has already started, or maybe a way to revert back one byte.

As a workaround I am doing this, but this is quite ugly:

public static RawRow read(JsonReader<?> reader) throws IOException {
            if (reader.wasNull()) return null;

            switch (reader.last()) {
                // OneApiError
                case '{':
                    reader.getNextToken();
                    return new _RawRow$Error_DslJsonConverter.ObjectFormatConverter(Frame.dslJson).readContent(reader);
                // Row (array of values)
                case '[':
                    return new Row(ArrayAnalyzer.Runtime.JSON_READER.read(reader));
                default:
                    throw reader.newParseError("Expecting '{' or '[' for object start");
            }
        }
@zapov
Copy link
Member

zapov commented Dec 19, 2023

Are you able to setup that static class and initialize Error converter.... eg something like

class RowConverter implements Configuration {
  private static JsonReader.ReadObject<Error> errorReader;
  public void configure(DslJson json) {
    errorReader = json.tryFindReader(Error.class);
  }
  ...
}

@AsafMah
Copy link
Author

AsafMah commented Dec 20, 2023

Sorry, I think I don't understand.

This is the full class:

@CompiledJson()
public class RawRow {
    
    @CompiledJson
    public static class Error extends RawRow {
        @JsonAttribute(name="OneApiErrors")
        private final OneApiError[] oneApiErrors;

        public Error(OneApiError[] oneApiErrors) {
            this.oneApiErrors = oneApiErrors;
        }

        public OneApiError[] getOneApiErrors() {
            return oneApiErrors;
        }
    }
    
    public static class Row extends RawRow {
        private final Object[] values;
        
        public Row(Object[] values) {
            this.values = values;
        }
        
        public Object[] getValues() {
            return values;
        }
    }
    
    @JsonConverter(target = RawRow.class)
    public static abstract class KustoRowConverter {
        public static RawRow read(JsonReader<?> reader) throws IOException {
            if (reader.wasNull()) return null;

            switch (reader.last()) {
                // OneApiError
                case '{':
                    reader.getNextToken();
                    return new _RawRow$Error_DslJsonConverter.ObjectFormatConverter(Frame.dslJson).readContent(reader);
                // Row (array of values)
                case '[':
                    return new Row(ArrayAnalyzer.Runtime.JSON_READER.read(reader));
                default:
                    throw reader.newParseError("Expecting '{' or '[' for object start");
            }
        }
        public static void write(JsonWriter writer, RawRow value) {
            throw new UnsupportedOperationException("Serialization not supported");
        }
    }
}

I do technically have converters for Error and Row, and they do work under a normal scenarios.

The problem is, that inside the KustoRowConverter.read method, the return reader.next(Error.class); fails.
Because of this:

	public final <T> T next(final Class<T> manifest) throws IOException {
		if (manifest == null) throw new IllegalArgumentException("manifest can't be null");
		if (typeLookup == null) throw new ConfigurationException("typeLookup is not defined for this JsonReader. Unable to lookup specified type " + manifest);
		if (this.getNextToken() == 'n') {
			return (T)readNull(manifest);
		}
		final ReadObject<T> reader = typeLookup.tryFindReader(manifest);
		if (reader == null) {
			throw new ConfigurationException("Reader not found for " + manifest + ". Check if reader was registered");
		}
		return reader.read(this);
	}

It reads an extra token before the lookup, so it doesn't work.

@zapov
Copy link
Member

zapov commented Dec 21, 2023

So, reader.next(class) is not going to be changed.

What I'm saying Is that if you don't want to access generated classes there, you have alternative options how to get to the converter.
If you have dslJson as static somewhere you can do

public static abstract class KustoRowConverter {
    private static JsonReader.Reader<Error> errorReader = Frame.dslJson.tryFindReader(Error.class);
    public static RawRow read(JsonReader<?> reader) throws IOException {
        if (reader.wasNull()) return null;
        switch (reader.last()) {
            case '{':
                return errorReader.read(reader);
       ...
}

If you don't have it as a static, you can do what I wrote earlier (set up this class during DslJson initialization)

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