Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complete serialisation for saved searches #4

Closed
stuarta0 opened this issue May 13, 2015 · 5 comments
Closed

Complete serialisation for saved searches #4

stuarta0 opened this issue May 13, 2015 · 5 comments

Comments

@stuarta0
Copy link
Owner

Provide base code for serialisation of an IParameter graph in the Standalone project based on XML. Protocol Buffers is nice to have for using over RESTful APIs, but for now it just needs to work locally.

Current implementation being developed under Standalone.Serialization.DTO.Criterion. The existing DTOs are heavily tied to protobuf so move these to their own namespace and redesign the classes for use with standard XML serialization.

@stuarta0
Copy link
Owner Author

A better alternative to the ParameterAssembler visitor concept in 17e5a01 might be to just put the Create/Restore functions in each DTO with abstract Create/Restore on the ParameterDTO so the ParameterAssembler just calls the objects func and returns that. It's still within the realms of concern for the DTO to do this.

Since one of the main reasons for the visitor pattern is to add additional functionality without requiring changes to the types it operates on, and I really don't need to be adding any more functionality and it can be solved using standard polymorphism, then just use standard polymorphism.

@stuarta0
Copy link
Owner Author

I started to investigate polymorphism for restore/create but it starts coming unstuck when either process requires further knowledge that is not the concern of the DTO being created (e.g. restoring a FieldPath needs a FieldPathAssembler which has its own requirements and is definitely nothing to do with a ParameterDTO).

After beginning to implement the Visitor pattern in 1090f72 it's also become evident this is a flawed approach. The Visitor pattern has some serious limitations.

  • It successfully removes the concern of how to transform data beyond an individual parameter's concern, but starts falling down when you need to do something with the new objects. Either using the visitor object to store state of the conversion due to the traditional void Visit signature, or muddying the contract with IParameter Visit which begins to erode the reuseability of the pattern. Plus the data structure follows the composite pattern, meaning we have an unknown depth of data structure making the state difficult to manage.
  • The Restore process can implement Accept/Visit behaviour on the DTO's and it works well, but when it comes to the Create process the visit pattern needs to exist on IParameter. STOP! This means embedding the visitor pattern into the core library and providing concrete interfaces with all known types defined for the visitor. This destroys extensibility as any additional parameters a user wants to add can't be added to the compiled interface.

Thinking of the problem trying to be solved is this: we're trying to serialize an IParameter composite type to XML.

  • We could implement our own serializer to control every aspect of how the XML is generated.
  • We could use DTO's to transform and simplify the data structure and use the built-in serializer.

Using our own serializer limits us to a specific implementation of serialization (in this case, XML). We'd have to rewrite all this code if we were to serialize to another format.

If we use DTO's we separate the concern of transforming the data to persisting it which is good, but how do we cleanly manage the transformations?

  • Have an assembler that has a big conditional block to transform each and every type.
    • Pros: it's simple.
    • Cons: source of bugs, runtime type checking, inflexible, hard to maintain.
  • Have an assembler using the visitor pattern to transform
    • Pros: compile time checking, no conditionals
    • Cons: everything above (high coupling, poor handling of composite data structures, lack of extensibility)
  • Use the chain of responsibility pattern for the assembler
    • Pros: modular handling of transformations, extensible
    • Cons: runtime type checking

For now, we'll attempt the chain of responsibility pattern. The caveat of runtime type-checking is unfortunate, but it's easily extended (no modifying of existing classes or issues of black box behaviour from inheriting the assembler and providing further logic in create/restore), and doesn't require modification of any other classes which keeps them decoupled.

stuarta0 added a commit that referenced this issue May 14, 2015
As per documentation in #4, Visitor pattern is unacceptable and we'll be
using Chain of Responsibility instead.  This commit contains some of the
classes and structure required to implement this behaviour.
@stuarta0
Copy link
Owner Author

After discussion, the only way to truly serialize what a user sees on screen and restore it successfully is to persist the state of the UI, not the final parameter (which undergoes parsing which transforms it's shape).

To do this, we'd need to persist:

  • The type of search that was created (Preset, Standard or Advanced)
  • For each element:
    • The FieldPath that is affected
    • The ParameterBuilder in use (not the IParameter itself which comes after parsing and transformations)
    • The raw value from the control. TBH the control should be able to provide whatever value it likes (as long as it's serializable) because it's the one that has to deal with restoring it

Issues to overcome are:

  • What if the configuration changes and the FieldPath is no longer valid?
    • Tough. The only way to fix it would be to save the entire configuration for every search, but that still wouldn't overcome if the database itself has changed and the configuration itself is no longer valid.
  • What if the UI changes? e.g. the default control for a list is no longer a ComboBox but a ListBox?
    • Try loading the values just like the GetValues/SetValues behaviour that currently exists when the UI factory changes the control (in the Standard search control)
    • Don't load it
    • Highlight the control as having changed (would need to persist the state of the control to know this is different)

In addition, we could still persist the final parameter set just in case we can't restore the parameters to the UI. The user could be offered the option to perform the search even though it can't be displayed. Or it could be used as a sanity check to ensure the current UI and parsers generate the same search structure as the original (maybe not values though as a date field with "last week" would generate a different value if opened in the future).

@stuarta0
Copy link
Owner Author

Add a project version number (alongside ID, configuration and connections). When a search is serialised it can save this version number too, so if either get out of date we'll know that the saved search is may no longer be valid.

@stuarta0
Copy link
Owner Author

Merged to master at 8521d66

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant