You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@avro.apache.org by GitBox <gi...@apache.org> on 2022/08/11 05:37:54 UTC
[GitHub] [avro] github-code-scanning[bot] commented on a diff in pull request #1819: AVRO-3603 dotnet reflect reader and writer refactoring
github-code-scanning[bot] commented on code in PR #1819:
URL: https://github.com/apache/avro/pull/1819#discussion_r943108992
##########
lang/csharp/src/apache/main/Reflect/ReflectDefaultWriter.cs:
##########
@@ -188,9 +181,9 @@
return obj is string;
case Schema.Type.Error:
case Schema.Type.Record:
- return _classCache.GetClass(sc as RecordSchema).GetClassType() == obj.GetType();
+ return _reflectCache.GetClass(sc as RecordSchema).GetClassType() == obj.GetType();
case Schema.Type.Enumeration:
- return EnumCache.GetEnumeration(sc as EnumSchema) == obj.GetType();
+ return _reflectCache.GetEnumeration(sc as EnumSchema) == obj.GetType();
Review Comment:
## Dereferenced variable may be null
Variable [obj](1) may be null here as suggested by [this](2) null check.
Variable [obj](1) may be null here as suggested by [this](3) null check.
[Show more details](https://github.com/apache/avro/security/code-scanning/2845)
##########
lang/csharp/src/apache/main/Reflect/Reflection/ReflectCache.cs:
##########
@@ -288,17 +256,53 @@
~ 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");
~ }
~ }
~ }
~
break;
}
}
+
+ /// <summary>
+ /// Lookup an entry in the cache - based on the schema fullname
+ /// </summary>
+ /// <param name="schema"></param>
+ /// <returns></returns>
+ public Type GetEnumeration(NamedSchema schema)
+ {
+ Type t;
+ if (!_nameEnumMap.TryGetValue(schema.Fullname, out t))
+ {
+ throw new AvroException($"Couldn't find enumeration for avro fullname: {schema.Fullname}");
+ }
+
+ return t;
+ }
+
+ private void AddEnumNameMapItem(NamedSchema schema, Type dotnetEnum)
+ {
+ _nameEnumMap.TryAdd(schema.Fullname, dotnetEnum);
+ }
+
+ private void AddClassNameMapItem(RecordSchema schema, Type dotnetClass)
+ {
+ if (schema != null && GetClass(schema) != null)
+ {
+ return;
+ }
+
+ if (!dotnetClass.IsClass)
+ {
+ throw new AvroException($"Type {dotnetClass.Name} is not a class");
+ }
+
+ _nameClassMap.TryAdd(schema.Fullname, new DotnetClass(dotnetClass, schema, this));
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/2847)
##########
lang/csharp/src/apache/main/Reflect/Reflection/ReflectCache.cs:
##########
@@ -19,63 +19,31 @@
using System;
using System.Collections;
using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Text;
+using Avro.Reflect.Converter;
+using Avro.Reflect.Interface;
-namespace Avro.Reflect
+namespace Avro.Reflect.Reflection
{
- /// <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
+ internal class ReflectCache : IReflectCache
{
- private static ConcurrentBag<IAvroFieldConverter> _defaultConverters = new ConcurrentBag<IAvroFieldConverter>();
-
+ private ConcurrentDictionary<string, Type> _nameEnumMap = new ConcurrentDictionary<string, Type>();
Review Comment:
## Missed 'readonly' opportunity
Field '_nameEnumMap' can be 'readonly'.
[Show more details](https://github.com/apache/avro/security/code-scanning/2850)
##########
lang/csharp/src/apache/main/Reflect/ReflectDefaultWriter.cs:
##########
@@ -188,9 +181,9 @@
return obj is string;
case Schema.Type.Error:
case Schema.Type.Record:
- return _classCache.GetClass(sc as RecordSchema).GetClassType() == obj.GetType();
+ return _reflectCache.GetClass(sc as RecordSchema).GetClassType() == obj.GetType();
Review Comment:
## Dereferenced variable may be null
Variable [obj](1) may be null here as suggested by [this](2) null check.
Variable [obj](1) may be null here as suggested by [this](3) null check.
[Show more details](https://github.com/apache/avro/security/code-scanning/2844)
##########
lang/csharp/src/apache/main/Reflect/ReflectDefaultWriter.cs:
##########
@@ -28,28 +30,19 @@
/// </summary>
public class ReflectDefaultWriter : SpecificDefaultWriter
{
- private ClassCache _classCache = new ClassCache();
-
- /// <summary>
- /// Class cache
- /// </summary>
- public ClassCache ClassCache { get => _classCache; }
+ private IReflectCache _reflectCache;
Review Comment:
## Missed 'readonly' opportunity
Field '_reflectCache' can be 'readonly'.
[Show more details](https://github.com/apache/avro/security/code-scanning/2849)
##########
lang/csharp/src/apache/main/Reflect/ReflectDefaultReader.cs:
##########
@@ -37,12 +39,7 @@
/// </summary>
public Type MapType { get => _mapType; set => _mapType = value; }
- private ClassCache _classCache = new ClassCache();
-
- /// <summary>
- /// Class cache
- /// </summary>
- public ClassCache ClassCache { get => _classCache; }
+ private IReflectCache _reflectCache;
Review Comment:
## Missed 'readonly' opportunity
Field '_reflectCache' can be 'readonly'.
[Show more details](https://github.com/apache/avro/security/code-scanning/2848)
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org