[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r265421619
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/SchemaDefinitionBuilder.java
##
@@ -0,0 +1,63 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.api.schema;
+
+import java.util.Map;
+
+/**
+ * Builder to build schema definition {@link SchemaDefinition}.
+ */
+public interface SchemaDefinitionBuilder {
+
+static final String ALWAYS_ALLOW_NULL = "__alwaysAllowNull";
Review comment:
This doesn’t have to be part of the interface. If needed, it should just be
part of implementation
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r265421903
##
File path:
pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/SchemaDefinitionBuilderImpl.java
##
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.impl.schema;
+
+import org.apache.pulsar.client.api.schema.SchemaDefinition;
+import org.apache.pulsar.client.api.schema.SchemaDefinitionBuilder;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Builder to build {@link org.apache.pulsar.client.api.schema.GenericRecord}.
+ */
+public class SchemaDefinitionBuilderImpl implements
SchemaDefinitionBuilder {
+
+/**
+ * the schema definition class
+ */
+private Class clazz;
+/**
+ * The flag of schema type always allow null
+ *
+ * If it's true, will make all of the pojo field generate schema
+ * define default can be null,false default can't be null, but it's
+ * false youcan define the field by yourself by the annotation@Nullable
+ *
+ */
+private boolean alwaysAllowNull = true;
+
+/**
+ * The schema info properties
+ */
+private Map properties = new HashMap<>();
+
+public SchemaDefinitionBuilderImpl(Class clazz){
+this.clazz = clazz;
+}
+
+@Override
+public SchemaDefinitionBuilder withAlwaysAllowNull(boolean
alwaysAllowNull) {
+this.alwaysAllowNull = alwaysAllowNull;
+return this;
+}
+
+@Override
+public SchemaDefinitionBuilder addProperty(String key, String value) {
+this.properties.put(key, value);
+return this;
+}
+
+
+@Override
+public SchemaDefinitionBuilder withProperties(Map
properties) {
+this.properties = properties;
+return this;
+}
+
+@Override
+public SchemaDefinition build() {
+properties.put(ALWAYS_ALLOW_NULL, this.alwaysAllowNull ? "true" :
"false");
+return new SchemaDefinitionImpl<>(clazz, alwaysAllowNull, properties);
Review comment:
I still don’t see why we need to put ALWAYS_ALLOW_NULL in the map when we’re
passing it as flag.
Properties should be reserved to user defined properties and we use the
variable for internal usage.
In any case , we don’t need to set it twice.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264984110
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
##
@@ -167,25 +168,47 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
}
/**
- * Create a Avro schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type by default configuration of the class
*
* @param clazz the POJO class to be used to extract the Avro schema
* @return a Schema instance
*/
static Schema AVRO(Class clazz) {
-return DefaultImplementation.newAvroSchema(clazz);
+return DefaultImplementation.newAvroSchema(new
SchemaDefinition<>(clazz));
}
/**
- * Create a JSON schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type with schema definition
+ *
+ * @param schemaDefinition the definition of the schema
+ * @return a Schema instance
+ */
+static Schema AVRO(SchemaDefinition schemaDefinition) {
+return DefaultImplementation.newAvroSchema(schemaDefinition);
+}
+
+
+/**
+ * Create a JSON schema type by default configuration of the class
*
* @param clazz the POJO class to be used to extract the JSON schema
* @return a Schema instance
*/
static Schema JSON(Class clazz) {
-return DefaultImplementation.newJSONSchema(clazz);
+return DefaultImplementation.newJSONSchema(new
SchemaDefinition<>(clazz));
+}
+
+/**
+ * Create a JSON schema type with schema definition
+ *
+ * @param schemaDefinition the definition of the schema
+ * @return a Schema instance
+ */
+static Schema JSON(SchemaDefinition schemaDefinition) {
Review comment:
A part from having this difference in the allow null, I think there is a
more fundamental difference between JSON and AVRO.
In JSON, the AVRO schema is extracted and used for the validation, but the
JSON serialization is done separately. In this case the POJO and the serialized
data could be different from the Avro schema that was validated with broker.
That's not a problem with Avro since the schema object is used at
serialization time so it doesn't matter which POJO we're passing there that the
result will be correct (or user gets exception).
For this I'm suggesting to avoid adding the option for JSON right now and
defer it until there's a clear use case for it (adding is always easy).
My guess is that this will probably not be needed as users will not already
have a precise Avro schema representation for the data used in other systems.
Rather, it's common practice in most languages to define POJOs for the JSON
data.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264982965
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
##
@@ -167,25 +168,47 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
}
/**
- * Create a Avro schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type by default configuration of the class
*
* @param clazz the POJO class to be used to extract the Avro schema
* @return a Schema instance
*/
static Schema AVRO(Class clazz) {
-return DefaultImplementation.newAvroSchema(clazz);
+return DefaultImplementation.newAvroSchema(new
SchemaDefinition<>(clazz));
}
/**
- * Create a JSON schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type with schema definition
+ *
+ * @param schemaDefinition the definition of the schema
+ * @return a Schema instance
+ */
+static Schema AVRO(SchemaDefinition schemaDefinition) {
Review comment:
> I don't think AvroDefinition is a good name. it is confusing - does it
mean it is for avro schema, or it is for generated the avro formatted schema. I
think SchemaDefinition is much better.
Sure that's probably not a good naming. Though I'd be careful in using
constructor approach since it makes it harder to extend, compared to using
builder pattern.
> if pojo class is omitted, the schema definition is generated from the json
string.
Ok that would make the `Producer` which should still be fine (in the
Avro case) since the avro schema is the one that is really deciding the
serialization.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264963480
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/SchemaDefinition.java
##
@@ -0,0 +1,103 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.api.schema;
+
+import lombok.Data;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A schema definition
+ * {@link org.apache.pulsar.client.api.Schema} for the schema definition value.
+ */
+@Data
+@EqualsAndHashCode
+@ToString
+public class SchemaDefinition {
+
+/**
+ * the schema definition class
+ */
+private final Class clazz;
+/**
+ * The flag of schema type always allow null
+ */
+private boolean alwaysAllowNull = true;
+/**
+ * The schema info properties
+ */
+private Map properties = new HashMap<>();
+
+
+public SchemaDefinition(Class clazz) {
+
+properties.put("__alwaysAllowNull", "true");
Review comment:
`"__alwaysAllowNull"` is used multiple times and it would have to be set as
a constant.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264964644
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
##
@@ -222,9 +245,9 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
/**
* Create a schema instance that automatically deserialize messages
* based on the current topic schema.
- *
+ *
Review comment:
Do not remove the `` as they're used in the generated javadoc.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264964829
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/SchemaDefinition.java
##
@@ -0,0 +1,103 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.api.schema;
+
+import lombok.Data;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A schema definition
+ * {@link org.apache.pulsar.client.api.Schema} for the schema definition value.
+ */
+@Data
+@EqualsAndHashCode
+@ToString
+public class SchemaDefinition {
+
+/**
+ * the schema definition class
+ */
+private final Class clazz;
+/**
+ * The flag of schema type always allow null
+ */
+private boolean alwaysAllowNull = true;
+/**
+ * The schema info properties
+ */
+private Map properties = new HashMap<>();
+
+
+public SchemaDefinition(Class clazz) {
+
+properties.put("__alwaysAllowNull", "true");
+this.clazz = clazz;
+
+}
+
+/**
+ * Set schema whether always allow null or not
Review comment:
We should explain here what are the implications of using
allowNull=true/false in the Avro context and indicate what is the default
behavior.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264963389
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/SchemaDefinition.java
##
@@ -0,0 +1,103 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.api.schema;
+
+import lombok.Data;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A schema definition
+ * {@link org.apache.pulsar.client.api.Schema} for the schema definition value.
+ */
+@Data
Review comment:
We generally avoid using Lombok in public API classes. Even if it the code
is generated at compile time, it will show up when a user look at the API code
in the IDE.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264962895
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
##
@@ -167,25 +168,47 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
}
/**
- * Create a Avro schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type by default configuration of the class
*
* @param clazz the POJO class to be used to extract the Avro schema
* @return a Schema instance
*/
static Schema AVRO(Class clazz) {
-return DefaultImplementation.newAvroSchema(clazz);
+return DefaultImplementation.newAvroSchema(new
SchemaDefinition<>(clazz));
}
/**
- * Create a JSON schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type with schema definition
+ *
+ * @param schemaDefinition the definition of the schema
+ * @return a Schema instance
+ */
+static Schema AVRO(SchemaDefinition schemaDefinition) {
+return DefaultImplementation.newAvroSchema(schemaDefinition);
+}
+
+
+/**
+ * Create a JSON schema type by default configuration of the class
*
* @param clazz the POJO class to be used to extract the JSON schema
* @return a Schema instance
*/
static Schema JSON(Class clazz) {
-return DefaultImplementation.newJSONSchema(clazz);
+return DefaultImplementation.newJSONSchema(new
SchemaDefinition<>(clazz));
+}
+
+/**
+ * Create a JSON schema type with schema definition
+ *
+ * @param schemaDefinition the definition of the schema
+ * @return a Schema instance
+ */
+static Schema JSON(SchemaDefinition schemaDefinition) {
Review comment:
I think that JSON is a bit different compared with AVRO in that it does not
have a canonical schema representation. In AVRO, the schema def is the source
of truth, but for JSON the pojo is typically the source of truth (and people
use annotations to customize the specific fields).
For now I'd prefer to not add it. We can always add it later if there are
good reasons for it (but we'd not be able to take it out once it's in there).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264964562
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
##
@@ -167,25 +168,47 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
}
/**
- * Create a Avro schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type by default configuration of the class
*
* @param clazz the POJO class to be used to extract the Avro schema
* @return a Schema instance
*/
static Schema AVRO(Class clazz) {
-return DefaultImplementation.newAvroSchema(clazz);
+return DefaultImplementation.newAvroSchema(new
SchemaDefinition<>(clazz));
}
/**
- * Create a JSON schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type with schema definition
+ *
+ * @param schemaDefinition the definition of the schema
+ * @return a Schema instance
+ */
+static Schema AVRO(SchemaDefinition schemaDefinition) {
Review comment:
Instead of having the `SchemaDefinition`, what about leaving the pojo and
adding the options like:
```java
Schema.AVRO(MyObject.class,
AvroDefinition.builder().allowNull(false).build());
```
This could be extended in #3766 with:
```java
Schema.AVRO(MyObject.class, AvroDefinition.fromSchema("...json
definition..."));
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264963567
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/schema/SchemaDefinition.java
##
@@ -0,0 +1,103 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.api.schema;
+
+import lombok.Data;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * A schema definition
+ * {@link org.apache.pulsar.client.api.Schema} for the schema definition value.
+ */
+@Data
+@EqualsAndHashCode
+@ToString
+public class SchemaDefinition {
+
+/**
+ * the schema definition class
+ */
+private final Class clazz;
+/**
+ * The flag of schema type always allow null
+ */
+private boolean alwaysAllowNull = true;
+/**
+ * The schema info properties
+ */
+private Map properties = new HashMap<>();
+
+
+public SchemaDefinition(Class clazz) {
+
+properties.put("__alwaysAllowNull", "true");
Review comment:
In any case, I don't understand why we need the property in the map when we
already have the boolean flag.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r263034204
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
##
@@ -167,49 +167,95 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
}
/**
- * Create a Avro schema type by extracting the fields of the specified
class.
+ * Create a allow null Avro schema type by extracting the fields of the
specified class.
*
* @param clazz the POJO class to be used to extract the Avro schema
* @return a Schema instance
*/
static Schema AVRO(Class clazz) {
-return DefaultImplementation.newAvroSchema(clazz);
+return DefaultImplementation.newAvroSchema(clazz, true);
}
/**
- * Create a JSON schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type by extracting the fields of the specified
class.
+ *
+ * @param clazz the POJO class to be used to extract the Avro schema
+ * allowNull the create a allow null or not null Avro schema
+ * @return a Schema instance
+ */
+static Schema AVRO(Class clazz, Boolean allowNull) {
Review comment:
`boolean`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r263034112
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
##
@@ -167,49 +167,95 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
}
/**
- * Create a Avro schema type by extracting the fields of the specified
class.
+ * Create a allow null Avro schema type by extracting the fields of the
specified class.
*
* @param clazz the POJO class to be used to extract the Avro schema
* @return a Schema instance
*/
static Schema AVRO(Class clazz) {
-return DefaultImplementation.newAvroSchema(clazz);
+return DefaultImplementation.newAvroSchema(clazz, true);
}
/**
- * Create a JSON schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type by extracting the fields of the specified
class.
+ *
+ * @param clazz the POJO class to be used to extract the Avro schema
+ * allowNull the create a allow null or not null Avro schema
+ * @return a Schema instance
+ */
+static Schema AVRO(Class clazz, Boolean allowNull) {
+return DefaultImplementation.newAvroSchema(clazz, allowNull);
+}
+
+
+/**
+ * Create a allow null JSON schema type by extracting the fields of the
specified class.
*
* @param clazz the POJO class to be used to extract the JSON schema
* @return a Schema instance
*/
static Schema JSON(Class clazz) {
-return DefaultImplementation.newJSONSchema(clazz);
+return DefaultImplementation.newJSONSchema(clazz, true);
+}
+
+/**
+ * Create a allow null JSON schema type by extracting the fields of the
specified class.
+ *
+ * @param clazz the POJO class to be used to extract the JSON schema
+ * allowNull the create a allow null or not null JSON schema
+ * @return a Schema instance
+ */
+static Schema JSON(Class clazz, Boolean allowNull) {
Review comment:
Since there's is no standard for JSON schemas, I'd prefer to leave it for
now with the fixed `allowNull=true` and don't offer the option here
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] merlimat commented on a change in pull request #3752: revise the schema default type not null
merlimat commented on a change in pull request #3752: revise the schema default
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r263035591
##
File path:
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
##
@@ -167,49 +167,95 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
}
/**
- * Create a Avro schema type by extracting the fields of the specified
class.
+ * Create a allow null Avro schema type by extracting the fields of the
specified class.
*
* @param clazz the POJO class to be used to extract the Avro schema
* @return a Schema instance
*/
static Schema AVRO(Class clazz) {
-return DefaultImplementation.newAvroSchema(clazz);
+return DefaultImplementation.newAvroSchema(clazz, true);
}
/**
- * Create a JSON schema type by extracting the fields of the specified
class.
+ * Create a Avro schema type by extracting the fields of the specified
class.
+ *
+ * @param clazz the POJO class to be used to extract the Avro schema
+ * allowNull the create a allow null or not null Avro schema
+ * @return a Schema instance
+ */
+static Schema AVRO(Class clazz, Boolean allowNull) {
+return DefaultImplementation.newAvroSchema(clazz, allowNull);
+}
+
+
+/**
+ * Create a allow null JSON schema type by extracting the fields of the
specified class.
*
* @param clazz the POJO class to be used to extract the JSON schema
* @return a Schema instance
*/
static Schema JSON(Class clazz) {
-return DefaultImplementation.newJSONSchema(clazz);
+return DefaultImplementation.newJSONSchema(clazz, true);
+}
+
+/**
+ * Create a allow null JSON schema type by extracting the fields of the
specified class.
+ *
+ * @param clazz the POJO class to be used to extract the JSON schema
+ * allowNull the create a allow null or not null JSON schema
+ * @return a Schema instance
+ */
+static Schema JSON(Class clazz, Boolean allowNull) {
+return DefaultImplementation.newJSONSchema(clazz, allowNull);
}
+
/**
* Key Value Schema using passed in schema type, support JSON and AVRO
currently.
*/
static Schema> KeyValue(Class key, Class
value, SchemaType type) {
-return DefaultImplementation.newKeyValueSchema(key, value, type);
+return DefaultImplementation.newKeyValueSchema(key, value, type, true);
}
+/**
+ * Key Value Schema using passed in schema type, support JSON and AVRO
currently.
+ */
+static Schema> KeyValue(Class key, Class
value, SchemaType type, Boolean allowNull) {
Review comment:
This becomes even more confusing, because `allowNull` here would be applied
to, say, a protobuf schema for which it will not make sense.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
