Thursday, May 30, 2024

JDK release cadence and the pain of endless technical debt

The shiny new code you are writing today is the legacy one tomorrow: this is probably the only flaw that new JDK release cadence brings to the table. Well, with all the respect, this is the price to pay to keep up with the pace of innovation, right? Yes, but your mileage may vary ... a lot.

To put things in perspective, let us start with a few examples, all taken from the open-source projects under Apache Software Foundation umbrella. The first one we are going to look at is switch statements, typically used like that:

@Override
protected int nextChild() {
    ElementFrame<Node, Node> frame = getCurrentFrame();
    if (frame.currentChild == null) {
        content = getCurrentNode().getFirstChild();
    } else {
        content = frame.currentChild.getNextSibling();
    }

    frame.currentChild = content;
    switch (content.getNodeType()) {
        case Node.ELEMENT_NODE:
            return START_ELEMENT;
        case Node.TEXT_NODE:
            return CHARACTERS;
        case Node.COMMENT_NODE:
            return COMMENT;
        case Node.CDATA_SECTION_NODE:
            return CDATA;
        case Node.ENTITY_REFERENCE_NODE:
            return ENTITY_REFERENCE;
        case Node.PROCESSING_INSTRUCTION_NODE:
            return PROCESSING_INSTRUCTION;
        default:
            throw new IllegalStateException("Found type: " + content.getClass().getName());
    }
}

Is it something you would type these days on modern JDK (and by modern, I would safely assume at least JDK-21), that has JEP-361: Switch Expressions incorporated? I doubt that, the switch expression is much better fit here:

@Override
protected int nextChild() {
    ElementFrame<Node, Node> frame = getCurrentFrame();
    if (frame.currentChild == null) {
        content = getCurrentNode().getFirstChild();
    } else {
        content = frame.currentChild.getNextSibling();
    }

    frame.currentChild = content;
    return switch (content.getNodeType()) {
        case Node.ELEMENT_NODE -> START_ELEMENT;
        case Node.TEXT_NODE -> CHARACTERS;
        case Node.COMMENT_NODE -> COMMENT;
        case Node.CDATA_SECTION_NODE -> CDATA;
        case Node.ENTITY_REFERENCE_NODE -> ENTITY_REFERENCE;
        case Node.PROCESSING_INSTRUCTION_NODE -> PROCESSING_INSTRUCTION;
        default -> throw new IllegalStateException("Found type: " + content.getClass().getName());
    };
}

If that is not convincing, another example would clear any doubts up. Here we have quite straightforward SimplePrincipal class (data class as we would normally call such classes):

public class SimplePrincipal implements Principal, Serializable {
    private static final long serialVersionUID = -5171549568204891853L;

    private final String name;

    public SimplePrincipal(String name) {
        if (name == null) {
            throw new IllegalArgumentException("Principal name can not be null");
        }
        this.name = name;
    }

    public String getName() {
        return name;
    }

    public boolean equals(Object obj) {
        if (!(obj instanceof SimplePrincipal)) {
            return false;
        }

        return name.equals(((SimplePrincipal)obj).name);
    }

    public int hashCode() {
        return name.hashCode();
    }

    public String toString() {
        return name;
    }
}

With the record classes, introduced by JEP 395: Records, we have much better tool at our disposal to model data classes:

public record SimplePrincipal(String name) implements Principal, Serializable {
    public SimplePrincipal {
        if (name == null) {
            throw new IllegalArgumentException("Principal name can not be null");
        }
    }
    
    @Override
    public String getName() {
        return name;
    }
}

The reduction in boilerplate code required is just astonishing. The next power feature (that could be actually used along with records) we are going to look at is sealed classes, interfaces and records, introduced by JEP 409: Sealed Classes. To showcase it, let us take a look at this (simplified) hierarchy of classed:

/**
 * This class is the base class for SSL/TLS parameters that are common
 * to both client and server sides.
 */
public class TLSParameterBase {
    protected static final Collection<String> DEFAULT_HTTPS_PROTOCOLS = 
        Arrays.asList(
            "TLSv1", 
            "TLSv1.1", 
            "TLSv1.2", 
            "TLSv1.3" 
        ); 
        
    // Other members
}

/**
 * This class extends {@link TLSParameterBase} with client-specific
 * SSL/TLS parameters.
 *
 */
public class TLSClientParameters extends TLSParameterBase {
   // TLSClientParameters class members
}

/**
 * This class extends {@link TLSParameterBase} with service-specific
 * SSL/TLS parameters.
 *
 */
public class TLSServerParameters extends TLSParameterBase {
   // TLSServerParameters class members
}

The problem with such design is that anyone could subclass TLSParameterBase (yes, we could make it package private but that would lead to inability to reference it outside of the package), violating the designer's invariants that only server and client ones are expected to exist.

/**
 * This class is the base class for SSL/TLS parameters that are common
 * to both client and server sides.
 */
public sealed class TLSParameterBase permits TLSClientParameters, TLSServerParameters {
    protected static final Collection<String> DEFAULT_HTTPS_PROTOCOLS = 
        Arrays.asList(
            "TLSv1", 
            "TLSv1.1", 
            "TLSv1.2", 
            "TLSv1.3" 
        ); 
        
    // Other members
}

/**
 * This class extends {@link TLSParameterBase} with client-specific
 * SSL/TLS parameters.
 *
 */
public final class TLSClientParameters extends TLSParameterBase {
   // TLSClientParameters class members
}

/**
 * This class extends {@link TLSParameterBase} with service-specific
 * SSL/TLS parameters.
 *
 */
public final class TLSServerParameters extends TLSParameterBase {
   // TLSServerParameters class members
}

The new sealed keyword at the TLSParameterBase class declaration level along with permits clause solves this design flaw once and for all (we could have pushed the refactoring even further and convert TLSParameterBase to record).

Let us move on to a slightly different subject: XML/JSON/YAML/(you name it) as Java Strings.

public class RequestParserUnitTest {
    private static final String USE_KEY_X509_REFERENCE = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
        + "<wst:RequestSecurityToken xmlns:wst=\"http://docs.oasis-open.org/ws-sx/ws-trust/200512\">"
        + "<wst:TokenType>http://schemas.xmlsoap.org/ws/2005/02/sc/sct</wst:TokenType>"
        + "<wst:RequestType>http://docs.oasis-open.org/ws-sx/ws-trust/200512/Issuelt;/wst:RequestType>"
        + "<wst:UseKey>"
        + "<wsse:SecurityTokenReference "
        + "xmlns:wsse=\"http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-secext-1.0.xsd\">"
        + "<wsse:Reference URI=\"#x509\">lt;/wsse:Reference>lt;/wsse:SecurityTokenReference>"
        + "</wst:UseKey>" 
        + "</wst:RequestSecurityToken>";
        
    // Other members      
}

Admittedly, it looks messy and unmanageable (write once, never touch again). More to that, since this is the unit test case, troubleshooting any failing test cases caused by XML/JSON/... changes is a nightmare. How about using text blocks (multi-line strings) instead, the gem brought to Java language by https://openjdk.org/jeps/378:

public class RequestParserUnitTest {
    private static final String USE_KEY_X509_REFERENCE = """
            <?xml version="1.0" encoding="UTF-8"?>"
            <wst:RequestSecurityToken xmlns:wst="http://docs.oasis-open.org/ws-sx/ws-trust/200512">
               <wst:TokenType>http://schemas.xmlsoap.org/ws/2005/02/sc/sct</wst:TokenType>
               <wst:RequestType>http://docs.oasis-open.org/ws-sx/ws-trust/200512/Issue</wst:RequestType>
               <wst:UseKey>
                  <wsse:SecurityTokenReference xmlns:wsse="http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-secext-1.0.xsd">
                      <wsse:Reference URI="#x509"></wsse:Reference>
                  </wsse:SecurityTokenReference>
              </wst:UseKey>
          </wst:RequestSecurityToken>
      """;

    // Other members      
}

Although the usage of instanceof is often considered as an anti-pattern (and lack of proper design), it is pervasive and sneaks into most (if not all) Java codebases.

public static Long getLong(Message message, String key) {
    Object o = message.getContextualProperty(key);
    if (o instanceof Long) {
        return (Long)o;
    } else if (o instanceof Number) {
        return ((Number)o).longValue();
    } else if (o instanceof String) {
        return Long.valueOf(o.toString());
    }
    return null;
}

The introduction of JEP 394: Pattern Matching for instanceof did at least make the instanceof constructs less verbose and more readable:

public static Long getLong(Message message, String key) {
    Object o = message.getContextualProperty(key);
    if (o instanceof Long l) {
        return l;
    } else if (o instanceof Number n) {
        return n.longValue();
    } else if (o instanceof String s) {
        return Long.valueOf(s);
    }
    return null;
}

If we want to make it even more compact, switch expression could help us once again, thanks to enhancements introduced by JEP 441: Pattern Matching for switch:

public static Long getLong(Message message, String key) {
    return switch (message.getContextualProperty(key)) {
        case Long l -> l;
        case Number n ->  n.longValue();
        case String s -> Long.valueOf(s);
        default -> null;
    };
}

The Java language is changing fast, some may say very fast (and we haven't even talked about JVM and standard library changes!). That brings a lot of benefits but at the same time, poses many challenges. Thankfully, the strict compatibility policy that Java language designers prioritize over everything else means that you don't need to rewrite your code, it will still work. It just becomes legacy ... it becomes yet another kind of technical debt to pay ...

I πŸ‡ΊπŸ‡¦ stand πŸ‡ΊπŸ‡¦ with πŸ‡ΊπŸ‡¦ Ukraine.

No comments: