Using DataSet view for Builders

[ankit27755]

---

DeputyDev generated PR summary:

---

Size L: This PR changes include 131 lines and should take approximately 1-3 hours to review

---

The pull request (PR) titled "Using DataSet view for Builders" primarily focuses on refactoring the way data is accessed by builders within the DataBuilderFramework. The main changes include:

1. Removal of Deprecated Method: The PR removes the deprecated getDataSet(DataBuilder builder) method from DataBuilderContext, which was previously used to get a filtered dataset accessible to a specific builder.

2. Introduction of DataSetView: A new class DataSetView is introduced. This class extends DataSet and is used to provide a view of the dataset that only includes data accessible to a specific builder. It effectively replaces the functionality of the deprecated method with a more structured approach.

3. Modification in DataSetAccessor: The getAccesibleDataSetFor method in DataSetAccessor now returns a DataSetView instead of a filtered DataSet. This change aligns with the new approach of using DataSetView.

4. Refactoring and Code Cleanup: Some minor refactoring and code cleanup are done, such as removing unused methods and improving formatting.

5. Performance Test Adjustment: The test in HomePageControllerTest is adjusted to run a larger number of iterations, changing the loop limit from 100,000 to 10,000,000.

This PR refines the data access mechanism for builders by introducing a more robust and maintainable way to manage data visibility, leveraging the new DataSetView class. This change makes the codebase cleaner and more efficient in handling data access permissions.

---

DeputyDev generated PR summary until 9404a37

[deputydev-agent]

SECURITY: The DataSetView implementation in src/main/java/io/appform/databuilderframework/model/DataSetView.java contains a TODO comment on line 60 that indicates a potential security vulnerability. The containsAll() method does not properly filter the requiredKeys against allowedKeys, which could allow a builder to check for the existence of keys it shouldn't have access to. This information disclosure vulnerability could be exploited if a malicious DataBuilder checks for existence of data it shouldn't be able to see.

public boolean containsAll(final Collection<String> requiredKeys) {
    // Filter requiredKeys to only include allowedKeys
    Set<String> filteredKeys = requiredKeys.stream()
        .filter(allowedKeys::contains)
        .collect(Collectors.toSet());
    return dataSet.containsAll(filteredKeys);
}

[deputydev-agent]

CODE_MAINTAINABILTIY: The DataSetView implementation doesn't properly override all methods from DataSet, which could lead to unexpected behavior. Specifically, methods like deepCopy(), merge(), and other common operations are missing, which violates the Liskov Substitution Principle (LSP), a key SOLID principle.

// Additional methods to implement for full DataSet compatibility
public DataSet deepCopy() {
    return new DataSetView(dataSet.deepCopy(), new HashSet<>(allowedKeys));
}

@Override
public DataSet merge(DataDelta dataDelta) {
    throw new UnsupportedOperationException("DataSetView is read-only and does not support merge operations");
}

// Ensure all relevant DataSet methods are properly overridden with appropriate behavior

[deputydev-agent]

ERROR: The DataSetView class extends DataSet but doesn't properly implement or override several methods. It throws UnsupportedOperationException for methods like add(), filter(), and copyInto() but fails to annotate them with @OverRide. This makes the class hierarchy error-prone and can cause confusion about which methods are actually overriding parent methods versus just hiding them.

// Add @Override annotation to all methods that override parent methods
@Override
public DataSetView add(String dataName, Data data) {
    throw new UnsupportedOperationException();
}

@Override
public DataSetView add(final Collection<Data> data) {
    throw new UnsupportedOperationException();
}

@Override
public <T extends Data> DataSetView add(T data) {
    throw new UnsupportedOperationException();
}

@Override
public Map<String, Data> filter(final Collection<String> requiredKeys) {
    throw new UnsupportedOperationException();
}

@Override
public boolean containsAll(final Collection<String> requiredKeys) {
    // Implementation here
}

@Override
public void copyInto(final Map<String, Data> outMap, Collection<String> excludedKeys) {
    throw new UnsupportedOperationException();
}

[deputydev-agent]

• PERFORMANCE_OPTIMIZATION: The test loop iteration count has been drastically increased from 100,000 to 10,000,000 (a 100x increase). This change could lead to excessively long test execution times and potential resource exhaustion, especially in resource-constrained CI/CD environments. It's important to ensure that regression tests remain efficient to provide timely feedback during development.

for(long i = 0; i < 100_000; i++) {
    HomePageResponse response = executor.run(homePageDataFlow, request).get(HomePageResponse.class);
    Assert.assertNotNull(response);
    //System.out.println(new ObjectMapper().writerWithDefaultPrettyPrinter().writeValueAsString(response));
}

[deputydev-agent]

DeputyDev has completed a review of your pull request for commit 9404a37.