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

java grpc validation in different projects #185

Closed
dainiusfigoras opened this issue May 7, 2019 · 8 comments · Fixed by #186
Closed

java grpc validation in different projects #185

dainiusfigoras opened this issue May 7, 2019 · 8 comments · Fixed by #186
Labels
Java Java/JVM language support Question Question about the library

Comments

@dainiusfigoras
Copy link

Hi,

just wanted to ask, if I'm doing it right or maybe there is better way.
I have few projects that are using different proto files, proto-files themself are extracted to separate project/jar and validator generation is run there. It creates validator file per proto file (I think) that contains static method to get ValidatorImpl for class (and static classes for every message that needs to be validated). It works very well in tests, when I know concrete proto message I want to test. But it gets a bit more complicated when I start using grpc and ServerInterceptor.
I found 2 ValidatorIndex implementations - ExplicitValidatorIndex and ReflectiveValidatorIndex. But seems like I can't really use any of them, ReflectiveValidatorIndex tries to find Validator based on requested class name, but it fails because actual validator class is static inner class. And I can't use ExplicitValidatorIndex because there are too many classes I would need manually add.
So I ended up with implementing Validator interface that caches locally Validator and ValidatorImpl :

return new ValidatingServerInterceptor(new ValidatorIndex() {
			private final ConcurrentHashMap<Class, ValidatorImpl> VALIDATOR_IMPL_INDEX = new ConcurrentHashMap();
			private final ConcurrentHashMap<Class, Validator> VALIDATOR_INDEX = new ConcurrentHashMap();

			@Override
			public <T> Validator<T> validatorFor(Class aClass) {
				return (Validator) this.VALIDATOR_INDEX.computeIfAbsent(aClass, (c) -> {
					return (proto) -> {
						this.VALIDATOR_IMPL_INDEX.computeIfAbsent(c, (clazz) -> {
							ValidatorImpl impl = MyApiValidator.validatorFor(aClass);
							if (null == impl) {
								impl = ValidatorImpl.ALWAYS_VALID;
							}
							return impl;
						}).assertValid(proto, this);
					};
				});
			}
		});

And because this looks a bit generic I wander if there is better way and maybe I missed something?

@rmichela
Copy link
Contributor

rmichela commented May 7, 2019

ReflectiveValidatorIndex tries to find Validator based on requested class name, but it fails because actual validator class is static inner class.

I'm not sure I understand why ReflectiveValidatorIndex doesn't work for you. The ValidatingServerInterceptor unit tests have an example of using ReflectiveValidatorIndex with an interceptor. There is also a test harness that uses ReflectiveValidatorIndex to validate a wide variety of protos.

Can you expand a bit on what errors you are seeing when ReflectiveValidatorIndex tries to resolve a validator type? Is there an exception? If you have a sample repo I can look at?

@rodaine rodaine added Java Java/JVM language support Question Question about the library labels May 7, 2019
@dainiusfigoras
Copy link
Author

dainiusfigoras commented May 8, 2019

Thanks for quick comment, that was really nice.
After generating validation code I get "AbtReferenceDataApiValidator" class:

package no.entur.abt.referencedata.exchange.grpc;

public class AbtReferenceDataApiValidator {
	public static io.envoyproxy.pgv.ValidatorImpl validatorFor(Class clazz) {
		
		if (clazz.equals(no.entur.abt.referencedata.exchange.grpc.GetDataRequest.class)) return new GetDataRequestValidator();		
		return null;
	}
        public static class GetDataRequestValidator implements io.envoyproxy.pgv.ValidatorImpl<no.entur.abt.referencedata.exchange.grpc.GetDataRequest> {
		public void assertValid(no.entur.abt.referencedata.exchange.grpc.GetDataRequest proto, io.envoyproxy.pgv.ValidatorIndex index) throws io.envoyproxy.pgv.ValidationException {
.....
		}
	}
}

In configuration class I have

@Bean
@GRpcGlobalInterceptor
public ServerInterceptor protobufValidationInterceptor() {
	return new ValidatingServerInterceptor(new ReflectiveValidatorIndex());
}

GetDataRequest class looks like this:

package no.entur.abt.referencedata.exchange.grpc;

/**
 * Protobuf type {@code no.entur.abt.referencedata.exchange.grpc.GetDataRequest}
 */
public  final class GetDataRequest extends
    com.google.protobuf.GeneratedMessageV3 implements
...

But when "GetDataRequest" message comes "ReflectiveValidatorIndex" is trying to load "no.entur.abt.referencedata.exchange.grpc.GetDataRequestValidator" and of course it fails (because GetDataRequestValidator is inside AbtReferenceDataApiValidator) then it returns Validator.ALWAYS_VALID.

Maybe if generated class would implement some interface or have annotation, it would be possible to search for these factory classes and then find right one (afaik it generates one such class per proto file, that means there will be a lot of them for us, with hundred different messages). Of course not all messages are used in all modules, so now I can just hardcode factory classes for right project, because I know what type of message are used there.

I'm using pga 0.1.0-SNAPSHOT and protoc:3.6.1

@rmichela
Copy link
Contributor

rmichela commented May 8, 2019

Can you attach a redacted version of your proto? Are you using nested types in your proto? I have a hunch the structure of your .proto file matters.

@rmichela
Copy link
Contributor

rmichela commented May 8, 2019

Also, are you using any of the java_package, java_outer_classname, or java_multiple_files options on your proto?

@dainiusfigoras
Copy link
Author

More or less every proto file starts like this:

syntax = "proto3";

option java_multiple_files = true;
package our.java.package.for.protobuf;

import "google/protobuf/timestamp.proto";
import "validate.proto";

and typical messages looks like this:

message LogEntry {
	MultilingualString name = 4;
	google.protobuf.Timestamp date = 6;
}

message SalesTransaction {
	LogEntry _log_entry = 1;
....
}

and some messages contains fields that come from different files as well, but validation seems to work just fine. At least I had no problems so far. I was just a bit uncertain about best way to initialise grpc interceptor.

I ended (so far) with this:

private ExplicitValidatorIndex validatorIndex = new ExplicitValidatorIndex();

private void searchForAllValidators() {
		try {
			ClassLoader cl = Thread.currentThread().getContextClassLoader();
			ClassPath cp = ClassPath.from(cl);

			cp.getAllClasses()
					.stream()
					.filter(c -> c.getName().startsWith("our.java.package.for.protobuf.") && c.getName().endsWith("Validator") && c.getName().contains("$"))
					.forEach(c -> {
						String commandName = c.getName().split("\\$")[1];
						commandName = commandName.substring(0, commandName.length() - 9); // 9 = strlen(Validator)
						String commandClassName = c.getPackageName() + "." + commandName;

						Class validatorImplClass = c.load();
						try {
							ValidatorImpl impl = (ValidatorImpl) validatorImplClass.newInstance();
							validatorIndex.add(cl.loadClass(commandClassName), impl);
						} catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) {
							e.printStackTrace();
						}
					});
		} catch (IOException e) {
			e.printStackTrace();
		}
	}

...

return new ValidatingServerInterceptor(validatorIndex);

For us it should be fine, there's just 1 place that contains package name, where it scans (could be parameter), but we put all proto files in same package (even if they are in different folders).
But if it would be possible to use something directly from library, that would be even better.

@rmichela
Copy link
Contributor

rmichela commented May 8, 2019

Ok. I've identified an issue with reflection and the java_multiple_files = true option.

@LethiferousMoose
Copy link

@rmichela So the Java usage doesn't seem to be documented anywhere. Is it just as simple as adding the ValidatingServerInterceptor to the gRPC server? I was digging through change history which lead me here.

@rmichela
Copy link
Contributor

rmichela commented Jun 20, 2019

Yes. It's that easy.

// Create a validator index that reflectively loads generated validators
ValidatorIndex index = new ReflectiveValidatorIndex();

// Create a gRPC client and server interceptor to automatically validate messages (requires pgv-java-grpc module)
clientStub = clientStub.withInterceptors(new ValidatingClientInterceptor(index));
serverBuilder.addService(ServerInterceptors.intercept(svc, new ValidatingServerInterceptor(index)));

@rmichela rmichela mentioned this issue Jun 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java Java/JVM language support Question Question about the library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants