Skip to content

Comments

Release/1.3#68

Open
sloppylopez wants to merge 6 commits intomainfrom
release/1.3
Open

Release/1.3#68
sloppylopez wants to merge 6 commits intomainfrom
release/1.3

Conversation

@sloppylopez
Copy link
Owner

Description

Brief description of the changes in this PR.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Performance improvement
  • Test improvements

Related Issue

Fixes #(issue number)

Changes Made

  • List key changes made in this PR
  • Be specific about what was added/modified/removed

Testing

  • All existing tests pass
  • New tests added for new functionality
  • Manual testing completed
  • Test coverage remains above 80%

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated CHANGELOG.md with my changes

Screenshots (if applicable)

Add screenshots to help explain your changes.

Additional Notes

Any additional information that reviewers should know.

… name extraction

- Implemented a workaround for the WireMock `equalToXml` bug with namespaced SOAP XML by using `matchesXPath` for requests with dynamic attributes.
- Improved method name extraction logic to correctly handle test method names containing underscores, ensuring proper application of ignore patterns.
- Updated XPath extraction to include discriminators for distinguishing between multiple parameterized requests, preventing response mixing during playback.
- Enhanced documentation in PITFALLS.md to outline the new SOAP handling approach and its implications.
… mappings handling and enhance XML matching logic

- Introduced a temporary mappings directory in StableMockExtension to isolate test method mappings and prevent conflicts during playback.
- Updated merge methods in MappingStorage and MultipleAnnotationMappingStorage to support output directories for better organization of merged mappings.
- Enhanced WireMockServerManager's XML handling to incorporate ignore patterns, improving the accuracy of XPath matches for SOAP requests.
- Added utility methods for extracting ignored local names and finding leaf elements in XML, streamlining the XML processing logic.
…le predicates

- Updated the logic to collect up to 3 known discriminator elements for better differentiation of parameterized cases.
- Improved fallback mechanism to build predicates from non-empty leaf element texts, ensuring accurate XPath matching.
- Streamlined the handling of ignored names and duplicate predicates to enhance overall XML processing efficiency.
- Added support for parameterized tests in StableMockExtension to ensure each invocation matches its own stubs by setting scenario states.
- Enhanced SingleAnnotationMappingStorage to add scenario names and required states for parameterized invocations, improving test isolation.
- Updated ParameterizedSoapTest and ParameterizedTestExample to execute tests in the same thread, ensuring consistent behavior during parallel execution.
- Introduced utility methods for managing parameterized invocation indices, streamlining the handling of multiple test cases.
…pport

- Introduced the ability to specify fields that must not be ignored during dynamic field detection, allowing for better stub matching in scenarios with similar requests.
- Updated the `@U` annotation to include a `dontIgnore` attribute for specifying protected fields.
- Enhanced `StableMockExtension` and `WireMockServerManager` to handle the new `dontIgnore` logic, ensuring these fields remain in the request matcher.
- Improved documentation in `README.md` and `PITFALLS.md` to clarify the usage of protected dynamic fields.
- Added unit tests to validate the exclusion of protected fields from ignore patterns during save and load operations.
… in StableMock

- Replaced references to `RatePlanCode` and `RoomTypeCode` with `SampleFieldA` and `SampleFieldC` in README.md, PITFALLS.md, and various Java files to reflect updated examples for protected dynamic fields.
- Enhanced clarity in comments and documentation regarding the handling of dynamic fields to ensure accurate stub matching during playback.
- Adjusted unit tests to align with the new field names, improving consistency across the codebase.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances StableMock’s playback/recording reliability for parameterized tests and SOAP/XML bodies by adding mechanisms to (a) prevent “over-ignoring” discriminator fields and (b) avoid XMLUnit issues with namespaced SOAP, while also introducing support for external ignore-pattern files and temporary merge output directories.

Changes:

  • Add “protected dynamic fields” support (config + annotation) so selected paths are excluded from ignore_patterns on save/load.
  • Improve playback for parameterized runs via scenario state handling and safer per-invocation isolation/merging behavior.
  • Add/expand documentation and tests around the new ignore/protection behavior and common pitfalls.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/test/java/com/stablemock/core/analysis/XmlFieldDetectorTest.java Updates XML test payloads used for heuristic detection.
src/test/java/com/stablemock/core/analysis/AnalysisResultStorageProtectedFieldsTest.java New tests for filtering protected fields and verifying save/load behavior.
src/main/java/com/stablemock/spring/BaseStableMockTest.java Adds override point for protected dynamic fields (defaults to config).
src/main/java/com/stablemock/core/storage/SingleAnnotationMappingStorage.java Adds output-dir support for merges + parameterized scenario augmentation in merged mappings.
src/main/java/com/stablemock/core/storage/MultipleAnnotationMappingStorage.java Adds output-dir support when merging multi-URL annotation mappings.
src/main/java/com/stablemock/core/storage/MappingStorage.java Exposes new overloads for merge APIs that accept an output directory.
src/main/java/com/stablemock/core/server/WireMockServerManager.java Adds dontIgnore plumbing, SOAP XPath matcher discriminator logic, priority tweaks, and scenario state setter.
src/main/java/com/stablemock/core/context/ExtensionContextManager.java Stores/retrieves a temp mappings directory in the extension context store.
src/main/java/com/stablemock/core/config/StableMockConfig.java Introduces stablemock.protectedDynamicFields parsing into a Set<String>.
src/main/java/com/stablemock/core/analysis/DynamicFieldAnalysisOrchestrator.java Passes through test class + dontIgnore set for protected-field filtering on persist.
src/main/java/com/stablemock/core/analysis/AnalysisResultStorage.java Filters protected paths out of saved/loaded ignore_patterns; loads ignore-patterns.json.
src/main/java/com/stablemock/U.java Adds dontIgnore() to prevent specific paths from being auto-ignored.
src/main/java/com/stablemock/StableMockExtension.java Uses temp mappings dir for merges/playback, collects dontIgnore, adds parameterized scenario locking/state logic.
examples/spring-boot-example/src/test/java/example/ParameterizedTestExample.java Marks parameterized examples as SAME_THREAD to avoid scenario-state races.
examples/spring-boot-example/src/test/java/example/ParameterizedSoapTest.java Marks parameterized SOAP example as SAME_THREAD and adds response assertion.
docs/PITFALLS.md Documents SOAP equalToXml/XMLUnit issue, underscore method-name pitfall, and parameterized scenario requirements.
README.md Documents ignore-patterns file support and protected dynamic fields configuration/usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +270 to +271
java.lang.reflect.Method m = testClass.getMethod("getProtectedDynamicFields");
if (m.getReturnType() == Set.class && java.lang.reflect.Modifier.isStatic(m.getModifiers())) {
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getProtectedFieldsForTestClass(Class<?>) uses testClass.getMethod("getProtectedDynamicFields"), which only finds public methods. BaseStableMockTest.getProtectedDynamicFields() is protected, so this reflection call will fail and subclass overrides will never be applied. Use getDeclaredMethod (and setAccessible(true)), or make getProtectedDynamicFields() public so the override mechanism actually works.

Suggested change
java.lang.reflect.Method m = testClass.getMethod("getProtectedDynamicFields");
if (m.getReturnType() == Set.class && java.lang.reflect.Modifier.isStatic(m.getModifiers())) {
java.lang.reflect.Method m = null;
Class<?> current = testClass;
while (current != null &&
com.stablemock.spring.BaseStableMockTest.class.isAssignableFrom(current)) {
try {
m = current.getDeclaredMethod("getProtectedDynamicFields");
m.setAccessible(true);
break;
} catch (NoSuchMethodException e) {
// Try superclass
current = current.getSuperclass();
}
}
if (m != null &&
m.getReturnType() == Set.class &&
java.lang.reflect.Modifier.isStatic(m.getModifiers())) {

Copilot uses AI. Check for mistakes.
private static String extractSoapXPathMatch(String xml, java.util.List<String> ignorePatterns) {
try {
javax.xml.parsers.DocumentBuilderFactory factory = javax.xml.parsers.DocumentBuilderFactory.newInstance();
factory.setNamespaceAware(true);
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractSoapXPathMatch parses XML with a namespace-aware DocumentBuilderFactory but does not disable DTDs/external entities. This is inconsistent with normalizeXmlStringWithPlaceholders() (which sets secure features) and can expose XXE risks if a recorded/loaded SOAP body contains a DOCTYPE. Configure the factory to disallow DOCTYPE and external entity expansion (same flags used elsewhere in this class) before parsing.

Suggested change
factory.setNamespaceAware(true);
factory.setNamespaceAware(true);
// Harden XML parsing against XXE and disallow DOCTYPEs, consistent with normalizeXmlStringWithPlaceholders()
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);

Copilot uses AI. Check for mistakes.
Comment on lines +997 to +998
if (val.indexOf('\'') < 0) return "'" + val + "'";
return "\"" + val.replace("\"", "\"\"") + "\"";
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xpathLiteral produces invalid XPath when val contains both single and double quotes. The current fallback wraps the value in double quotes and doubles embedded quotes, which is not valid XPath escaping. Use a proper XPath literal builder (e.g., concat('a', '"', 'b') when both quote types are present) so generated matchesXPath expressions remain valid for all values.

Suggested change
if (val.indexOf('\'') < 0) return "'" + val + "'";
return "\"" + val.replace("\"", "\"\"") + "\"";
// If there are no single quotes, we can safely wrap in single quotes
if (val.indexOf('\'') < 0) {
return "'" + val + "'";
}
// If there are no double quotes, we can safely wrap in double quotes
if (val.indexOf('"') < 0) {
return "\"" + val + "\"";
}
// Value contains both single and double quotes: build using concat()
StringBuilder sb = new StringBuilder("concat(");
String[] parts = val.split("'");
for (int i = 0; i < parts.length; i++) {
if (i > 0) {
// Add a single quote character between parts: "'"
sb.append(", \"'\", ");
}
sb.append('\'').append(parts[i]).append('\'');
}
sb.append(')');
return sb.toString();

Copilot uses AI. Check for mistakes.

ExtensionContextManager.MethodLevelStore methodStore = new ExtensionContextManager.MethodLevelStore(context);
String testMethodIdentifier = TestContextResolver.getTestMethodIdentifier(context);
boolean isParameterized = testMethodIdentifier.contains("[");
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable testMethodIdentifier may be null at this access as suggested by this null guard.

Suggested change
boolean isParameterized = testMethodIdentifier.contains("[");
boolean isParameterized = testMethodIdentifier != null && testMethodIdentifier.contains("[");

Copilot uses AI. Check for mistakes.
private static int parseParameterizedInvocationIndex(String methodDirName) {
if (methodDirName == null) return -1;
java.util.regex.Matcher m = java.util.regex.Pattern.compile("\\[(\\d+)\\]").matcher(methodDirName);
return m.find() ? Integer.parseInt(m.group(1)) : -1;
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential uncaught 'java.lang.NumberFormatException'.

Suggested change
return m.find() ? Integer.parseInt(m.group(1)) : -1;
if (!m.find()) {
return -1;
}
String indexStr = m.group(1);
try {
return Integer.parseInt(indexStr);
} catch (NumberFormatException e) {
// If the index is not a valid int (e.g., too large), treat as no match.
return -1;
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant