github-code-scanning[bot] commented on code in PR #1823:
URL: https://github.com/apache/avro/pull/1823#discussion_r944128110
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -179,126 +179,108 @@
/// <returns></returns>
public DotnetClass GetClass(RecordSchema schema)
{
- DotnetClass c;
- if (!_nameClassMap.TryGetValue(schema.Fullname, out c))
- {
- return null;
- }
-
- return c;
+ return (DotnetClass)GetClass(schema.Fullname);
}
/// <summary>
- /// Add an entry to the class cache.
+ /// Add an entry to the class cache.
/// </summary>
/// <param name="objType">Type of the C# class</param>
/// <param name="s">Schema</param>
+ [Obsolete()]
public void LoadClassCache(Type objType, Schema s)
{
- switch (s)
- {
- case RecordSchema rs:
- if (!objType.IsClass)
- {
- throw new AvroException($"Cant map scalar type
{objType.Name} to record {rs.Fullname}");
- }
+ _reflectFactory.LoadClassCache(objType, s);
+ }
- if (typeof(byte[]).IsAssignableFrom(objType)
- || typeof(string).IsAssignableFrom(objType)
- || typeof(IEnumerable).IsAssignableFrom(objType)
- || typeof(IDictionary).IsAssignableFrom(objType))
- {
- throw new AvroException($"Cant map type {objType.Name}
to record {rs.Fullname}");
- }
+ // IReflectCache is implemented for backward compatibility
+ #region IReflectCahce
- AddClassNameMapItem(rs, objType);
- var c = GetClass(rs);
- foreach (var f in rs.Fields)
- {
- /*
- //.StackOverflowException
- var t = c.GetPropertyType(f);
- LoadClassCache(t, f.Schema);
- */
- if (_previousFields.TryAdd(f.Name, f.Schema))
- {
- var t = c.GetPropertyType(f);
- LoadClassCache(t, f.Schema);
- }
- }
+ /// <summary>
+ /// Find a class that matches the schema full name.
+ /// </summary>
+ /// <param name="schemaFullName"></param>
+ /// <returns></returns>
+ [Obsolete()]
+ public IDotnetClass GetClass(string schemaFullName)
+ {
+ IDotnetClass c;
+ if (!_nameClassMap.TryGetValue(schemaFullName, out c))
+ {
+ return null;
+ }
- break;
- case ArraySchema ars:
- if (!typeof(IEnumerable).IsAssignableFrom(objType))
- {
- throw new AvroException($"Cant map type {objType.Name}
to array {ars.Name}");
- }
+ return c;
+ }
- if (!objType.IsGenericType)
- {
- throw new AvroException($"{objType.Name} needs to be a
generic type");
- }
+ /// <summary>
+ /// Add a class that for schema full name.
+ /// </summary>
+ /// <param name="schemaFullName"></param>
+ /// <param name="dotnetClass"></param>
+ [Obsolete()]
+ public void AddClass(string schemaFullName, IDotnetClass dotnetClass)
+ {
+ _nameClassMap.TryAdd(schemaFullName, dotnetClass);
+ }
- LoadClassCache(objType.GenericTypeArguments[0],
ars.ItemSchema);
- break;
- case MapSchema ms:
- if (!typeof(IDictionary).IsAssignableFrom(objType))
- {
- throw new AvroException($"Cant map type {objType.Name}
to map {ms.Name}");
- }
+ /// <summary>
+ /// Find a enum type that matches the schema full name.
+ /// </summary>
+ /// <param name="schemaFullName"></param>
+ /// <returns></returns>
+ [Obsolete()]
+ public Type GetEnum(string schemaFullName)
+ {
+ return EnumCache.GetEnumeration(schemaFullName);
+ }
- if (!objType.IsGenericType)
- {
- throw new AvroException($"Cant map non-generic type
{objType.Name} to map {ms.Name}");
- }
+ /// <summary>
+ /// Add a class that for schema full name.
+ /// </summary>
+ /// <param name="schemaFullName"></param>
+ /// <param name="enumType"></param>
+ [Obsolete()]
+ public void AddEnum(string schemaFullName, Type enumType)
+ {
+ EnumCache.AddEnumNameMapItem(schemaFullName, enumType);
+ }
- if
(!typeof(string).IsAssignableFrom(objType.GenericTypeArguments[0]))
- {
- throw new AvroException($"First type parameter of
{objType.Name} must be assignable to string");
- }
+ /// <summary>
+ /// Add an array helper type. Array helpers are used for collections
that are not generic lists.
+ /// </summary>
+ /// <param name="arrayHelperName">Name of the helper. Corresponds to
metadata "helper" field in the schema.</param>
+ public void AddArrayHelperType<T>(string arrayHelperName) where T :
IArrayHelper
+ {
+ _nameArrayMap.TryAdd(arrayHelperName, typeof(T));
+ }
- LoadClassCache(objType.GenericTypeArguments[1],
ms.ValueSchema);
- break;
- case NamedSchema ns:
- EnumCache.AddEnumNameMapItem(ns, objType);
- break;
- case UnionSchema us:
- if (us.Schemas.Count == 2 && (us.Schemas[0].Tag ==
Schema.Type.Null || us.Schemas[1].Tag == Schema.Type.Null))
- {
- // in this case objType will match the non null type
in the union
- foreach (var o in us.Schemas)
- {
- if (o.Tag == Schema.Type.Null)
- {
- continue;
- }
+ /// <summary>
+ /// Find an array helper type for an array schema node.
+ /// </summary>
+ /// <param name="arrayHelperName">Schema</param>
+ /// <param name="arrayHelperType">Schema</param>
+ /// <returns></returns>
+ public bool TryGetArrayHelperType(string arrayHelperName, out Type
arrayHelperType)
+ {
+ return _nameArrayMap.TryGetValue(arrayHelperName, out
arrayHelperType);
+ }
- if (objType.IsClass)
- {
- LoadClassCache(objType, o);
- }
+ #endregion
- var innerType =
Nullable.GetUnderlyingType(objType);
- if (innerType != null && innerType.IsEnum)
- {
- LoadClassCache(innerType, o);
- }
- }
- }
- else
- {
- // check the schema types are registered
- foreach (var o in us.Schemas)
- {
- if (o.Tag == Schema.Type.Record && GetClass(o as
RecordSchema) == null)
- {
- throw new AvroException($"Class for union
record type {o.Fullname} is not registered. Create a ClassCache object and call
LoadClassCache");
- }
- }
- }
+ #region IConverterService
- break;
- }
+ /// <summary>
+ /// Get converter
+ /// </summary>
+ /// <param name="schema"></param>
+ /// <param name="property"></param>
+ /// <returns></returns>
+ public IAvroFieldConverter GetConverter(Schema schema, PropertyInfo
property)
+ {
+ return GetDefaultConverter(schema.Tag, property.PropertyType);
Review Comment:
## Call to obsolete method
Call to obsolete method [GetDefaultConverter](1).
[Show more
details](https://github.com/apache/avro/security/code-scanning/2879)
##########
lang/csharp/src/apache/main/Reflect/Service/ReflectFactory.cs:
##########
@@ -0,0 +1,202 @@
+/*
+ * 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
+ *
+ * https://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.
+ */
+
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Text;
+using Avro.Reflect.Model;
+
+namespace Avro.Reflect.Service
+{
+ /// <summary>
+ /// Factory to create objects for reflest serialixation and deserialization
+ /// </summary>
+ public class ReflectFactory : IReflectFactory
+ {
+ private readonly IReflectCache _refletCache;
+ private readonly IArrayService _arrayService;
+ private readonly IDotnetclassFactory _dotnetclassFactory;
+
+ /// <summary>
+ /// Public constructor
+ /// </summary>
+ public ReflectFactory(
+ IReflectCache refletCache,
+ IArrayService arrayService,
+ IDotnetclassFactory dotnetclassFactory)
+ {
+ _refletCache = refletCache;
+ _arrayService = arrayService;
+ _dotnetclassFactory = dotnetclassFactory;
+ }
+
+ /// <summary>
+ /// Create reflect reader
+ /// </summary>
+ /// <typeparam name="T"></typeparam>
+ /// <param name="writerSchema"></param>
+ /// <param name="readerSchema"></param>
+ /// <returns></returns>
+ public ReflectReader<T> CreateReader<T>(Schema writerSchema, Schema
readerSchema)
+ {
+ LoadClassCache(typeof(T), readerSchema);
+
+ return new ReflectReader<T>(writerSchema, readerSchema,
_refletCache, _arrayService);
+ }
+
+ /// <summary>
+ /// Create reflect writer
+ /// </summary>
+ /// <typeparam name="T"></typeparam>
+ /// <param name="writerSchema"></param>
+ /// <returns></returns>
+ public ReflectWriter<T> CreateWriter<T>(Schema writerSchema)
+ {
+ LoadClassCache(typeof(T), writerSchema);
+
+ return new ReflectWriter<T>(writerSchema, _refletCache,
_arrayService);
+ }
+
+ internal void LoadClassCache(Type objType, Schema s)
+ {
+ Dictionary<string, Schema> previousFields = new Dictionary<string,
Schema>();
+
+ switch (s)
+ {
+ case RecordSchema rs:
+ if (!objType.IsClass)
+ {
+ throw new AvroException($"Cant map scalar type
{objType.Name} to record {rs.Fullname}");
+ }
+
+ if (typeof(byte[]).IsAssignableFrom(objType)
+ || typeof(string).IsAssignableFrom(objType)
+ || typeof(IEnumerable).IsAssignableFrom(objType)
+ || typeof(IDictionary).IsAssignableFrom(objType))
+ {
+ throw new AvroException($"Cant map type {objType.Name}
to record {rs.Fullname}");
+ }
+
+ AddClassNameMapItem(rs, objType);
+ var c = _refletCache.GetClass(rs.Fullname);
+ foreach (var f in rs.Fields)
+ {
+ /*
+ //.StackOverflowException
+ var t = c.GetPropertyType(f);
+ LoadClassCache(t, f.Schema);
+ */
+ if (!previousFields.ContainsKey(f.Name))
+ {
+ previousFields.Add(f.Name, f.Schema);
+ var t = c.GetPropertyType(f);
+ LoadClassCache(t, f.Schema);
+ }
+ }
Review Comment:
## Missed opportunity to use Where
This foreach loop implicitly filters its target sequence [here](1) -
consider filtering the sequence explicitly using '.Where(...)'.
[Show more
details](https://github.com/apache/avro/security/code-scanning/2887)
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -179,126 +179,108 @@
/// <returns></returns>
public DotnetClass GetClass(RecordSchema schema)
{
- DotnetClass c;
- if (!_nameClassMap.TryGetValue(schema.Fullname, out c))
- {
- return null;
- }
-
- return c;
+ return (DotnetClass)GetClass(schema.Fullname);
Review Comment:
## Call to obsolete method
Call to obsolete method [GetClass](1).
[Show more
details](https://github.com/apache/avro/security/code-scanning/2878)
##########
lang/csharp/src/apache/main/Reflect/Service/ReflectFactory.cs:
##########
@@ -0,0 +1,202 @@
+/*
+ * 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
+ *
+ * https://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.
+ */
+
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Text;
+using Avro.Reflect.Model;
+
+namespace Avro.Reflect.Service
+{
+ /// <summary>
+ /// Factory to create objects for reflest serialixation and deserialization
+ /// </summary>
+ public class ReflectFactory : IReflectFactory
+ {
+ private readonly IReflectCache _refletCache;
+ private readonly IArrayService _arrayService;
+ private readonly IDotnetclassFactory _dotnetclassFactory;
+
+ /// <summary>
+ /// Public constructor
+ /// </summary>
+ public ReflectFactory(
+ IReflectCache refletCache,
+ IArrayService arrayService,
+ IDotnetclassFactory dotnetclassFactory)
+ {
+ _refletCache = refletCache;
+ _arrayService = arrayService;
+ _dotnetclassFactory = dotnetclassFactory;
+ }
+
+ /// <summary>
+ /// Create reflect reader
+ /// </summary>
+ /// <typeparam name="T"></typeparam>
+ /// <param name="writerSchema"></param>
+ /// <param name="readerSchema"></param>
+ /// <returns></returns>
+ public ReflectReader<T> CreateReader<T>(Schema writerSchema, Schema
readerSchema)
+ {
+ LoadClassCache(typeof(T), readerSchema);
+
+ return new ReflectReader<T>(writerSchema, readerSchema,
_refletCache, _arrayService);
+ }
+
+ /// <summary>
+ /// Create reflect writer
+ /// </summary>
+ /// <typeparam name="T"></typeparam>
+ /// <param name="writerSchema"></param>
+ /// <returns></returns>
+ public ReflectWriter<T> CreateWriter<T>(Schema writerSchema)
+ {
+ LoadClassCache(typeof(T), writerSchema);
+
+ return new ReflectWriter<T>(writerSchema, _refletCache,
_arrayService);
+ }
+
+ internal void LoadClassCache(Type objType, Schema s)
+ {
+ Dictionary<string, Schema> previousFields = new Dictionary<string,
Schema>();
+
+ switch (s)
+ {
+ case RecordSchema rs:
+ if (!objType.IsClass)
+ {
+ throw new AvroException($"Cant map scalar type
{objType.Name} to record {rs.Fullname}");
+ }
+
+ if (typeof(byte[]).IsAssignableFrom(objType)
+ || typeof(string).IsAssignableFrom(objType)
+ || typeof(IEnumerable).IsAssignableFrom(objType)
+ || typeof(IDictionary).IsAssignableFrom(objType))
+ {
+ throw new AvroException($"Cant map type {objType.Name}
to record {rs.Fullname}");
+ }
+
+ AddClassNameMapItem(rs, objType);
+ var c = _refletCache.GetClass(rs.Fullname);
+ foreach (var f in rs.Fields)
+ {
+ /*
+ //.StackOverflowException
+ var t = c.GetPropertyType(f);
+ LoadClassCache(t, f.Schema);
+ */
+ if (!previousFields.ContainsKey(f.Name))
+ {
+ previousFields.Add(f.Name, f.Schema);
+ var t = c.GetPropertyType(f);
+ LoadClassCache(t, f.Schema);
+ }
+ }
+
+ break;
+ case ArraySchema ars:
+ if (!typeof(IEnumerable).IsAssignableFrom(objType))
+ {
+ throw new AvroException($"Cant map type {objType.Name}
to array {ars.Name}");
+ }
+
+ if (!objType.IsGenericType)
+ {
+ throw new AvroException($"{objType.Name} needs to be a
generic type");
+ }
+
+ LoadClassCache(objType.GenericTypeArguments[0],
ars.ItemSchema);
+ break;
+ case MapSchema ms:
+ if (!typeof(IDictionary).IsAssignableFrom(objType))
+ {
+ throw new AvroException($"Cant map type {objType.Name}
to map {ms.Name}");
+ }
+
+ if (!objType.IsGenericType)
+ {
+ throw new AvroException($"Cant map non-generic type
{objType.Name} to map {ms.Name}");
+ }
+
+ if
(!typeof(string).IsAssignableFrom(objType.GenericTypeArguments[0]))
+ {
+ throw new AvroException($"First type parameter of
{objType.Name} must be assignable to string");
+ }
+
+ LoadClassCache(objType.GenericTypeArguments[1],
ms.ValueSchema);
+ break;
+ case NamedSchema ns:
+ _refletCache.AddEnum(ns.Fullname, objType);
+ break;
+ case UnionSchema us:
+ if (us.Schemas.Count == 2 && (us.Schemas[0].Tag ==
Schema.Type.Null || us.Schemas[1].Tag == Schema.Type.Null))
+ {
+ // in this case objType will match the non null type
in the union
+ foreach (var o in us.Schemas)
+ {
+ if (o.Tag == Schema.Type.Null)
+ {
+ continue;
+ }
+
+ if (objType.IsClass)
+ {
+ LoadClassCache(objType, o);
+ }
+
+ var innerType =
Nullable.GetUnderlyingType(objType);
+ if (innerType != null && innerType.IsEnum)
+ {
+ LoadClassCache(innerType, o);
+ }
+ }
+ }
+ else
+ {
+ // check the schema types are registered
+ foreach (var o in us.Schemas)
+ {
+ if (o.Tag == Schema.Type.Record &&
_refletCache.GetClass(o.Fullname) == null)
+ {
+ throw new AvroException($"Class for union
record type {o.Fullname} is not registered. Create a ClassCache object and call
LoadClassCache");
+ }
+ }
+ }
+
+ break;
+ }
+ }
+
+ private void AddClassNameMapItem(RecordSchema schema, Type dotnetClass)
+ {
+ if (schema != null && _refletCache.GetClass(schema.Fullname) !=
null)
+ {
+ return;
+ }
+
+ if (!dotnetClass.IsClass)
+ {
+ throw new AvroException($"Type {dotnetClass.Name} is not a
class");
+ }
+
+ _refletCache.AddClass(schema.Fullname,
_dotnetclassFactory.CreateDotnetClass(schema, dotnetClass));
Review Comment:
## Dereferenced variable may be null
Variable [schema](1) may be null here as suggested by [this](2) null check.
[Show more
details](https://github.com/apache/avro/security/code-scanning/2880)
##########
lang/csharp/src/apache/main/Reflect/Service/ConverterService.cs:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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
+ *
+ * https://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.
+ */
+
+using System;
+using System.Linq;
+using System.Collections.Generic;
+using System.Reflection;
+using System.Text;
+using Avro.Reflect.Converter;
+
+namespace Avro.Reflect.Service
+{
+ /// <summary>
+ /// Service to work with default IAvroFieldConverter
+ /// </summary>
+ public class ConverterService : IConverterService
+ {
+ private readonly IEnumerable<IAvroFieldConverter> _convertors;
+
+ /// <summary>
+ /// Public constructor
+ /// </summary>
+ public ConverterService(IEnumerable<IAvroFieldConverter> convertors)
+ {
+ _convertors = convertors;
+ }
+
+ /// <summary>
+ /// Find a converter
+ /// </summary>
+ /// <param name="schema"></param>
+ /// <param name="property"></param>
+ /// <returns>The first matching converter - null if there isn't
one</returns>
+ public IAvroFieldConverter GetConverter(Schema schema, PropertyInfo
property)
+ {
+ return GetFieldConverter(schema, property) ??
GetRegisteredConverter(schema, property);
+ }
+
+ internal IAvroFieldConverter GetFieldConverter(Schema schema,
PropertyInfo property)
+ {
+ var avroFieldConverters = property
+ .GetCustomAttributes(typeof(AvroFieldAttribute), true)
+ .Select(attr => ((AvroFieldAttribute)attr).Converter)
+ .Where(converter => converter != null)
+ .ToArray();
+
+ if (avroFieldConverters.Length > 1)
+ throw new AvroException($"More them one converter is defined
by AvroFieldAttribute for '{property.Name}'.");
+
+ return avroFieldConverters.Length == 1 ? avroFieldConverters[0] :
null;
+ }
+
+ internal IAvroFieldConverter GetRegisteredConverter(Schema schema,
PropertyInfo property)
+ {
+ Type avroType;
+ switch (schema.Tag)
+ {
+ case Avro.Schema.Type.Null:
+ return null;
+ case Avro.Schema.Type.Boolean:
+ avroType = typeof(bool);
+ break;
+ case Avro.Schema.Type.Int:
+ avroType = typeof(int);
+ break;
+ case Avro.Schema.Type.Long:
+ avroType = typeof(long);
+ break;
+ case Avro.Schema.Type.Float:
+ avroType = typeof(float);
+ break;
+ case Avro.Schema.Type.Double:
+ avroType = typeof(double);
+ break;
+ case Avro.Schema.Type.Bytes:
+ avroType = typeof(byte[]);
+ break;
+ case Avro.Schema.Type.String:
+ avroType = typeof(string);
+ break;
+ case Avro.Schema.Type.Record:
+ return null;
+ case Avro.Schema.Type.Enumeration:
+ return null;
+ case Avro.Schema.Type.Array:
+ return null;
+ case Avro.Schema.Type.Map:
+ return null;
+ case Avro.Schema.Type.Union:
+ return null;
+ case Avro.Schema.Type.Fixed:
+ avroType = typeof(byte[]);
+ break;
+ case Avro.Schema.Type.Error:
+ return null;
+ default:
+ return null;
+ }
+
+ foreach (var c in _convertors)
+ {
+ if (c.GetAvroType() == avroType && c.GetPropertyType() ==
property.PropertyType)
+ {
+ return c;
+ }
+ }
Review Comment:
## Missed opportunity to use Where
This foreach loop implicitly filters its target sequence [here](1) -
consider filtering the sequence explicitly using '.Where(...)'.
[Show more
details](https://github.com/apache/avro/security/code-scanning/2886)
##########
lang/csharp/src/apache/main/Reflect/Service/DotnetclassFactory.cs:
##########
@@ -17,30 +17,92 @@
*/
using System;
-using System.Reflection;
using System.Collections;
+using System.Collections.Generic;
+using System.Reflection;
+using System.Text;
+using Avro.Reflect.Converter;
+using Avro.Reflect.Model;
-namespace Avro.Reflect
+namespace Avro.Reflect.Service
{
- internal class DotnetProperty
+ /// <summary>
+ /// Factory to create DotnetClass and related objects
+ /// </summary>
+ public class DotnetclassFactory : IDotnetclassFactory
{
- private PropertyInfo _property;
+ private readonly IConverterService _converterService;
- public IAvroFieldConverter Converter { get; set; }
+ /// <summary>
+ /// Public constructor
+ /// </summary>
+ /// <param name="converterService"></param>
+ public DotnetclassFactory(IConverterService converterService)
+ {
+ _converterService = converterService;
+ }
- private bool IsPropertyCompatible(Avro.Schema schema)
+ /// <summary>
+ /// Create IDotnetClass object
+ /// </summary>
+ /// <param name="schema"></param>
+ /// <param name="type"></param>
+ /// <returns></returns>
+ public IDotnetClass CreateDotnetClass(RecordSchema schema, Type type)
{
- Type propType;
- var schemaTag = schema.Tag;
+ Dictionary<string, IDotnetProperty> properties = new
Dictionary<string, IDotnetProperty>();
- if (Converter == null)
+ foreach (var field in schema.Fields)
{
- propType = _property.PropertyType;
+ PropertyInfo property = GetPropertyInfo(field, type);
+ properties.Add(field.Name, CreateDotnetProperty(field.Schema,
property));
}
- else
+
+ return new DotnetClass(type, properties);
+ }
+
+ /// <summary>
+ /// Create DotnetProperty object
+ /// </summary>
+ /// <param name="schema"></param>
+ /// <param name="property"></param>
+ /// <returns></returns>
+ private IDotnetProperty CreateDotnetProperty(Schema schema,
PropertyInfo property)
+ {
+ var converter = _converterService.GetConverter(schema, property);
+ var type = converter?.GetPropertyType() ?? property.PropertyType;
+
+ if (IsTypeCompatibleWithSchema(schema, type))
+ return new DotnetProperty(property, converter);
+
+ throw new AvroException($"Property {property.Name} in object
{property.DeclaringType} isn't compatible with Avro schema type {schema.Tag}");
+ }
+
+ private PropertyInfo GetPropertyInfo(Field field, Type type)
+ {
+ var prop = type.GetProperty(field.Name);
+ if (prop != null)
{
- propType = Converter.GetAvroType();
+ return prop;
}
+ foreach (var p in type.GetProperties())
+ {
+ foreach (var attr in p.GetCustomAttributes(true))
+ {
+ var avroAttr = attr as AvroFieldAttribute;
+ if (avroAttr != null && avroAttr.FieldName != null &&
avroAttr.FieldName == field.Name)
+ {
+ return p;
+ }
+ }
Review Comment:
## Missed opportunity to use OfType
This foreach loop immediately uses 'as' to coerce its iteration variable to
another type [here](1) - consider using '.OfType(...)' instead.
[Show more
details](https://github.com/apache/avro/security/code-scanning/2885)
##########
lang/csharp/src/apache/main/Reflect/Model/DotnetClass.cs:
##########
@@ -18,72 +18,28 @@
using System;
using System.Reflection;
-using System.Collections.Concurrent;
using Avro;
+using System.Collections.Generic;
-namespace Avro.Reflect
+namespace Avro.Reflect.Model
{
/// <summary>
/// Collection of DotNetProperty objects to repre
/// </summary>
- public class DotnetClass
+ public class DotnetClass : IDotnetClass
{
- private ConcurrentDictionary<string, DotnetProperty> _propertyMap =
new ConcurrentDictionary<string, DotnetProperty>();
-
+ private Dictionary<string, IDotnetProperty> _propertyMap;
Review Comment:
## Missed 'readonly' opportunity
Field '_propertyMap' can be 'readonly'.
[Show more
details](https://github.com/apache/avro/security/code-scanning/2882)
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -19,40 +19,48 @@
using System;
using System.Collections;
using System.Collections.Concurrent;
+using System.Reflection;
+using Avro.Reflect.Array;
+using Avro.Reflect.Converter;
+using Avro.Reflect.Model;
+using Avro.Reflect.Service;
namespace Avro.Reflect
{
/// <summary>
/// Class holds a cache of C# classes and their properties. The key for
the cache is the schema full name.
/// </summary>
- public class ClassCache
+ [Obsolete()]
+ public class ClassCache : IReflectCache, IConverterService
{
private static ConcurrentBag<IAvroFieldConverter> _defaultConverters =
new ConcurrentBag<IAvroFieldConverter>();
- private ConcurrentDictionary<string, DotnetClass> _nameClassMap = new
ConcurrentDictionary<string, DotnetClass>();
+ private ConcurrentDictionary<string, IDotnetClass> _nameClassMap = new
ConcurrentDictionary<string, IDotnetClass>();
Review Comment:
## Missed 'readonly' opportunity
Field '_nameClassMap' can be 'readonly'.
[Show more
details](https://github.com/apache/avro/security/code-scanning/2881)
##########
lang/csharp/src/apache/main/Reflect/Service/ReflectFactory.cs:
##########
@@ -0,0 +1,202 @@
+/*
+ * 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
+ *
+ * https://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.
+ */
+
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Text;
+using Avro.Reflect.Model;
+
+namespace Avro.Reflect.Service
+{
+ /// <summary>
+ /// Factory to create objects for reflest serialixation and deserialization
+ /// </summary>
+ public class ReflectFactory : IReflectFactory
+ {
+ private readonly IReflectCache _refletCache;
+ private readonly IArrayService _arrayService;
+ private readonly IDotnetclassFactory _dotnetclassFactory;
+
+ /// <summary>
+ /// Public constructor
+ /// </summary>
+ public ReflectFactory(
+ IReflectCache refletCache,
+ IArrayService arrayService,
+ IDotnetclassFactory dotnetclassFactory)
+ {
+ _refletCache = refletCache;
+ _arrayService = arrayService;
+ _dotnetclassFactory = dotnetclassFactory;
+ }
+
+ /// <summary>
+ /// Create reflect reader
+ /// </summary>
+ /// <typeparam name="T"></typeparam>
+ /// <param name="writerSchema"></param>
+ /// <param name="readerSchema"></param>
+ /// <returns></returns>
+ public ReflectReader<T> CreateReader<T>(Schema writerSchema, Schema
readerSchema)
+ {
+ LoadClassCache(typeof(T), readerSchema);
+
+ return new ReflectReader<T>(writerSchema, readerSchema,
_refletCache, _arrayService);
+ }
+
+ /// <summary>
+ /// Create reflect writer
+ /// </summary>
+ /// <typeparam name="T"></typeparam>
+ /// <param name="writerSchema"></param>
+ /// <returns></returns>
+ public ReflectWriter<T> CreateWriter<T>(Schema writerSchema)
+ {
+ LoadClassCache(typeof(T), writerSchema);
+
+ return new ReflectWriter<T>(writerSchema, _refletCache,
_arrayService);
+ }
+
+ internal void LoadClassCache(Type objType, Schema s)
+ {
+ Dictionary<string, Schema> previousFields = new Dictionary<string,
Schema>();
+
+ switch (s)
+ {
+ case RecordSchema rs:
+ if (!objType.IsClass)
+ {
+ throw new AvroException($"Cant map scalar type
{objType.Name} to record {rs.Fullname}");
+ }
+
+ if (typeof(byte[]).IsAssignableFrom(objType)
+ || typeof(string).IsAssignableFrom(objType)
+ || typeof(IEnumerable).IsAssignableFrom(objType)
+ || typeof(IDictionary).IsAssignableFrom(objType))
+ {
+ throw new AvroException($"Cant map type {objType.Name}
to record {rs.Fullname}");
+ }
+
+ AddClassNameMapItem(rs, objType);
+ var c = _refletCache.GetClass(rs.Fullname);
+ foreach (var f in rs.Fields)
+ {
+ /*
+ //.StackOverflowException
+ var t = c.GetPropertyType(f);
+ LoadClassCache(t, f.Schema);
+ */
+ if (!previousFields.ContainsKey(f.Name))
+ {
+ previousFields.Add(f.Name, f.Schema);
+ var t = c.GetPropertyType(f);
+ LoadClassCache(t, f.Schema);
+ }
+ }
+
+ break;
+ case ArraySchema ars:
+ if (!typeof(IEnumerable).IsAssignableFrom(objType))
+ {
+ throw new AvroException($"Cant map type {objType.Name}
to array {ars.Name}");
+ }
+
+ if (!objType.IsGenericType)
+ {
+ throw new AvroException($"{objType.Name} needs to be a
generic type");
+ }
+
+ LoadClassCache(objType.GenericTypeArguments[0],
ars.ItemSchema);
+ break;
+ case MapSchema ms:
+ if (!typeof(IDictionary).IsAssignableFrom(objType))
+ {
+ throw new AvroException($"Cant map type {objType.Name}
to map {ms.Name}");
+ }
+
+ if (!objType.IsGenericType)
+ {
+ throw new AvroException($"Cant map non-generic type
{objType.Name} to map {ms.Name}");
+ }
+
+ if
(!typeof(string).IsAssignableFrom(objType.GenericTypeArguments[0]))
+ {
+ throw new AvroException($"First type parameter of
{objType.Name} must be assignable to string");
+ }
+
+ LoadClassCache(objType.GenericTypeArguments[1],
ms.ValueSchema);
+ break;
+ case NamedSchema ns:
+ _refletCache.AddEnum(ns.Fullname, objType);
+ break;
+ case UnionSchema us:
+ if (us.Schemas.Count == 2 && (us.Schemas[0].Tag ==
Schema.Type.Null || us.Schemas[1].Tag == Schema.Type.Null))
+ {
+ // in this case objType will match the non null type
in the union
+ foreach (var o in us.Schemas)
+ {
+ if (o.Tag == Schema.Type.Null)
+ {
+ continue;
+ }
+
+ if (objType.IsClass)
+ {
+ LoadClassCache(objType, o);
+ }
+
+ var innerType =
Nullable.GetUnderlyingType(objType);
+ if (innerType != null && innerType.IsEnum)
+ {
+ LoadClassCache(innerType, o);
+ }
+ }
+ }
+ else
+ {
+ // check the schema types are registered
+ foreach (var o in us.Schemas)
+ {
+ if (o.Tag == Schema.Type.Record &&
_refletCache.GetClass(o.Fullname) == null)
+ {
+ throw new AvroException($"Class for union
record type {o.Fullname} is not registered. Create a ClassCache object and call
LoadClassCache");
+ }
+ }
Review Comment:
## Missed opportunity to use Where
This foreach loop implicitly filters its target sequence [here](1) -
consider filtering the sequence explicitly using '.Where(...)'.
[Show more
details](https://github.com/apache/avro/security/code-scanning/2888)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
