Uploaded image for project: 'Corda'
  1. CORDA-2860

Property type checking is overly strict

    Details

    • Type: Bug
    • Status: Done (View workflow)
    • Priority: High
    • Resolution: Done
    • Affects versions: Corda 4, Corda Enterprise 4
    • Fix versions: Corda 4.1
    • Components: None
    • Labels:
      None

      Description

      When scanning the properties of a class during type analysis for serialisation, the types of getter and setter methods, and of backing fields, are checked to make sure that they line up. A getter should return a type that is assignable from the type of the corresponding backing field (if there is one), and a setter should receive a type that is assignable to the type returned by the corresponding getter.

      A bug in the implementation of this checking meant that in the case of the getter/backing field pair, the return type of the getter was checked against itself, rather than against the type of the backing field. This would ordinarily have no effect (besides making the check redundant), since we would expect a type always to be assignable from itself. However, some versions of Guava prior to 25.1-jre erroneously return false when asked to compare TypeToken.of(type).isSuperclassOf(TypeToken.of(type)) for some types. This has caused the check to fail on TraversableType.commands, which has the type List<Command<*>>, when an earlier version of Guava is in scope - an error which looks like:

      [ERROR] 10:09:49,500 [Mock network] amqp.SerializationOutput. - Serialization failed direction="Serialize", type="net.corda.core.transactions.TraversableTransaction", msg="Defined getter for parameter commands returns type java.util.List> yet underlying type is java.util.List>", ClassChain="net.corda.core.transactions.TraversableTransaction" {actor_id=Only For Testing, actor_owning_identity=O=Supplier 1, L=London, C=GB, actor_store_id=TEST, fiber-id=10000001, flow-id=e61b7da7-6826-4410-9414-1c03bebbc3fe, invocation_id=58c5f5ff-09f8-4bfa-bbe5-cf6b0ef47d06, invocation_timestamp=2019-03-28T10:09:46.205Z, origin=Only For Testing, session_id=58c5f5ff-09f8-4bfa-bbe5-cf6b0ef47d06, session_timestamp=2019-03-28T10:09:46.205Z, thread-id=400}
      

      This has been reported on StackOverflow: https://stackoverflow.com/questions/55400626/corda-4-0-transaction-serialization-issue/55444039#55444039

      A related issue occurred for B3i when attempting to serialize a class with a mutable field of type Set<T>, e.g.

      class HasMutableSetProperty(var property: Set<Foo>)
      

      Kotlin generates a getter for property which returns Set<T>, but a setter which accepts Set<? extends T>, which is correct behaviour for an immutable set. However, Guava does not recognise Set<T> as a superclass of Set<? extends T>, so the validation check on the getter/setter pair fails.

      This can be worked around with an annotation which instructs Kotlin not to generate the wildcard on the setter:

      @JvmSuppressWildcards
      class HasMutableSetProperty(var property: Set<Foo>)
      

      However, it would be better not to enforce an overly-strict compatibility rule in the first place.

      The validation checks on field/getter/setter types were introduced in Corda v4, and have caused issues for clients which they did not experience with Corda v3. We should do one of the following:

      • Remove them altogether, restoring the previous status quo.
      • Keep them, but ensure that they are correct for all cases (but note we have already encountered cases, such as the one above, which we did not anticipate and for which we had not tests in place).
      • Keep them but make them more lenient, e.g. checking only the assignability of erased types, and perhaps writing a warning to the logs rather than blowing up with an exception.

        Attachments

          Activity

            People

            • Assignee:
              Dominic.Fox Dominic.Fox@r3.com
              Reporter:
              Dominic.Fox Dominic.Fox@r3.com
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: