With add ns

[grechaw]

Kim's query about namespace adding in es:init-instance led me to understand that a serious bug is in place.

Before this PR, namespace data could come not only from the model, but from the source document, which is inappropriate. This change adds a new helper function to the es module (which is late in the game I know) that makes sure to add namespace information as appropriate to every instance.

Now, if a model declares a type to be of namespace X, it will be persisted properly. Previously, if no namespace were declared, the instance would be persisted with the namespace of source XML, which was just wrong.

[grechaw]

Hi Srikanth, I'll go ahead and ask mr triage about this now to grease the wheels. The only key I had to update for this change (because it was not appropriately tested, frankly) was the one that changes every time I update the instance converter.

Erik reviewed the code change.

[bsrikan]

I have couple of tests failing and not sure if they should. Can you take a look at following functional tests: testInstanceToXMLEnvelopeForNamespace.java testInstanceToCanonicalForNamespace.java testExtractInstanceForNamespace.java testInstanceToEnvelopeForNamespace.java(this is backward compatible version of first one in this list)

all under TestEsConversionModuleGenerator.java

[grechaw]

Will do.

[bsrikan]

examples also would need update for this change?

[grechaw]

example-person, yes. These tests mostly look like they need key updates-- while I'm in here, if it's simple, I'll propose a commit.

---

From: Srikanth Balasubramanian notifications@github.com Sent: Thursday, September 14, 2017 3:45:42 PM To: marklogic/entity-services Cc: Charles Greer; Author Subject: Re: [marklogic/entity-services] With add ns (#348)

examples also would need update for this change?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/marklogic/entity-services/pull/348#issuecomment-329629219, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAXW0JRdix1r4s5eHe07V-ETMhzOoFXhks5siayWgaJpZM4PX-iy.

[grechaw]

looks like some modules need regeneration to make tests pass. That is true for

/conv/valid-2-namespace-gen.xqy

That's what needs to happen, we can chat about getting it done. I'm not sure how these modules were generated.

[bsrikan]

Okay I thought so too. For testExtractInstanceForNamespace.java though the namespace info appears in 2 places. One inside Ship-Address as well. Is it appropriate. The model did not have namespace for entity type ShipDetails but Ship-Address property, part of ET Order, does:

"Ship-Address":[  
    {  
      "$type":"ShipDetails",
      "$namespace":"http://marklogic.com/super",
      "$namespacePrefix":"sup",
       .......

[grechaw]

Hi @bsrikan I left off last night changing some of the keys for the functional tests. I'll continue that process now, and then will be able to show you the differences. I think you'll agree that some of the current expected values are not right.

[grechaw]

In fact, I think you raised concerns about namespaces before and I may have not followed through sufficiently.

[grechaw]

Hi @bsrikan take a look at this PR now -- I added a commit that adjusts the functional tests. I think you'll see why I did so for a few of them. One curiousity was the type of prices... I'm thinking maybe it was not working as expected previously, because the inputs sometimes were not the right type. So I adjusted models and types to be 'double' for prices rather than 'integer'

[bsrikan]

thanks @grechaw . Looks good.

[grechaw]

Updated person example and moving ahead with merge.