Fi 898 generic generator from capability statement #477

ghost · 2020-08-04T21:10:03Z

This PR adds search and interaction tests based off the capability statement for the generic generator. The search tests only perform the basic search and validate functionality. It doesn't handle comparators and modifiers yet.

It also defines a new generic_utilities file that is included in all the generated sequences. You can add functions that all the generated sequences can access through that file. This way, we don' need to modify sequence_base to create shared code.

It also adds the sequence definition files like there is in us core. Right now, it contains search parameter metadata, but it will later contain the terminology and must support information.

I also added a generic_generator_utilities file that is used to store shared code across the generator.

Jammjammjamm

I need to do some more testing, but here are some initial thoughts.

Jammjammjamm · 2020-08-05T13:14:33Z

generator/generic/generic_generator.rb

+      end
+
+      def generate_sequence_definitions(metadata)
+        file_name = sequence_out_path + '/profile_definitions/' + metadata.file_name + '_definitions.rb'


Suggested change

file_name = sequence_out_path + '/profile_definitions/' + metadata.file_name + '_definitions.rb'

output_directory = File.join(sequence_out_path, 'profile_definitions')

file_name = File.join(output_directory, metadata.file_name + '_definitions.rb')

This directory is used also used below. We should probably try to be consistent about using File.join.

Jammjammjamm · 2020-08-05T14:04:31Z

generator/generic/templates/sequence_definition.rb.erb

+# frozen_string_literal: true
+
+module Inferno
+  module USCore310ProfileDefinitions


This needs a new name.

@czh-orz renamed this to ProfileDefinitions

It can't just be ProfileDefinitions because then every generated sequence will share the same definitions. It needs to be Inferno::SOMETHING_UNIQUEProfileDefinitions or Inferno::SOMETHING_UNIQUE::ProfileDefinitions.

Renamed so it includes the path.

If the module is in the ig directory it will be named IgProfileDefinitions and if its in the uscore directory it will be UscoreProfileDefinition. Not super familiar with all the metadata attributes that would be available to use here, but settled on this after talking with @czh-orz.

Jammjammjamm · 2020-08-05T14:49:56Z

generator/sequence_metadata.rb

+          .find { |rest| rest['mode'] == 'server' }['resource']
+          .find { |resource| resource['type'] == profile['type'] }
+
+        @search_parameter_metadata = capabilities['searchParam']&.map do |param|


We should seriously consider moving the search stuff to its own class in the near future.

Agreed, but I think "near future" needs to be a future PR 🙂 .

generator/sequence_metadata.rb

Jammjammjamm · 2020-08-05T14:57:53Z

generator/sequence_metadata.rb

+      def search_combinations_from_capability_statement(capabilities)
+        search_combo_url = 'http://hl7.org/fhir/StructureDefinition/capabilitystatement-search-parameter-combination'
+        capabilities['extension']
+            &.select { |ext| ext['url'] == search_combo_url }


Indentation is wonky in this method.

I updated the spacing and removed the redundant variable.

The spacing is still a little wonky but Rubocop wants it this way 🤷‍♂️ .

generator/generic/static/generic_utilities.rb

Co-authored-by: Stephen MacVicar <Jammjammjamm@users.noreply.github.com>

Jammjammjamm · 2020-08-07T12:59:35Z

generator/sequence_metadata.rb

+        capabilities['extension']
+          &.select { |ext| ext['url'] == search_combo_url }
+          &.map do |combo|
+          expectation = combo['extension'].find { |ext| ext['url'] == EXPECTATION_URL }['valueCode']


indentation.

As with the previous comment on this, rubocop has a problem with me fixing the indentation of the code in the do block.

I tried updating rubocop to see if that fixed it incase a recent update (like rubocop/rubocop#6749) had changed this, but no 🎲 .

Instead I added disabled the rubocop indentation checks for just that block of code. Hilariously, I also had to disable the CommentIndentation check as well to indent the rubocop:disable directive the way I wanted.

Jammjammjamm · 2020-08-07T13:09:53Z

generator/generic_generator_utilities.rb

+
+module Inferno
+  module Generator
+    module GenericGeneratorUtilties


I don't think this module forms a cohesive unit.

structure_to_string is only used by GenericGenerator

create_search_validation is only used by SequenceMetadata

the other methods are helpers for create_search_validation

The name doesn't help you know why a method would be in here rather than in one of those classes.

I wanted to leave the create_search_validation stuff out of SequenceMetadata because I wanted to keep that file for just profile information. create_search_validation just uses the metadata to form code and doesn't add any metadata. I couldn't put it in search_test either because it doesn't go inside of a test itself. So I just put it in the utilities file because I didn't want to cluttter up SequenceMetadata with non-metadata stuff I guess.

structure_to_string is going to be needed later on when I add must support stuff.

Jammjammjamm · 2020-08-07T13:45:09Z

generator/generator_base.rb

+            begin
+              new_resource_json = JSON.parse(File.read(resource))
+            rescue JSON::ParserError # failed to parse json
+              next


Should probably log something here.

Jammjammjamm · 2020-08-07T13:48:29Z

generator/generic_generator_utilities.rb

+        validate_functions
+      end
+
+      def search_param_match_found_code(type, element)


Rather than duplicating this code in each sequence, can you stick this method's logic (without all the stringifying) into SequenceUtilities.

I think this is reasonable. But I do remember Rob saying something about writing out functions explicitly like this in each sequence to make it easier to understand. But anyway, can we leave this to a future PR?

My preference in general is for it to be fairly obvious what a given test does without having to continually hop to other shared code locations. Yeah, it results in duplication of generated code, but that duplicated code never needs to managed indivdually (its managed as a group in the generator). Just a preference. But if we end up with a lot of tests that basically are just a call to a shared function that needs to be followed to understand what is going on at all i think we are doing it wrong.

…le name

arscan

I'm good with this as a start. We may reconsider the name (SimpleSearchGenerator or something) -- our intent is to not have a Generic Generator that will be expanded to do everything. But we can explore that as we continue down this path.

radamson

Just ran through it again and everything went smoothly.

radamson requested review from Jammjammjamm and radamson August 5, 2020 11:20

radamson force-pushed the fi-898-generic-generator-from-capability-statement branch from 81a0282 to d44eb61 Compare August 5, 2020 13:58

Jammjammjamm reviewed Aug 5, 2020

View reviewed changes

Chase and others added 18 commits August 6, 2020 12:50

saving some progress

7e4c985

more progress

c751a7f

add interaction test

f1806c2

fix some interaction tests

cfdf984

some fixes

4621846

more fixes

768698a

get correct type for parameters

66223a3

add requirements to module file for generic ui

87a3e75

nil safety for multiple_and_expecation and multiple_or_expectation

2b19c8e

break out search combos and add some nil safety

d487170

move validation tests after read and search tests

7a6efbb

Update generator/generic/generic_generator.rb

636fc73

Co-authored-by: Stephen MacVicar <Jammjammjamm@users.noreply.github.com>

rubocop and fix spacing

467b335

use capabilities attr_accessor instead of passing it around

e421cda

fix search combo change and change definitions module

e045686

rename generic_utilities to sequence_utilities

f8221c0

use map in basic_searches_from_capability_statement

755a88c

catch json parsing error

31020e7

radamson force-pushed the fi-898-generic-generator-from-capability-statement branch from b308c72 to 31020e7 Compare August 6, 2020 16:50

radamson and others added 3 commits August 6, 2020 14:47

update generator to test against correct profile

e9d87d4

save resources with profile reference

84fab52

us path when making sequence class name

40c9969

Jammjammjamm reviewed Aug 7, 2020

View reviewed changes

radamson added 3 commits August 7, 2020 11:03

rename ProfileDefinitions to include module path

e946a81

Definitions -> ProfileDefinitions to match what previously existed

ff99f47

fix indentation

f3240fc

Chase added 2 commits August 7, 2020 13:40

add logger line for json parsing error and fix profiledefinition modu…

1c4f538

…le name

fix rubocop spacing issue

467895f

arscan approved these changes Aug 7, 2020

View reviewed changes

radamson approved these changes Aug 7, 2020

View reviewed changes

radamson merged commit 1364e7b into development Aug 7, 2020

radamson deleted the fi-898-generic-generator-from-capability-statement branch August 7, 2020 19:18

radamson mentioned this pull request Aug 9, 2020

Version 2.10.0 #481

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fi 898 generic generator from capability statement #477

Fi 898 generic generator from capability statement #477

ghost commented Aug 4, 2020

Jammjammjamm left a comment

Jammjammjamm Aug 5, 2020

Jammjammjamm Aug 5, 2020

radamson Aug 5, 2020

Jammjammjamm Aug 7, 2020

radamson Aug 7, 2020 •

edited

Jammjammjamm Aug 5, 2020

radamson Aug 7, 2020

Jammjammjamm Aug 5, 2020

radamson Aug 5, 2020

Jammjammjamm Aug 7, 2020

radamson Aug 7, 2020

Jammjammjamm Aug 7, 2020

ghost Aug 7, 2020 •

edited by ghost

Jammjammjamm Aug 7, 2020

ghost Aug 7, 2020

Jammjammjamm Aug 7, 2020

ghost Aug 7, 2020

arscan Aug 7, 2020

arscan left a comment

radamson left a comment

	file_name = sequence_out_path + '/profile_definitions/' + metadata.file_name + '_definitions.rb'
	output_directory = File.join(sequence_out_path, 'profile_definitions')
	file_name = File.join(output_directory, metadata.file_name + '_definitions.rb')

Fi 898 generic generator from capability statement #477

Fi 898 generic generator from capability statement #477

Conversation

ghost commented Aug 4, 2020

Jammjammjamm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radamson Aug 7, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Aug 7, 2020 • edited by ghost

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arscan left a comment

Choose a reason for hiding this comment

radamson left a comment

Choose a reason for hiding this comment

radamson Aug 7, 2020 •

edited

ghost Aug 7, 2020 •

edited by ghost