selenium icon indicating copy to clipboard operation
selenium copied to clipboard

[java] Made ``JsonToWebElementConverter`` methods/fields protected

Open AB-xdev opened this issue 5 months ago • 3 comments

User description

So that the class can be properly extended.

🔗 Related Issues

Fixes #15884

💥 What does this PR do?

Make the corresponding fields/methods protected

🔧 Implementation Notes

So that the class can be properly extended/overridden.

💡 Additional Considerations

Don't use private (or similar things) in libraries that other people use.

I already encountered this problem so often that I had to write a small blog post.

🔄 Types of changes

  • Cleanup

PR Type

Other


Description

• Changed private fields/methods to protected in JsonToWebElementConverter • Enables proper class extension and customization • Improves library extensibility for downstream users


Changes walkthrough 📝

Relevant files
Enhancement
JsonToWebElementConverter.java
Change visibility modifiers from private to protected       

java/src/org/openqa/selenium/remote/JsonToWebElementConverter.java

• Changed driver field from private to protected
• Made setOwner()
method protected instead of private
• Made getElementKey() method
protected instead of private
• Made getShadowRootKey() method
protected instead of private

+4/-4     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • AB-xdev avatar Jun 10 '25 14:06 AB-xdev

    CLA assistant check
    All committers have signed the CLA.

    CLAassistant avatar Jun 10 '25 14:06 CLAassistant

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    API Change

    Changing visibility from private to protected is a breaking change that expands the API surface. This should be carefully considered as it makes these methods part of the public contract that subclasses can depend on, potentially limiting future refactoring flexibility.

    protected final RemoteWebDriver driver;
    
    public JsonToWebElementConverter(RemoteWebDriver driver) {
      this.driver = driver;
    }
    
    @Override
    public Object apply(Object result) {
      if (result instanceof Collection<?>) {
        Collection<?> results = (Collection<?>) result;
        return results.stream().map(this).collect(Collectors.toList());
      }
    
      if (result instanceof Map<?, ?>) {
        Map<?, ?> resultAsMap = (Map<?, ?>) result;
        String elementKey = getElementKey(resultAsMap);
        if (null != elementKey) {
          RemoteWebElement element = newRemoteWebElement();
          element.setId(String.valueOf(resultAsMap.get(elementKey)));
          return element;
        }
    
        String shadowKey = getShadowRootKey(resultAsMap);
        if (null != shadowKey) {
          return new ShadowRoot(driver, String.valueOf(resultAsMap.get(shadowKey)));
        }
    
        // some values are converted to null, so we can not use Collectors.toMap here
        Map<Object, Object> converted = new LinkedHashMap<>();
        resultAsMap.forEach((k, v) -> converted.put(k, this.apply(v)));
        return converted;
      }
    
      if (result instanceof RemoteWebElement) {
        return setOwner((RemoteWebElement) result);
      }
    
      if (result instanceof Number) {
        if (result instanceof Float || result instanceof Double) {
          return ((Number) result).doubleValue();
        }
        return ((Number) result).longValue();
      }
    
      return result;
    }
    
    protected RemoteWebElement newRemoteWebElement() {
      return setOwner(new RemoteWebElement());
    }
    
    protected RemoteWebElement setOwner(RemoteWebElement element) {
      if (driver != null) {
        element.setParent(driver);
        element.setFileDetector(driver.getFileDetector());
      }
      return element;
    }
    
    protected String getElementKey(Map<?, ?> resultAsMap) {
      for (Dialect d : Dialect.values()) {
        String elementKeyForDialect = d.getEncodedElementKey();
        if (resultAsMap.containsKey(elementKeyForDialect)) {
          return elementKeyForDialect;
        }
      }
      return null;
    }
    
    protected String getShadowRootKey(Map<?, ?> resultAsMap) {
    

    qodo-merge-pro[bot] avatar Jun 10 '25 14:06 qodo-merge-pro[bot]

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    qodo-merge-pro[bot] avatar Jun 10 '25 14:06 qodo-merge-pro[bot]